Commit Graph

54976 Commits

Author SHA1 Message Date
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
Andres Freund ab72a31f6c meson: Mark PROVE as not required
In the meson build the prove binary is currently not even used. It will soon
be, for PGXS compatibility, but even then we should build without it around.

Author: Justin Pryzby <pryzbyj@telsasoft.com>
Discussion: https://postgr.es/m/20221021034040.GT16921@telsasoft.com
Discussion: https://postgr.es/m/20221104235412.GE16921@telsasoft.com
2022-11-04 18:08:05 -07:00
Tom Lane 34fa0ddae5 Fix CREATE DATABASE so we can pg_upgrade DBs with OIDs above 2^31.
Commit aa0105141 repeated one of the oldest mistakes in our book:
thinking that OID is the same as int32.  It isn't of course, and
unsurprisingly the first person who came along with a database
OID above 2 billion broke it.  Repair.

Per bug #17677 from Sergey Pankov.  Back-patch to v15.

Discussion: https://postgr.es/m/17677-a99fa067d7ed71c9@postgresql.org
2022-11-04 10:39:52 -04: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
Amit Kapila 3e77b2eb34 Doc: Improve the description of confirmed_flush_lsn in pg_replication_slots.
Make it clear that the data corresponding to the transactions committed
before confirmed_flush_lsn is not available anymore.

Author: Ashutosh Sharma
Reviewd by: Ashutosh Bapat
Discussion: https://postgr.es/m/CAE9k0P=hiqRXUonnmtS-5Pu8SbO=yF6vcrVBcfEf2+93ng_f5Q@mail.gmail.com
2022-11-04 09:00:46 +05:30
John Naylor 233cf6e8ad Remove outdated include
In the wake of bfb9dfd93, there are no longer any stat() calls in
guc-file.l, but the work leading to dac048f71 did not get the memo.

Noted by Michael Paquier

Discussion: https://www.postgresql.org/message-id/Y2OosGi1Xh9x/lEn%40paquier.xyz
2022-11-04 07:50:57 +07: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
Peter Eisentraut 2fe4c7384f Make AssertPointerAlignment available to frontend code
We don't need separate definitions for frontend and backend, since the
contained Assert() will take care of the difference.  So this also
makes it simpler overall.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/f64365b1-d5f9-ef83-41fe-404810f10e5a@enterprisedb.com
2022-11-03 12:04:22 -04:00
Tom Lane dea8349380 Avoid crash after function syntax error in a replication worker.
If a syntax error occurred in a SQL-language or PL/pgSQL-language
CREATE FUNCTION or DO command executed in a logical replication worker,
we'd suffer a null pointer dereference or assertion failure.  That
seems like a rather contrived case, but nonetheless worth fixing.

The cause is that function_parse_error_transpose assumes it must be
executing within the context of a Portal, but logical/worker.c
doesn't create a Portal since it's not running the standard executor.
We can just back off the hard Assert check and make it fail gracefully
if there's not an ActivePortal.  (I have a feeling that the aggressive
check here was my fault originally, probably because I wasn't sure if
the case would always hold and wanted to find out.  Well, now we know.)

The hazard seems to exist in all branches that have logical replication,
so back-patch to v10.

Maxim Orlov, Anton Melnikov, Masahiko Sawada, Tom Lane

Discussion: https://postgr.es/m/b570c367-ba38-95f3-f62d-5f59b9808226@inbox.ru
Discussion: https://postgr.es/m/adf0452f-8c6b-7def-d35e-ab516c80088e@inbox.ru
2022-11-03 12:02:14 -04:00
Alvaro Herrera 5fca91025e
Resolve partition strategy during early parsing
This has little practical value, but there's no reason to let the
partition strategy names travel through DDL as strings.

