Commit Graph

5368 Commits

Author SHA1 Message Date
Dean Rasheed 5defdef8aa Update MERGE docs to mention that ONLY is supported.
Commit 7103ebb7aa added support for MERGE, which included support for
inheritance hierarchies, but didn't document the fact that ONLY could
be specified before the source and/or target tables to exclude tables
inheriting from the tables specified.

Update merge.sgml to mention this, and while at it, add some
regression tests to cover it.

Dean Rasheed, reviewed by Nathan Bossart.

Backpatch to 15, where MERGE was added.

Discussion: https://postgr.es/m/CAEZATCU0XM-bJCvpJuVRU3UYNRqEBS6g4-zH%3Dj9Ye0caX8F6uQ%40mail.gmail.com
2022-12-09 10:00:01 +00:00
Andres Freund 3f0e786ccb meson: Add 'running' test setup, as a replacement for installcheck
To run all tests that support running against existing server:
$ meson test --setup running

To run just the main pg_regress tests against existing server:
$ meson test --setup running regress-running/regress

To ensure the 'running' setup continues to work, test it as part of the
freebsd CI task.

Discussion: https://postgr.es/m/CAH2-Wz=XDQcmLoo7RR_i6FKQdDmcyb9q5gStnfuuQXrOGhB2sQ@mail.gmail.com
2022-12-07 12:13:35 -08:00
Andres Freund 79f7c482f6 meson: Basic cygwin support
There likely are further issues, but as evidenced by the CI task proposed by
Justin in the referenced thread, this suffices to build and run basic tests in
cygwin (some fixes for the test infrastructure are needed, but that's
independent of the meson aspect).

Author: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20221021034040.GT16921@telsasoft.com
2022-12-06 11:25:54 -08:00
Alvaro Herrera a61b1f7482
Rework query relation permission checking
Currently, information about the permissions to be checked on relations
mentioned in a query is stored in their range table entries.  So the
executor must scan the entire range table looking for relations that
need to have permissions checked.  This can make the permission checking
part of the executor initialization needlessly expensive when many
inheritance children are present in the range range.  While the
permissions need not be checked on the individual child relations, the
executor still must visit every range table entry to filter them out.

This commit moves the permission checking information out of the range
table entries into a new plan node called RTEPermissionInfo.  Every
top-level (inheritance "root") RTE_RELATION entry in the range table
gets one and a list of those is maintained alongside the range table.
This new list is initialized by the parser when initializing the range
table.  The rewriter can add more entries to it as rules/views are
expanded.  Finally, the planner combines the lists of the individual
subqueries into one flat list that is passed to the executor for
checking.

To make it quick to find the RTEPermissionInfo entry belonging to a
given relation, RangeTblEntry gets a new Index field 'perminfoindex'
that stores the corresponding RTEPermissionInfo's index in the query's
list of the latter.

ExecutorCheckPerms_hook has gained another List * argument; the
signature is now:
typedef bool (*ExecutorCheckPerms_hook_type) (List *rangeTable,
					      List *rtePermInfos,
					      bool ereport_on_violation);
The first argument is no longer used by any in-core uses of the hook,
but we leave it in place because there may be other implementations that
do.  Implementations should likely scan the rtePermInfos list to
determine which operations to allow or deny.

Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqGjJDmUhDSfv-U2qhKJjt9ST7Xh9JXC_irsAQ1TAUsJYg@mail.gmail.com
2022-12-06 16:09:24 +01:00
Tom Lane d69d01ba9d Fix Memoize to work with partitionwise joining.
A couple of places weren't up to speed for this.  By sheer good
luck, we didn't fail but just selected a non-memoized join plan,
at least in the test case we have.  Nonetheless, it's a bug,
and I'm not quite sure that it couldn't have worse consequences
in other examples.  So back-patch to v14 where Memoize came in.

Richard Guo

Discussion: https://postgr.es/m/CAMbWs48GkNom272sfp0-WeD6_0HSR19BJ4H1c9ZKSfbVnJsvRg@mail.gmail.com
2022-12-05 12:36:40 -05:00
Dean Rasheed 2605643a3a Fix DEFAULT handling for multi-row INSERT rules.
When updating a relation with a rule whose action performed an INSERT
from a multi-row VALUES list, the rewriter might skip processing the
VALUES list, and therefore fail to replace any DEFAULTs in it. This
would lead to an "unrecognized node type" error.

The reason was that RewriteQuery() assumed that a query doing an
INSERT from a multi-row VALUES list would necessarily only have one
item in its fromlist, pointing to the VALUES RTE to read from. That
assumption is correct for the original query, but not for product
queries produced for rule actions. In such cases, there may be
multiple items in the fromlist, possibly including multiple VALUES
RTEs.

What is required instead is for RewriteQuery() to skip any RTEs from
the product query's originating query, which might include one or more
already-processed VALUES RTEs. What's left should then include at most
one VALUES RTE (from the rule action) to be processed.

Patch by me. Thanks to Tom Lane for reviewing.

Back-patch to all supported branches.

Discussion: https://postgr.es/m/CAEZATCV39OOW7LAR_Xq4i%2BLc1Byux%3DeK3Q%3DHD_pF1o9LBt%3DphA%40mail.gmail.com
2022-12-03 12:11:33 +00:00
Tom Lane cabfb8241d Fix psql's \sf and \ef for new-style SQL functions.
Some options of these commands need to be able to identify the start
of the function body within the output of pg_get_functiondef().
It used to be that that always began with "AS", but since the
introduction of new-style SQL functions, it might also start with
"BEGIN" or "RETURN".  Fix that on the psql side, and add some
regression tests.

Noted by me awhile ago, but I didn't do anything about it.
Thanks to David Johnston for a nag.

Discussion: https://postgr.es/m/AM9PR01MB8268D5CDABDF044EE9F42173FE8C9@AM9PR01MB8268.eurprd01.prod.exchangelabs.com
2022-12-02 14:24:44 -05:00
Tom Lane b23cd185fd Remove logic for converting a table to a view.
Up to now we have allowed manual creation of an ON SELECT rule on
a table to convert it into a view.  That was never anything but a
horrid, error-prone hack though.  pg_dump used to rely on that
behavior to deal with cases involving circular dependencies,
where a dependency loop could be broken by separating the creation
of a view from installation of its ON SELECT rule.  However, we
changed pg_dump to use CREATE OR REPLACE VIEW for that in commit
d8c05aff5 (which was later back-patched as far as 9.4), so there's
not a good argument anymore for continuing to support the behavior.

The proximate reason for axing it now is that we found that the
new statistics code has failure modes associated with the relkind
change caused by this behavior.  We'll patch around that in v15,
but going forward it seems like a better idea to get rid of the
need to support relkind changes.

Discussion: https://postgr.es/m/CALDaNm2yXz+zOtv7y5zBd5WKT8O0Ld3YxikuU3dcyCvxF7gypA@mail.gmail.com
2022-12-02 12:14:32 -05:00
Tom Lane 1dd6700f44 Fix under-parenthesized display of AT TIME ZONE constructs.
In commit 40c24bfef, I forgot to use get_rule_expr_paren() for the
arguments of AT TIME ZONE, resulting in possibly not printing parens
for expressions that need it.  But get_rule_expr_paren() wouldn't have
gotten it right anyway, because isSimpleNode() hadn't been taught that
COERCE_SQL_SYNTAX parent nodes don't guarantee sufficient parentheses.
Improve all that.  Also use this methodology for F_IS_NORMALIZED, so
that we don't print useless parens for that.

In passing, remove a comment that was obsoleted later.

Per report from Duncan Sands.  Back-patch to v14 where this code
came in.  (Before that, we didn't try to print AT TIME ZONE that way,
so there was no bug just ugliness.)

Discussion: https://postgr.es/m/f41566aa-a057-6628-4b7c-b48770ecb84a@deepbluecap.com
2022-12-01 11:38:14 -05:00
Tom Lane d5515ca7cf Reject missing database name in pg_regress and cohorts.
Writing "pg_regress --dbname= ..." led to a crash, because
we weren't expecting there to be no database name supplied.
It doesn't seem like a great idea to run regression tests
in whatever is the user's default database; so rather than
supporting this case let's explicitly reject it.

Per report from Xing Guo.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/CACpMh+A8cRvtvtOWVAZsCM1DU81GK4DL26R83y6ugZ1osV=ifA@mail.gmail.com
2022-11-30 13:01:41 -05:00
Michael Paquier d2a4490401 Add regression tests for psql's \o and \g on files
This adds coverage for a few scenarios not checked yet in psql, with
multiple query combined across files defined by \o, \g or both at the
same time.  The results are saved in the output files defined, then
reloaded back to check what has been written to them.

Author: Daniel Verite
Discussion: https://postgr.es/m/25c2bb5b-9012-40f8-8088-774cb764046d@manitou-mail.org
2022-11-30 14:46:43 +09:00
Tom Lane 51dfaa0b01 Remove bogus Assert and dead code in remove_useless_results_recurse().
The JOIN_SEMI case Assert'ed that there are no PlaceHolderVars that
need to be evaluated at the semijoin's RHS, which is wrong because
there could be some in the semijoin's qual condition.  However, there
could not be any references further up than that, and within the qual
there is not any way that such a PHV could have gone to null yet, so
we don't really need the PHV and there is no need to avoid making the
RHS-removal optimization.  The upshot is that there's no actual bug
in production code, and we ought to just remove this misguided Assert.