Reviewed-by: Japin Li <japinli@hotmail.com>
Discussion: https://postgr.es/m/20221021093216.ffupd7epy2mytkux@alvherre.pgsql
2022-11-03 16:25:54 +01:00
Tom Lane cf8b7d374a Add casts to simplehash.h to silence C++ warnings.
Casting the result of palloc etc. to the intended type is more per
project style anyway.

(The fact that cpluspluscheck doesn't notice these problems is
because it doesn't expand any macros, which seems like a troubling
shortcoming.  Don't have a good idea about improving that.)

Back-patch to v13, which is as far as the patch applies cleanly;
doesn't seem worth working harder.

David Geier

Discussion: https://postgr.es/m/aa5d88a3-71f4-3455-11cf-82de0372c941@gmail.com
2022-11-03 10:47:31 -04:00
John Naylor 062eef3a9b Straighten include order in guc-file.l
Oversight in dac048f71e

Michael Paquier

Reviewed by Julien Rouhaud
Discussion: https://www.postgresql.org/message-id/Y2IATvRGo347Lvd1%40paquier.xyz
2022-11-03 12:38:44 +07:00
Tom Lane 1c72d82c25 Allow use of __sync_lock_test_and_set for spinlocks on any machine.
If we have no special-case code in s_lock.h for the current platform,
but the compiler has __sync_lock_test_and_set, use that instead of
failing.  It's unlikely that anybody's __sync_lock_test_and_set
would be so awful as to be worse than our semaphore-based fallback,
but if it is, they can (continue to) use --disable-spinlocks.

This allows removal of the RISC-V special case installed by commit
c32fcac56, which generated exactly the same code but only on that
platform.  Usefully, the RISC-V buildfarm animals should now test
at least the int variant of this patch.

I've manually tested both variants on ARM by dint of removing the
ARM-specific stanza.  We don't want to drop that, because it already
has some special knowledge and is likely to grow more over time.
Likewise, this is not meant to preclude installing special cases
for other arches if that proves worthwhile.

Per discussion of a request to install the same code for loongarch64.
Like the previous patch, we might as well back-patch to supported
branches.

Discussion: https://postgr.es/m/761ac43d44b84d679ba803c2bd947cc0@HSMAILSVR04.hs.handsome.com.cn
2022-11-02 17:37:29 -04:00
Peter Eisentraut 3655b46aa0 pg_dump: Refactor code that constructs ALTER ... OWNER TO commands
Avoid having to list all the possible object types twice.  Instead,
only _getObjectDescription() needs to know about specific object
types.  It communicates back to _printTocEntry() whether an owner is
to be set.

In passing, remove the logic to use ALTER TABLE to set the owner of
views and sequences.  This is no longer necessary.  Furthermore, if
pg_dump doesn't recognize the object type, this is now a fatal error,
not a warning.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/0a00f923-599a-381b-923f-0d802a727715@enterprisedb.com
2022-11-02 17:24:38 -04:00
Tom Lane be541efbfd Defend against unsupported partition relkind in logical replication worker.
Since partitions can be foreign tables not only plain tables, but
logical replication only supports plain tables, we'd better check the
relkind of a partition after we find it.  (There was some discussion
of checking this when adding a partitioned table to a subscription;
but that would be inadequate since the troublesome partition could be
added later.)  Without this, the situation leads to a segfault or
assertion failure.

In passing, add a separate variable for the target Relation of
a cross-partition UPDATE; reusing partrel seemed mighty confusing
and error-prone.

Shi Yu and Tom Lane, per report from Ilya Gladyshev.  Back-patch
to v13 where logical replication into partitioned tables became
a thing.

Discussion: https://postgr.es/m/6b93e3748ba43298694f376ca8797279d7945e29.camel@gmail.com
2022-11-02 12:29:39 -04:00
Tom Lane 26ee7fb368 pg_dump: fix failure to dump comments on constraints in some cases.
Thinko in commit 5209c0ba0: I checked the wrong object's
DUMP_COMPONENT_COMMENT bit in two places.

Per bug #17675 from Franz-Josef Färber.

Discussion: https://postgr.es/m/17675-c69c001e06390867@postgresql.org
2022-11-02 11:30:04 -04:00
Etsuro Fujita d54e79ba28 Fix copy-and-pasteo in comment. 2022-11-02 18:15:00 +09:00
Amit Kapila 155601de7f Doc: Update information about manually creating slots.
There are some cases (e.g. when the subscription is created using the
connect = false option) where the remote replication slot was not created
automatically and the user must create it manually before the subscription
can be activated. There was not enough information in the docs for users
to do this easily.

Author: Peter Smith
Reviewd by: Shi yu, Amit Kapila
Discussion: https://postgr.es/m/CAHut+PvqdqOanheWSHDyhQiF+Z-7w=-+k4U+bwbT=b6YQ_hrXQ@mail.gmail.com
2022-11-02 11:40:37 +05:30
Amit Kapila 568546f7e4 Improve the description of XLOG_RUNNING_XACTS.
Previously, the description of XLOG_RUNNING_XACTS showed only
top-transaction XIDs and whether subtransactions overflowed. This commit
improves it to show individual subtransaction XIDs. This also improves the
description of overflowed subtransactions.

This additional information can be helpful for testing and debugging
purposes.

Author: Masahiko Sawada
Reviewd by: Fujii Masao, Kyotaro Horiguchi, Ashutosh Bapat, Bharath Rupireddy
Discussion: https://postgr.es/m/CAD21AoAqvaE+XEeXHHPdAGQPcCoGXxuoeutq_nWhUSQvTt5+tA@mail.gmail.com
2022-11-02 10:06:55 +05:30
Michael Paquier e76502871e doc: Fix some descriptions related to pg_ident_file_mappings
pg_ident_file_mappings.line_number was described as a line number in
pg_ident.conf for a "rule" number, but this should refer to a "map".
The same inconsistent term was used in the main paragraph describing the
view.

Extracted from a patch by the same author.  Issue introduced by
a2c8499 where this view has been added.

Author: Julien Rouhaud
Discussion: https://postgr.es/m/20221026031948.cbrnzgy5e7glsq2d@jrouhaud
Backpatch-through: 15
2022-11-02 11:56:06 +09:00
David Rowley 3712e0ed47 Fix outdated comment in tuplesort.h
This was outdated by 77bae396d.

Backpatch-through: 15, where 77bae396d was added
2022-11-02 15:29:31 +13:00
Michael Paquier 8e621c10c7 Remove code handling FORCE_NULL and FORCE_NOT_NULL for COPY TO
These two options are only available with COPY FROM, so the extra logic
in charge of checking the validity of the attributes given has no
purpose.

Author: Zhang Mingli
Reviewed-by: Richard Guo, Kyotaro Horiguchi
Discussion: https://postgr.es/m/F28F0B5A-766F-4D33-BF44-43B3A052D833@gmail.com
2022-11-02 10:15:19 +09:00
David Rowley 7c335b7a20 Add doubly linked count list implementation
We have various requirements when using a dlist_head to keep track of the
number of items in the list.  This, traditionally, has been done by
maintaining a counter variable in the calling code.  Here we tidy this up
by adding "dclist", which is very similar to dlist but also keeps track of
the number of items stored in the list.

Callers may use the new dclist_count() function when they need to know how
many items are stored. Obtaining the count is an O(1) operation.

For simplicity reasons, dclist and dlist both use dlist_node as their node
type and dlist_iter/dlist_mutable_iter as their iterator type. dclists
have all of the same functionality as dlists except there is no function
named dclist_delete().  To remove an item from a list dclist_delete_from()
must be used.  This requires knowing which dclist the given item is stored
in.

Additionally, here we also convert some dlists where additional code
exists to keep track of the number of items stored and to make these use
dclists instead.

Author: David Rowley
Reviewed-by: Bharath Rupireddy, Aleksander Alekseev
Discussion: https://postgr.es/m/CAApHDvrtVxr+FXEX0VbViCFKDGxA3tWDgw9oFewNXCJMmwLjLg@mail.gmail.com
2022-11-02 14:06:05 +13: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 e7c7605a76 Update time zone data files to tzdata release 2022f.
DST law changes in Chile, Fiji, Iran, Jordan, Mexico, Palestine,
and Syria.  Historical corrections for Chile, Crimea, Iran, and
Mexico.

Also, the Europe/Kiev zone has been renamed to Europe/Kyiv
(retaining the old name as a link).

The following zones have been merged into nearby, more-populous zones
whose clocks have agreed since 1970: Antarctica/Vostok, Asia/Brunei,
Asia/Kuala_Lumpur, Atlantic/Reykjavik, Europe/Amsterdam,
Europe/Copenhagen, Europe/Luxembourg, Europe/Monaco, Europe/Oslo,
Europe/Stockholm, Indian/Christmas, Indian/Cocos, Indian/Kerguelen,
Indian/Mahe, Indian/Reunion, Pacific/Chuuk, Pacific/Funafuti,
Pacific/Majuro, Pacific/Pohnpei, Pacific/Wake and Pacific/Wallis.
(This indirectly affects zones that were already links to one of
these: Arctic/Longyearbyen, Atlantic/Jan_Mayen, Iceland,
Pacific/Ponape, Pacific/Truk, and Pacific/Yap.)  America/Nipigon,
America/Rainy_River, America/Thunder_Bay, Europe/Uzhgorod, and
Europe/Zaporozhye were also merged into nearby zones after discovering
that their claimed post-1970 differences from those zones seem to have
been errors.

While the IANA crew have been working on merging zones that have no
post-1970 differences for some time, this batch of changes affects
some zones that are significantly more populous than those merged
in the past, notably parts of Europe.  The loss of pre-1970 timezone
history for those zones may be troublesome for applications
expecting consistency of timestamptz display.  As an example, the
stored value '1944-06-01 12:00 UTC' would previously display as
'1944-06-01 13:00:00+01' if the Europe/Stockholm zone is selected,
but now it will read out as '1944-06-01 14:00:00+02'.

There exists a "packrat" option that will build the timezone data
files with this old data preserved, but the problem is that it also
resurrects a bunch of other, far less well-attested data; so much so
that actually more zones' contents change from 2022a with that option
than without it.  I have chosen not to do that here, for that reason
and because it appears that no major OS distributions are using the
"packrat" option, so that doing so would cause Postgres' behavior
to diverge significantly depending on whether it was built with
--with-system-tzdata.  However, for anyone for whom these changes pose
significant problems, there is a solution: build a set of timezone
files with the "packrat" option and use those with Postgres.
2022-11-01 17:08:28 -04: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 495e73c207 pg_stat_statements: fetch stmt location/length before it disappears.
When executing a utility statement, we must fetch everything
we need out of the PlannedStmt data structure before calling
standard_ProcessUtility.  In certain cases (possibly only ROLLBACK
in extended query protocol), that data structure will get freed
during command execution.  The situation is probably often harmless
in production builds, but in debug builds we intentionally overwrite
the freed memory with garbage, leading to picking up garbage values
of statement location and length, typically causing an assertion
failure later in pg_stat_statements.  In non-debug builds, if
something did go wrong it would likely lead to storing garbage
for the query string.

Report and fix by zhaoqigui (with cosmetic adjustments by me).
It's an old problem, so back-patch to all supported versions.

Discussion: https://postgr.es/m/17663-a344fd0675f92128@postgresql.org
Discussion: https://postgr.es/m/1667307420050.56657@hundsun.com
2022-11-01 12:48:01 -04:00
Peter Eisentraut 84387fc889 doc: Add note about lack of publication privileges
This gives some additional advice on using row filters and column
lists on publications securely.

Author: Antonin Houska <ah@cybertec.at>
Reviewed-by: Euler Taveira <euler@eulerto.com>
Discussion: https://www.postgresql.org/message-id/flat/20330.1652105397@antos
2022-11-01 14:19:41 +01:00
Peter Eisentraut 2ea5de296e psql: Improve tab completion for ALTER TABLE on identity columns
- Add tab completion for ALTER SEQUENCE … START …
- Add tab completion for ALTER COLUMN … SET GENERATED …
- Add tab completion for ALTER COLUMN … SET <sequence option>
- Add tab completion for ALTER COLUMN … ADD GENERATED … AS IDENTITY

Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Reviewed-by: Matheus Alcantara <mths.dev@pm.me>
Discussion: https://www.postgresql.org/message-id/flat/87mta1jfax.fsf@wibble.ilmari.org
2022-11-01 12:07:40 +01: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
Jeff Davis 0717f2fedb Fix ALTER COLLATION "default" REFRESH VERSION.
Issue a helpful error message rather than an internal error.

Discussion: https://postgr.es/m/51fb77507cafd43fc1a2e733c23045873d93ae60.camel%40j-davis.com
Reviewed-by: Thomas Munro
2022-10-31 15:44:52 -07:00
Jeff Davis 10932ed5e5 Enable pg_collation_actual_version() to work on the default collation.
Previously, it would simply return NULL, which was less useful.

Discussion: https://postgr.es/m/51fb77507cafd43fc1a2e733c23045873d93ae60.camel%40j-davis.com
Reviewed-by: Thomas Munro
2022-10-31 15:43:23 -07:00
Peter Eisentraut 6e10631d1e pg_dump test: Make concatenated create_sql commands more readable
When the pg_dump 002_pg_dump.pl test generates the command to load the
schema, it does

    # Add terminating semicolon
    $create_sql{$test_db} .= $tests{$test}->{create_sql} . ";";

In some cases, this creates a duplicate semicolon, but more
importantly, this doesn't add any newline.  So if you look at the
result in either the server log or in
tmp_check/log/regress_log_002_pg_dump, it looks like a complete mess.
This patch makes the output look cleaner for manual inspection: add
semicolon only if necessary, and add two newlines.

Discussion: https://www.postgresql.org/message-id/flat/d6aec95a-8729-43cc-2578-f2a5e46640e0%40enterprisedb.com
2022-10-31 14:19:08 +01:00
Michael Paquier a73952b795 Add check on initial and boot values when loading GUCs
This commit adds a function to perform a cross-check between the initial
value of the C declaration associated to a GUC and its actual boot
value in assert-enabled builds.  The purpose of this is to prevent
anybody reading these C declarations from being fooled by mismatched
values before they are loaded at program startup.

The following rules apply depending on the GUC type:
* bool - can be false, or same as boot_val.
* int - can be 0, or same as the boot_val.
* real - can be 0.0, or same as the boot_val.
* string - can be NULL, or strcmp'd equal to the boot_val.
* enum - equal to the boot_val.

This is done for the system as well custom GUCs loaded by external
modules, which may require extension developers to adapt the C
declaration of the variables used by these GUCs (testing this change
with some of my own modules has allowed me to catch some stupid typos,
FWIW).  This may finish by being a bad experiment depending on the
feedbcak received, but let's see how it goes.

Author: Peter Smith
Reviewed-by: Nathan Bossart, Tom Lane, Michael Paquier, Justin Pryzby
Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com
2022-10-31 13:54:23 +09:00
Michael Paquier d9d873bac6 Clean up some inconsistencies with GUC declarations
This is similar to 7d25958, and this commit takes care of all the
remaining inconsistencies between the initial value used in the C
variable associated to a GUC and its default value stored in the GUC
tables (as of pg_settings.boot_val).

Some of the initial values of the GUCs updated rely on a compile-time
default.  These are refactored so as the GUC table and its C declaration
use the same values.  This makes everything consistent with other
places, backend_flush_after, bgwriter_flush_after, port,
checkpoint_flush_after doing so already, for example.

Extracted from a larger patch by Peter Smith.  The spots updated in the
modules are from me.

Author: Peter Smith, Michael Paquier
Reviewed-by: Nathan Bossart, Tom Lane, Justin Pryzby
Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com
2022-10-31 12:44:48 +09:00
Noah Misch a9f8ca6005 Under has_wal_read_bug, skip recovery/t/032_relfilenode_reuse.pl.
Per buildfarm member kittiwake.  Back-patch to v15, where this test
first appeared.

Discussion: https://postgr.es/m/20220116210241.GC756210@rfd.leadboat.com
2022-10-29 10:42:16 -07: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
Peter Eisentraut b1099eca8f Remove AssertArg and AssertState
These don't offer anything over plain Assert, and their usage had
already been declared obsolescent.

Author: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/20221009210148.GA900071@nathanxps13
2022-10-28 09:19:06 +02:00
David Rowley d37aa3d358 Allow nodeSort to perform Datum sorts for byref types
Here we add a new 'copy' parameter to tuplesort_getdatum so that we can
instruct the function not to datumCopy() byref Datums before returning.

Similar to 91e9e89dc, this can provide significant performance
improvements in nodeSort when sorting by a single byref column and the
sort's targetlist contains only that column.

This allows us to re-enable Datum sorts for byref types which was disabled
in 3a5817695 due to a reported memory leak.

Additionally, here we slightly optimize DISTINCT aggregates so that we no
longer perform any datumCopy() when we find the current value not to be
distinct from the previous value.  Previously the code would always take a
copy of the most recent Datum and pfree the previous value, even when the
values were the same.  Testing shows a small but noticeable performance
increase when aggregate transitions are skipped due to the current
transition value being the same as the prior one.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvqS6wC5U==k9Hd26E4EQXH3QR67-T4=Q1rQ36NGvjfVSg@mail.gmail.com
Discussion: https://postgr.es/m/CAApHDvqHonfe9G1cVaKeHbDx70R_zCrM3qP2AGXpGrieSKGnhA@mail.gmail.com
2022-10-28 09:25:12 +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 4ab8c81bd9 Move pg_pwritev_with_retry() to src/common/file_utils.c
This commit moves pg_pwritev_with_retry(), a convenience wrapper of
pg_writev() able to handle partial writes, to common/file_utils.c so
that the frontend code is able to use it.  A first use-case targetted
for this routine is pg_basebackup and pg_receivewal, for the
zero-padding of a newly-initialized WAL segment.  This is used currently
in the backend when the GUC wal_init_zero is enabled (default).

Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart, Thomas Munro
Discussion: https://postgr.es/m/CALj2ACUq7nAb7=bJNbK3yYmp-SZhJcXFR_pLk8un6XgDzDF3OA@mail.gmail.com
2022-10-27 14:39:42 +09: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 37d264478a Fix variable assignment thinko in hba.c
The intention behind 1b73d0b was to limit the use of TokenizedAuthLine,
but I have fat-fingered one location in parse_hba_line() when creating
the HbaLine, where this should use the local variable and not the value
coming from TokenizedAuthLine.  This logic is the exactly the same, but
let's be clean about all that on consistency grounds.

Reported-by: Julien Rouhaud
Discussion: https://postgr.es/m/20221026032730.k3sib5krgm7l6njk@jrouhaud
2022-10-26 12:57:40 +09:00
Michael Paquier 1b73d0b1c3 Refactor code handling the names of files loaded in hba.c
This has the advantage to limit the presence of the GUC values
hba_file and ident_file to the code paths where these files are loaded,
easing the introduction of an upcoming feature aimed at adding inclusion
logic for files and directories in HBA and ident files.

Note that this needs the addition of the source file name to HbaLine, in
addition to the line number, which is something needed by the backend in
two places of auth.c (authentication failure details and auth_id log
when log_connections is enabled).

While on it, adjust a log generated on authentication failure to report
the name of the actual HBA file on which the connection attempt matched,
where the line number and the raw line written in the HBA file were
already included.  This was previously hardcoded as pg_hba.conf, which
would be incorrect when a custom value is used at postmaster startup for
the GUC hba_file.

Extracted from a larger patch by the same author.

Author: Julien Rouhaud
Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud
2022-10-26 11:42:13 +09:00
Tom Lane 13d53aa7a8 Doc/improve confusing, inefficient tests to locate CTID variable.
The IsCTIDVar() tests in nodeTidscan.c and nodeTidrangescan.c
look buggy at first sight: they aren't checking that the varno
matches the table to be scanned.  Actually they're safe because
any Var in a scan-level qual must be for the correct table ...
but if we're depending on that, it's pretty pointless to verify
varlevelsup.  (Besides which, varlevelsup is *always* zero at
execution, since we've flattened the rangetable long since.)

Remove the useless varlevelsup check, and instead add some
commentary explaining why we don't need to check varno.

Noted while fooling with a planner change that causes the order
of "t1.ctid = t2.ctid" to change in some tidscan.sql tests;
I was briefly fooled into thinking there was a live bug here.
2022-10-25 17:35:19 -04:00
Heikki Linnakangas 0e972f50fd Update outdated comment for TransactionIdSetTreeStatus
Commit 06da3c570f changed the way subtransactions are marked as
SUBCOMMITTED, but the example it included actually documented the old
way. Update it.

Author: Japin Li
Discussion: https://www.postgresql.org/message-id/MEYP282MB16690BC96DFBE08CC857E1E3B6319%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
2022-10-25 21:43:52 +02:00
Michael Paquier 7d25958453 Clean up some GUC declarations and comments
This adjusts a few things for GUCs related to logical replication,
replication slots and WAL senders, in the shape of incorrect comments
and values inconsistent with their initial default value.

Author: Peter Smith
Reviewed-by: Nathan Bossart, Tom Lane, Justin Pryzby
Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com
2022-10-25 14:06:07 +09:00
Thomas Munro e109e43921 Fix unlink() for STATUS_DELETE_PENDING on Windows.
Commit f357233c assumed that it was OK to return ENOENT directly if
lstat() failed that way.  If we got STATUS_DELETE_PENDING while trying
to unlink a file that we had already unlinked successfully once before
but someone else still had open (on a kernel version that has "pending"
unlinks by default), then we would no longer reach the retry loop in
pgunlink().  That loop claims to be only for handling sharing violations
(a different phenomenon), but the errno is the same.

Restore that behavior with an explicit check, to see if it fixes the
occasional 'directory not empty' failures seen in the pg_upgrade tests
on CI.  Further improvements are possible with proposed upgrades to
modern Windows APIs that would replace this convoluted code.

Reported-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/20220920013122.GA31833%40telsasoft.com
Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com
2022-10-25 16:26:58 +13:00
Thomas Munro 4517358ee7 Fix stat() for recursive junction points on Windows.
Commit c5cb8f3b supposed that we'd only ever have to follow one junction
point in stat(), because we don't construct longer chains of them ourselves.
When examining a parent directory supplied by the user, we should really be
able to cope with longer chains, just in case someone has their system
set up that way.  Choose an arbitrary cap of 8, to match the minimum
acceptable value of SYMLOOP_MAX in POSIX.

Previously I'd avoided reporting ELOOP thinking Windows didn't have it,
but it turns out that it does, so we can use the proper error number.

Reviewed-by: Roman Zharkov <r.zharkov@postgrespro.ru>
Discussion: https://postgr.es/m/CA%2BhUKGJ7JDGWYFt9%3D-TyJiRRy5q9TtPfqeKkneWDr1XPU1%2Biqw%40mail.gmail.com
Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com
2022-10-25 16:19:05 +13:00