While we're here, also drop the JOIN_RIGHT case, which is dead code
because reduce_outer_joins() already got rid of JOIN_RIGHT.

Per bug #17700 from Xin Wen.  Uselessness of the JOIN_RIGHT case
pointed out by Richard Guo.  Back-patch to v12 where this code
was added.

Discussion: https://postgr.es/m/17700-2b5c10d917c30687@postgresql.org
2022-11-29 10:52:44 -05:00
Andrew Dunstan 4441fc704d Provide non-superuser predefined roles for vacuum and analyze
This provides two new predefined roles: pg_vacuum_all_tables and
pg_analyze_all_tables. Roles which have been granted these roles can
perform vacuum or analyse respectively on any or all tables as if they
were a superuser. This removes the need to grant superuser privilege to
roles just so they can perform vacuum and/or analyze.

Nathan Bossart

Reviewed by: Bharath Rupireddy, Kyotaro Horiguchi, Stephen Frost, Robert
Haas, Mark Dilger, Tom Lane, Corey Huinker, David G. Johnston, Michael
Paquier.

Discussion: https://postgr.es/m/20220722203735.GB3996698@nathanxps13
2022-11-28 12:08:14 -05:00
Andrew Dunstan b5d6382496 Provide per-table permissions for vacuum and analyze.
Currently a table can only be vacuumed or analyzed by its owner or
a superuser. This can now be extended to any user by means of an
appropriate GRANT.

Nathan Bossart

Reviewed by: Bharath Rupireddy, Kyotaro Horiguchi, Stephen Frost, Robert
Haas, Mark Dilger, Tom Lane, Corey Huinker, David G. Johnston, Michael
Paquier.

Discussion: https://postgr.es/m/20220722203735.GB3996698@nathanxps13
2022-11-28 12:08:14 -05:00
Michael Paquier a54b658ce7 Add support for file inclusions in HBA and ident configuration files
pg_hba.conf and pg_ident.conf gain support for three record keywords:
- "include", to include a file.
- "include_if_exists", to include a file, ignoring it if missing.
- "include_dir", to include a directory of files.  These are classified
by name (C locale, mostly) and need to be prefixed by ".conf", hence
following the same rules as GUCs.

This commit relies on the refactoring pieces done in efc9816, ad6c528,
783e8c6 and 1b73d0b, adding a small wrapper to build a list of
TokenizedAuthLines (tokenize_include_file), and the code is shaped to
offer some symmetry with what is done for GUCs with the same options.

pg_hba_file_rules and pg_ident_file_mappings gain a new field called
file_name, to track from which file a record is located, taking
advantage of the addition of rule_number in c591300 to offer an
organized view of the HBA or ident records loaded.

Bump catalog version.

Author: Julien Rouhaud
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud
2022-11-24 13:51:34 +09:00
Andrew Dunstan b7a5ef17cf Simplify WARNING messages from skipped vacuum/analyze on a table
This will more easily accomodate adding new permissions for vacuum and
analyze.

Nathan Bossart following a suggestion from Kyotaro Horiguchi

Discussion: https://postgr.es/m/20220726.104712.912995710251150228.horikyota.ntt@gmail.com
2022-11-23 14:43:16 -05:00
Tom Lane 56d0ed3b75 Give better hints for ambiguous or unreferenceable columns.
Examine ParseNamespaceItem flags to detect whether a column name
is unreferenceable for lack of LATERAL, or could be referenced if
a qualified name were used, and give better hints for such cases.
Also, don't phrase the message to imply that there's only one
matching column when there is really more than one.

Many of the regression test output changes are not very interesting,
but just reflect reclassifying the "There is a column ... but it
cannot be referenced from this part of the query" messages as DETAIL
rather than HINT.  They are details per our style guide, in the sense
of being factual rather than offering advice; and this change provides
room to offer actual HINTs about what to do.

While here, adjust the fuzzy-name-matching code to be a shade less
impenetrable.  It was overloading the meanings of FuzzyAttrMatchState
fields way too much IMO, so splitting them into multiple fields seems
to make it clearer.  It's not like we need to shave bytes in that
struct.

Per discussion of bug #17233 from Alexander Korolev.

Discussion: https://postgr.es/m/17233-afb9d806aaa64b17@postgresql.org
2022-11-22 18:46:31 -05:00
Alvaro Herrera 0538d4c0c3
Remove useless MERGE test
This was trying to exercise an ERROR we don't actually have.

Backpatch to 15.

Reported by Teja Mupparti <Tejeswar.Mupparti@microsoft.com>
Discussion: https://postgr.es/m/SN6PR2101MB1040BDAF740EA4389484E92BF0079@SN6PR2101MB1040.namprd21.prod.outlook.com
2022-11-22 11:26:47 +01:00
Michael Paquier f193883fc9 Replace SQLValueFunction by COERCE_SQL_SYNTAX
This switch impacts 9 patterns related to a SQL-mandated special syntax
for function calls:
- LOCALTIME [ ( typmod ) ]
- LOCALTIMESTAMP [ ( typmod ) ]
- CURRENT_TIME [ ( typmod ) ]
- CURRENT_TIMESTAMP [ ( typmod ) ]
- CURRENT_DATE

Five new entries are added to pg_proc to compensate the removal of
SQLValueFunction to provide backward-compatibility and making this
change transparent for the end-user (for example for the attribute
generated when a keyword is specified in a SELECT or in a FROM clause
without an alias, or when specifying something else than an Iconst to
the parser).

The parser included a set of checks coming from the files in charge of
holding the C functions used for the SQLValueFunction calls (as of
transformSQLValueFunction()), which are now moved within each function's
execution path, so this reduces the dependencies between the execution
and the parsing steps.  As of this change, all the SQL keywords use the
same paths for their work, relying only on COERCE_SQL_SYNTAX.  Like
fb32748, no performance difference has been noticed, while the perf
profiles get reduced with ExecEvalSQLValueFunction() gone.

Bump catalog version.

Reviewed-by: Corey Huinker, Ted Yu
Discussion: https://postgr.es/m/YzaG3MoryCguUOym@paquier.xyz
2022-11-21 18:31:59 +09:00
Tom Lane b62303794e Fix sloppy cleanup of roles in privileges.sql.
Commit 3d14e171e dropped regress_roleoption_donor twice and
regress_roleoption_protagonist not at all.  Leaving roles behind
after "make installcheck" is unfriendly in its own right, plus
it causes repeated runs of "make installcheck" to fail.
2022-11-20 11:30:50 -05:00
Robert Haas 3d14e171e9 Add a SET option to the GRANT command.
Similar to how the INHERIT option controls whether or not the
permissions of the granted role are automatically available to the
grantee, the new SET permission controls whether or not the grantee
may use the SET ROLE command to assume the privileges of the granted
role.

In addition, the new SET permission controls whether or not it
is possible to transfer ownership of objects to the target role
or to create new objects owned by the target role using commands
such as CREATE DATABASE .. OWNER. We could alternatively have made
this controlled by the INHERIT option, or allow it when either
option is given. An advantage of this approach is that if you
are granted a predefined role with INHERIT TRUE, SET FALSE, you
can't go and create objects owned by that role.

The underlying theory here is that the ability to create objects
as a target role is not a privilege per se, and thus does not
depend on whether you inherit the target role's privileges. However,
it's surely something you could do anyway if you could SET ROLE
to the target role, and thus making it contingent on whether you
have that ability is reasonable.

Design review by Nathan Bossat, Wolfgang Walther, Jeff Davis,
Peter Eisentraut, and Stephen Frost.

Discussion: http://postgr.es/m/CA+Tgmob+zDSRS6JXYrgq0NWdzCXuTNzT5eK54Dn2hhgt17nm8A@mail.gmail.com
2022-11-18 12:32:56 -05:00
Peter Eisentraut d8678aba2e Make object_address test output easier to update
The object_address test file turns to psql unaligned output for some
tests to avoid huge diffs for changes.  But this is useful also to the
other large test in that file, so apply it there as well.  This also
makes verifying the null and whitespace behavior easier.
2022-11-18 16:00:52 +01:00
Peter Eisentraut d0ca708540 Clean up SQL code indentation in test file
This makes the code layout more consistent inside the same file.
2022-11-18 15:31:55 +01:00
Tom Lane adaf34241a Improve ruleutils' printout of LATERAL references within subplans.
Commit 1cc29fe7c, which taught EXPLAIN to print PARAM_EXEC Params as
the referenced expressions, included some checks to prevent matching
Params found in SubPlans or InitPlans to NestLoopParams of upper query
levels.  At the time, this seemed possibly necessary to avoid false
matches because of the planner's habit of re-using the same PARAM_EXEC
slot in multiple places in a plan.  Furthermore, in the absence of
LATERAL no such reference could be valid anyway.  But it's possible
now that we have LATERAL, and in the wake of 46c508fbc and 1db5667ba
I believe the false-match hazard is gone.  Hence, remove the
in_same_plan_level checks.  As shown in the regression test changes,
this provides a useful improvement in readability for EXPLAIN of
LATERAL-using subplans.

Richard Guo, reviewed by Greg Stark and myself

Discussion: https://postgr.es/m/CAMbWs4-YSOcQXAagJetP95cAeZPqzOy5kM5yijG0PVW5ztRb4w@mail.gmail.com
2022-11-16 20:06:09 -05:00
Peter Eisentraut 90e4f308b4 Add missing object classes to object_address test
Per the comment, fill in classes mentioned in getObjectIdentityParts()
but not in the test.
2022-11-16 19:44:38 +01:00
Peter Eisentraut 5b66de3433 psql: Add command to use extended query protocol
This adds a new psql command \bind that sets query parameters and
causes the next query to be sent using the extended query protocol.
Example:

    SELECT $1, $2 \bind 'foo' 'bar' \g

This may be useful for psql scripting, but one of the main purposes is
also to be able to test various aspects of the extended query protocol
from psql and to write tests more easily.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/e8dd1cd5-0e04-3598-0518-a605159fe314@enterprisedb.com
2022-11-15 14:27:46 +01:00
Tom Lane b9424d014e Support writing "CREATE/ALTER TABLE ... SET STORAGE DEFAULT".
We already allow explicitly writing DEFAULT for SET COMPRESSION,
so it seems a bit inflexible and non-orthogonal to not have it
for STORAGE.

Aleksander Alekseev

Discussion: https://postgr.es/m/CAJ7c6TMX9ui+6y3TQFaXJYVpZyBukvqhQbVDJ8OUokeLRhtnpA@mail.gmail.com
2022-11-10 18:20:49 -05:00
Tom Lane b158e0b1b1 Fix alter_table.sql test case to test what it claims to.
The stanza "SET STORAGE may need to add a TOAST table" does not
test what it's supposed to, and hasn't done so since we added
the ability to store constant column default values as metadata.
We need to use a non-constant default to get the expected table
rewrite to actually happen.

Fix that, and add the missing checks that would have exposed the
problem to begin with.

Noted while reviewing a patch that made changes in this test case.
Back-patch to v11 where the problem came in.
2022-11-10 17:24:26 -05:00
Tom Lane 4f981df8e0 Report a more useful error for reloptions on a partitioned table.
Previously, trying to set storage parameters on a partitioned table
always led to "unrecognized parameter foo", because the code expected
there might be some valid parameters; but there aren't any.  The docs
make clear that it's intended that there never will be any, so let's
replace this useless search with a more to-the-point message.

Simon Riggs and Karina Litskevich

Discussion: https://postgr.es/m/CANbhV-H=eZ9kTR9mUgKGK0Qv9uXP=U+dQg3rinQHfTdFMhBA2A@mail.gmail.com
2022-11-09 12:28:34 -05:00
Michael Paquier bd95816f74 psql: Add information in \d+ about foreign partitions and child tables
\d+ is already able to show if a partition or a child table is
"PARTITIONED" via its relkind, hence the addition of a keyword for
"FOREIGN" in the relation description is basically free.

Author: Ian Lawrence Barwick
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CAB8KJ=iwzbEz2HR9EhNxQLVhMk2G_OYtQPJ9V=jWLadseggrOA@mail.gmail.com
2022-11-08 14:19:09 +09:00
Michael Paquier d7744d50a5 Fix initialization of pg_stat_get_lastscan()
A NULL result should be reported when a stats timestamp is set to 0, but
c037471 missed that, leading to a confusing timestamp value after for
example a DML on a freshly-created relation with no scans done on it
yet.

This impacted the following attributes for two system views:
- pg_stat_all_tables.last_idx_scan
- pg_stat_all_tables.last_seq_scan
- pg_stat_all_indexes.last_idx_scan

Reported-by: Robert Treat
Analyzed-by: Peter Eisentraut
Author: Dave Page
Discussion: https://postgr.es/m/CABV9wwPzMfSaz3EfKXXDxKmMprbxwF5r6WPuxqA=5mzRUqfTGg@mail.gmail.com
2022-11-08 10:50:09 +09:00
Tom Lane ff8fa0bf7e Handle SubPlan cases in find_nonnullable_rels/vars.
We can use some variants of SubPlan to deduce that Vars appearing
in the testexpr must be non-null.

Richard Guo

Discussion: https://postgr.es/m/CAMbWs4-jV=199A2Y_6==99dYnpnmaO_Wz_RGkRTTaCB=Pihw2w@mail.gmail.com
2022-11-05 15:24:36 -04:00
Andres Freund a5ac3e76fe meson: Split 'main' suite into 'regress' and 'isolation'
Several people didn't like the 'main' name and found it confusing that the
main regression and isolation tests were in one suite.

Author: Justin Pryzby <pryzbyj@telsasoft.com>
Discussion: https://postgr.es/m/20221001221514.2yy257v4zdfhwiy2@awork3.anarazel.de
Discussion: https://postgr.es/m/20221021123435.GU16921@telsasoft.com
2022-11-04 18:08:44 -07:00
Etsuro Fujita 8c71467908 Correct error message for row-level triggers with transition tables on partitioned tables.
"Triggers on partitioned tables cannot have transition tables." is
incorrect as we allow statement-level triggers on partitioned tables to
have transition tables.

This has been wrong since commit 86f575948; back-patch to v11 where that
commit came in.

Reviewed by Tom Lane.

Discussion: https://postgr.es/m/CAPmGK17gk4vXLzz2iG%2BG4LWRWCoVyam70nZ3OuGm1hMJwDrhcg%40mail.gmail.com
2022-11-04 19:15:00 +09:00
Alvaro Herrera b0284bfb1d
Create FKs properly when attaching table as partition
Commit f56f8f8da6 added some code in CloneFkReferencing that's way too
lax about a Constraint node it manufactures, not initializing enough
struct members -- initially_valid in particular was forgotten.  This
causes some FKs in partitions added by ALTER TABLE ATTACH PARTITION to
be marked as not validated.  Set initially_valid true, which fixes the
bug.

While at it, make the struct initialization more complete.  Very similar
code was added in two other places by the same commit; make them all
follow the same pattern for consistency, though no bugs are apparent
there.

This bug has never been reported: I only happened to notice while
working on commit 614a406b4f.  The test case that was added there with
the improper result is repaired.

Backpatch to 12.

Discussion: https://postgr.es/m/20221005105523.bhuhkdx4olajboof@alvherre.pgsql
2022-11-03 20:40:21 +01:00
Michael Paquier 451d1164b9 Add more tests for COPY with incorrect option combinations
Based on the existing coverage report, some combinations were not
checked at all, so add some tests to do so.  Spotted while looking at
the area.

Discussion: https://postgr.es/m/Y2DNm9u7hzIxCXHn@paquier.xyz
2022-11-02 09:57:54 +09:00
Tom Lane f4857082bc Fix planner failure with extended statistics on partitioned tables.
Some cases would result in "cache lookup failed for statistics object",
due to trying to fetch inherited statistics when only non-inherited
ones are available or vice versa.

Richard Guo and Justin Pryzby

Discussion: https://postgr.es/m/20221030170520.GM16921@telsasoft.com
2022-11-01 14:34:44 -04:00
Tom Lane 0043aa6b85 Add basic regression tests for semi/antijoin recognition.
Add some simple tests that the planner recognizes all the
standard idioms for SEMI and ANTI joins.  Failure to optimize
in this way won't necessarily cause any visible change in
query results, so check the plans.  We had no similar coverage
before, at least for some variants of antijoin, as noted by
Richard Guo.

Discussion: https://postgr.es/m/CAMbWs4-mvPPCJ1W6iK6dD5HiNwoJdi6mZp=-7mE8N9Sh+cd0tQ@mail.gmail.com
2022-10-31 19:52:33 -04:00
David Rowley 5543677ec9 Use Limit instead of Unique to implement DISTINCT, when possible
When all of the query's DISTINCT pathkeys have been marked as redundant
due to EquivalenceClasses existing which contain constants, we can just
implement the DISTINCT operation on a query by just limiting the number of
returned rows to 1 instead of performing a Unique on all of the matching
(duplicate) rows.

This applies in cases such as:

SELECT DISTINCT col,col2 FROM tab WHERE col = 1 AND col2 = 10;

If there are any matching rows, then they must all be {1,10}.  There's no
point in fetching all of those and running a Unique operator on them to
leave only a single row.  Here we effectively just find the first row and
then stop.  We are obviously unable to apply this optimization if either
the col = 1 or col2 = 10 were missing from the WHERE clause or if there
were any additional columns in the SELECT clause.

Such queries are probably not all that common, but detecting when we can
apply this optimization amounts to checking if the distinct_pathkeys are
NULL, which is very cheap indeed.

Nothing is done here to check if the query already has a LIMIT clause.  If
it does then the plan may end up with 2 Limits nodes.  There's no harm in
that and it's probably not worth the complexity to unify them into a
single Limit node.

Author: David Rowley
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/CAApHDvqS0j8RUWRUSgCAXxOqnYjHUXmKwspRj4GzVfOO25ByHA@mail.gmail.com
Discussion: https://postgr.es/m/MEYPR01MB7101CD5DA0A07C9DE2B74850A4239@MEYPR01MB7101.ausprd01.prod.outlook.com
2022-10-28 23:04:38 +13:00
Tom Lane a5fc46414d Avoid making commutatively-duplicate clauses in EquivalenceClasses.
When we decide we need to make a derived clause equating a.x and
b.y, we already will re-use a previously-made clause "a.x = b.y".
But we might instead have "b.y = a.x", which is perfectly usable
because equivclass.c has never promised anything about the
operand order in clauses it builds.  Saving construction of a
new RestrictInfo doesn't matter all that much in itself --- but
because we cache selectivity estimates and so on per-RestrictInfo,
there's a possibility of saving a fair amount of duplicative
effort downstream.

Hence, check for commutative matches as well as direct ones when
seeing if we have a pre-existing clause.  This changes the visible
clause order in several regression test cases, but they're all
clearly-insignificant changes.

Checking for the reverse operand order is simple enough, but
if we wanted to check for operator OID match we'd need to call
get_commutator here, which is not so cheap.  I concluded that
we don't really need the operator check anyway, so I just
removed it.  It's unlikely that an opfamily contains more than
one applicable operator for a given pair of operand datatypes;
and if it does they had better give the same answers, so there
seems little need to insist that we use exactly the one
select_equality_operator chose.

Using the current core regression suite as a test case, I see
this change reducing the number of new join clauses built by
create_join_clause from 9673 to 5142 (out of 26652 calls).
So not quite 50% savings, but pretty close to it.

Discussion: https://postgr.es/m/78062.1666735746@sss.pgh.pa.us
2022-10-27 14:42:18 -04:00
Michael Paquier 1b9cd69c5b Add some tests to check the SQL functions of control file
As the recent commit 05d4cbf (reverted after as a448e49) has proved,
there is zero coverage for the four SQL functions that can scan the
control file data:
- pg_control_checkpoint()
- pg_control_init()
- pg_control_recovery()
- pg_control_system()

This commit adds a minimal coverage for these functions, checking that
their execution is able to complete.  This would have been enough to
catch the problems introduced in the commit mentioned above.  More
checks could be done for each individual fields, but it is unclear
whether this would be better than the other checks in place in the
backend code.

Per discussion with Bharath Rupireddy.

Discussion: https://postgr.es/m/Y1d2FZmQmyAhPSRG@paquier.xyz
2022-10-27 09:58:44 +09:00
Michael Paquier c591300a8f Add rule_number to pg_hba_file_rules and map_number to pg_ident_file_mappings
These numbers are strictly-monotone identifiers assigned to each rule
of pg_hba_file_rules and each map of pg_ident_file_mappings when loading
the HBA and ident configuration files, indicating the order in which
they are checked at authentication time, until a match is found.

With only one file loaded currently, this is equivalent to the line
numbers assigned to the entries loaded if one wants to know their order,
but this becomes mandatory once the inclusion of external files is
added to the HBA and ident files to be able to know in which order the
rules and/or maps are applied at authentication.  Note that NULL is used
when a HBA or ident entry cannot be parsed or validated, aka when an
error exists, contrary to the line number.

Bump catalog version.

Author: Julien Rouhaud
Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud
2022-10-26 15:22:15 +09:00
Michael Paquier 2e0d80c5bb Improve coverage of ruleutils.c for SQLValueFunctions
While looking at how these are handled in the parser and the executor, I
have noticed that there is no test coverage for most of these when
reverse-engineering an expression for a SQLValueFunction node in
ruleutils.c, including how these are reparsed when included in a FROM
clause.  Some hacking in this area has showed me that these could break
easily, so add some coverage to track the existing compatibility.

Extracted from a much larger patch by me.

Discussion: https://postgr.es/m/YzaG3MoryCguUOym@paquier.xyz
2022-10-24 16:53:54 +09:00
Dean Rasheed 40c7fcbbed Improve the accuracy of numeric power() for integer exponents.
This makes the choice of result scale of numeric power() for integer
exponents consistent with the choice for non-integer exponents, and
with the result scale of other numeric functions. Specifically, the
result scale will be at least as large as the scale of either input,
and sufficient to ensure that the result has at least 16 significant
digits.

Formerly, the result scale was based only on the scale of the first
input, without taking into account the weight of the result. For
results with negative weight, that could lead to results with very few
or even no non-zero significant digits (e.g., 10.0 ^ (-18) produced
0.0000000000000000).

Fix this by moving responsibility for the choice of result scale into
power_var_int(), which already has code to estimate the result weight.

Per report by Adrian Klaver and suggested fix by Tom Lane.

No back-patch -- arguably this is a bug fix, but one which is easy to
work around, so it doesn't seem worth the risk of changing query
results in stable branches.

Discussion: https://postgr.es/m/12a40226-70ac-3a3b-3d3a-fdaf9e32d312%40aklaver.com
2022-10-20 10:10:17 +01:00
Tom Lane 8272749e8c Record dependencies of a cast on other casts that it requires.
When creating a cast that uses a conversion function, we've
historically allowed the input and result types to be
binary-compatible with the function's input and result types,
rather than necessarily being identical.  This means that the new
cast is logically dependent on the binary-compatible cast or casts
that it references: if those are defined by pg_cast entries, and you
try to restore the new cast without having defined them, it'll fail.
Hence, we should make pg_depend entries to record these dependencies
so that pg_dump knows that there is an ordering requirement.

This is not the only place where we allow such shortcuts; aggregate
functions for example are similarly lax, and in principle should gain
similar dependencies.  However, for now it seems sufficient to fix
the cast-versus-cast case, as pg_dump's other ordering heuristics
should keep it out of trouble for other object types.

Per report from David Turoň; thanks also to Robert Haas for
preliminary investigation.  I considered back-patching, but
seeing that this issue has existed for many years without
previous reports, it's not clear it's worth the trouble.
Moreover, back-patching wouldn't be enough to ensure that the
new pg_depend entries exist in existing databases anyway.

Discussion: https://postgr.es/m/OF0A160F3E.578B15D1-ONC12588DA.003E4857-C12588DA.0045A428@notes.linuxbox.cz
2022-10-17 14:02:05 -04:00
Tom Lane d57534740b Fix EXPLAIN of SEARCH BREADTH FIRST with a constant initial value.
If the non-recursive term of a SEARCH BREADTH FIRST recursive
query has only constants in its target list, the planner will
fold the starting RowExpr added by rewrite into a simple Const
of type RECORD.  The executor doesn't have any problem with
that --- but EXPLAIN VERBOSE will encounter the Const as the
ultimate source of truth about what the field names of the
SET column are, and it didn't know what to do with that.
Fortunately, we can pull the identifying typmod out of the
Const, in much the same way that record_out would.

For reasons that remain a bit obscure to me, this only fails
with SEARCH BREADTH FIRST, not SEARCH DEPTH FIRST or CYCLE.
But I added regression test cases for both of those options
too, just to make sure we don't break it in future.

Per bug #17644 from Matthijs van der Vleuten.  Back-patch
to v14 where these constructs were added.

Discussion: https://postgr.es/m/17644-3bd1f3036d6d7a16@postgresql.org
2022-10-16 19:18:08 -04:00
Andres Freund c037471832 pgstat: Track time of the last scan of a relation
It can be useful to know when a relation has last been used, e.g., when
evaluating whether an index is still required. It was already possible to
infer the time of the last usage by tracking, e.g.,
pg_stat_all_indexes.idx_scan over time. But far from everybody does so.

To make it easier to detect the last time a relation has been scanned, track
that time in each relation's pgstat entry. To minimize overhead a) the
timestamp is updated only when the backend pending stats entry is flushed to
shared stats b) the last transaction's stop timestamp is used as the
timestamp.

Bumps catalog and stats format versions.

Author: Dave Page <dpage@pgadmin.org>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Bruce Momjian <bruce@momjian.us>
Reviewed-by: Vik Fearing <vik@postgresfriends.org>
Discussion: https://postgr.es/m/CA+OCxozrVHNFVEPkweUHMZje+t1tfY816d9MZYc6eZwOOusOaQ@mail.gmail.com
2022-10-14 11:11:34 -07:00
Peter Eisentraut 9786b89bd1 Put tests of md5() function into separate test file
In FIPS mode, these calls will fail.  By having them in a separate
file, it would make it easier to have an alternative output file or
selectively disable these tests.  This isn't done here; this is just
some preparation.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/647f6cc1-473d-f788-ade0-c09201e5ab6a@enterprisedb.com
2022-10-13 12:02:31 +02:00
Amit Kapila 5263c6b095 Improve the WARNING message for CREATE SUBSCRIPTION.
Author: Peter Smith
Reviewed-By: Alvaro Herrera, Tom Lane, Amit Kapila
Discussion: https://postgr.es/m/CAHut+PvqdqOanheWSHDyhQiF+Z-7w=-+k4U+bwbT=b6YQ_hrXQ@mail.gmail.com
2022-10-13 06:09:43 +05:30
Tom Lane b8f2687fdc Yet further fixes for multi-row VALUES lists for updatable views.
DEFAULT markers appearing in an INSERT on an updatable view
could be mis-processed if they were in a multi-row VALUES clause.
This would lead to strange errors such as "cache lookup failed
for type NNNN", or in older branches even to crashes.

The cause is that commit 41531e42d tried to re-use rewriteValuesRTE()
to remove any SetToDefault nodes (that hadn't previously been replaced
by the view's own default values) appearing in "product" queries,
that is DO ALSO queries.  That's fundamentally wrong because the
DO ALSO queries might not even be INSERTs; and even if they are,
their targetlists don't necessarily match the view's column list,
so that almost all the logic in rewriteValuesRTE() is inapplicable.

What we want is a narrow focus on replacing any such nodes with NULL
constants.  (That is, in this context we are interpreting the defaults
as being strictly those of the view itself; and we already replaced
any that aren't NULL.)  We could add still more !force_nulls tests
to further lobotomize rewriteValuesRTE(); but it seems cleaner to
split out this case to a new function, restoring rewriteValuesRTE()
to the charter it had before.

Per bug #17633 from jiye_sw.  Patch by me, but thanks to
Richard Guo and Japin Li for initial investigation.
Back-patch to all supported branches, as the previous fix was.

Discussion: https://postgr.es/m/17633-98cc85e1fa91e905@postgresql.org
2022-10-11 18:24:14 -04:00