If -l was specified together with selective-restore options such as -n
or -N, dependent TOC entries such as comments would be omitted from
the listing, even when an actual restore would have selected them.
This happened because PrintTOCSummary neglected to update the te->reqs
marking of the entry they depended on.
Per report from Justin Pryzby. This has been wrong since 0d4e6ed30
taught _tocEntryRequired to sometimes look at the "reqs" marking of
other TOC entries, so back-patch to all supported branches.
Discussion: https://postgr.es/m/ZjoeirG7yxODdC4P@pryzbyj2023
If a plpython-language trigger caused another one to be invoked,
the "TD" dictionary created for the inner one would overwrite the
outer one's "TD" dictionary. This is more or less the same problem
that 1d2fe56e4 fixed for ordinary functions in plpython, so fix it
the same way, by saving and restoring "TD" during a recursive
invocation.
This fix makes an ABI-incompatible change in struct PLySavedArgs.
I'm not too worried about that because it seems highly unlikely that
any extension is messing with those structs. We could imagine doing
something weird to preserve nominal ABI compatibility in the back
branches, like keeping the saved TD object in an extra element of
namedargs[]. However, that would only be very nominal compatibility:
if anything *is* touching PLySavedArgs, it would likely do the wrong
thing due to not knowing about the additional value. So I judge it
not worth the ugliness to do something different there.
(I also changed struct PLyProcedure, but its added field fits
into formerly-padding space, so that should be safe.)
Per bug #18456 from Jacques Combrink. This bug is very ancient,
so back-patch to all supported branches.
Discussion: https://postgr.es/m/3008982.1714853799@sss.pgh.pa.us
When changing the data type of a column of a partitioned table, craft
the ALTER SEQUENCE command only once. Partitions do not have identity
sequences of their own and thus do not need a ALTER SEQUENCE command
for each partition.
Fix getIdentitySequence() to fetch the identity sequence associated
with the top-level partitioned table when a Relation of a partition is
passed to it. While doing so, translate the attribute number of the
partition into the attribute number of the partitioned table.
Author: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com>
Discussion: https://www.postgresql.org/message-id/3b8a9dc1-bbc7-0ef5-6863-c432afac7d59@gmail.com
The executor only supports CurrentOfExpr as the sole tidqual of a
TidScan plan node. tidpath.c failed to take any particular care about
that, but would just take the first ctid equality qual it could find
in the target relation's baserestrictinfo list. Originally that was
fine because the grammar prevents any other WHERE conditions from
being combined with CURRENT OF <cursor>. However, if the relation has
RLS visibility policies then those would get included in the list.
Should such a policy include a condition on ctid, we'd typically grab
the wrong qual and produce a malfunctioning plan.
To fix, introduce a simplistic priority ordering scheme for which ctid
equality qual to prefer. Real-world cases involving more than one
such qual are so rare that it doesn't seem worth going to any great
trouble to choose one over another, so I didn't work very hard; but
this code could be extended in future if someone thinks differently.
It's extremely difficult to think of a reasonable use-case for an RLS
restriction involving ctid, and certainly we've heard no field reports
of this failure. So this doesn't seem worthy of back-patching, but
in the name of cleanliness let's fix it going forward.
Patch by me, per report from Robert Haas.
Discussion: https://postgr.es/m/3914881.1715038270@sss.pgh.pa.us
Both the pgbench --help output and the reference page have sections
for initialization options, benchmarking options, and common options.
But the --debug option ended up in the wrong place on the reference
page. Fix that by making the documentation match the --help output.
The code change this made might well be fine to keep, but the
comment justifying it by reference to self-join removal isn't.
Let's just go back to the status quo ante, pending a more thorough
review/redesign of SJE.
(I found this by grepping to see if any references to self-join
removal remained in the tree.)
The catalog view pg_stats_ext fails to consider privileges for
expression statistics. The catalog view pg_stats_ext_exprs fails
to consider privileges and row-level security policies. To fix,
restrict the data in these views to table owners or roles that
inherit privileges of the table owner. It may be possible to apply
less restrictive privilege checks in some cases, but that is left
as a future exercise. Furthermore, for pg_stats_ext_exprs, do not
return data for tables with row-level security enabled, as is
already done for pg_stats_ext.
On the back-branches, a fix-CVE-2024-4317.sql script is provided
that will install into the "share" directory. This file can be
used to apply the fix to existing clusters.
Bumps catversion on 'master' branch only.
Reported-by: Lukas Fittl
Reviewed-by: Noah Misch, Tomas Vondra, Tom Lane
Security: CVE-2024-4317
Backpatch-through: 14
Both the initdb --help output and the reference page have a section
for options and a section for less commonly used options. But some
recently added options were sprinkled around inconsistently. Fix that
by making the documentation match the --help output.
Injection points created under injection_points_set_local() are cleaned
up by a shmem_exit() callback. The spinlock used by the module would
be hold while calling InjectionPointDetach(), which is incorrect as
spinlocks should avoid external calls while hold.
This commit changes the shmem_exit() callback to detach the points in
three steps with the spinlock acquired twice, knowing that the
injection points should be around with the conditions related to them:
- Scans for the points to detach in a first loop, while holding the
spinlock.
- Detach them.
- Remove the registered conditions.
It is still possible for other processes to detach local points
concurrently of the callback. I have wanted to restrict the detach, but
Noah has mentioned that he has in mind some cases that may require this
capability. No tests in the tree based on injection points need that
currently.
Thinko in f587338dec.
Reported-by: Noah Misch
Reviewed-by: Noah Misch
Discussion: https://postgr.es/m/20240501231214.40@rfd.leadboat.com
If pg_init_privs were to contain a NULL ACL field, this code would
pass old_acl == NULL to merge_acl_with_grant, which would crash.
The case shouldn't happen, but it just takes a couple more lines
of code to guard against it, so do so.
Oversight in 534287403; no back-patch needed.
94985c210 added code to detect when WindowFuncs were monotonic and
allowed additional quals to be "pushed down" into the subquery to be
used as WindowClause runConditions in order to short-circuit execution
in nodeWindowAgg.c.
The Node representation of runConditions wasn't well selected and
because we do qual pushdown before planning the subquery, the planning
of the subquery could perform subquery pull-up of nested subqueries.
For WindowFuncs with args, the arguments could be changed after pushing
the qual down to the subquery.
This was made more difficult by the fact that the code duplicated the
WindowFunc inside an OpExpr to include in the WindowClauses runCondition
field. This could result in duplication of subqueries and a pull-up of
such a subquery could result in another initplan parameter being issued
for the 2nd version of the subplan. This could result in errors such as:
ERROR: WindowFunc not found in subplan target lists
To fix this, we change the node representation of these run conditions
and instead of storing an OpExpr containing the WindowFunc in a list
inside WindowClause, we now store a new node type named
WindowFuncRunCondition within a new field in the WindowFunc. These get
transformed into OpExprs later in planning once subquery pull-up has been
performed.
This problem did exist in v15 and v16, but that was fixed by 9d36b883b
and e5d20bbd.
Cat version bump due to new node type and modifying WindowFunc struct.
Bug: #18305
Reported-by: Zuming Jiang
Discussion: https://postgr.es/m/18305-33c49b4c830b37b3%40postgresql.org
Commit 619bc23a1 changed "make dist" to invoke "git archive",
but hard-wired the call to specify that the HEAD revision should
be packaged. Our tarball building process needs to be able to
specify which git commit to package (notably, for packaging
back branches). While we could make that work with some hackery
to operate in detached-HEAD state, it's a lot nicer just to expose
git archive's ability to specify what to package. Hence, invent
a new make variable PG_GIT_REVISION. This is undocumented, but
so is "make dist".
Also make corresponding changes in the meson scripts. We have no
near-term intention of using that for package building, but it
will likely happen eventually, so stay prepared.
Discussion: https://postgr.es/m/3552543.1713909947@sss.pgh.pa.us
While converting a pg_attribute tuple into a ColumnDef,
ColumnDef::compression remains NULL if there is no compression method
set fot the attribute. Calling strcmp() with NULL
ColumnDef::compression, when comparing compression methods of parents,
causes segmentation fault in MergeInheritedAttribute(). Skip
comparing compression methods if either of them is NULL.
Author: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://www.postgresql.org/message-id/b22a6834-aacb-7b18-0424-a3f5fe889667%40gmail.com
We support changing NO INHERIT constraint to INHERIT for constraints in
child relations when adding a constraint to some ancestor relation, and
also during pg_upgrade's schema restore; but other than those special
cases, command ALTER TABLE ADD CONSTRAINT should not be allowed to
change an existing constraint from NO INHERIT to INHERIT, as that would
require to process child relations so that they also acquire an
appropriate constraint, which we may not be in a position to do. (It'd
also be surprising behavior.)
It is conceivable that we want to allow ALTER TABLE SET NOT NULL to make
such a change; but in that case some more code is needed to implement it
correctly, so for now I've made that throw the same error message.
Also, during the prep phase of ALTER TABLE ADD CONSTRAINT, acquire locks
on all descendant tables; otherwise we might operate on child tables on
which no locks are held, particularly in the mode where a primary key
causes not-null constraints to be created on children.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/7d923a66-55f0-3395-cd40-81c142b5448b@gmail.com
libpq's pqTraceOutputMessage() used to look like this:
case 'Z': /* Ready For Query */
pqTraceOutputZ(conn->Pfdebug, message, &logCursor);
break;
Commit f4b54e1ed9 introduced macros for protocol characters, so now
it looks like this:
case PqMsg_ReadyForQuery:
pqTraceOutputZ(conn->Pfdebug, message, &logCursor);
break;
But this introduced a disconnect between the symbol in the switch case
and the function name to be called, so this made the manageability of
this file a bit worse.
This patch changes the function names to match, so now it looks like
this:
case PqMsg_ReadyForQuery:
pqTraceOutput_ReadyForQuery(conn->Pfdebug, message, &logCursor);
break;
(This also improves the readability of the file in general, since some
function names like "pqTraceOutputt" were a little hard to read
accurately.)
Some protocol characters have different meanings to and from the
server. The old code structure had a common function for both, for
example, pqTraceOutputD(). The new structure splits this up into
separate ones to match the protocol message name, like
pqTraceOutput_Describe() and pqTraceOutput_DataRow().
Reviewed-by: Yugo NAGATA <nagata@sraoss.co.jp>
Discussion: https://www.postgresql.org/message-id/flat/575e4f9d-acfe-45e3-b7f1-7e32c579090e%40eisentraut.org
Such constraints are semantically useless and only bring weird cases
along, so reject them.
As a side effect, we can no longer have "throwaway" constraints in
pg_dump for primary keys in partitioned tables, but since they don't
serve any useful purpose, we can just omit them.
Maybe this should be done for all types of constraints, but it's just
not-null ones that acquired this "ability" in the 17 timeframe, so for
the moment I'm not changing anything else.
Per note by Alexander Lakhin.
Discussion: https://postgr.es/m/7d923a66-55f0-3395-cd40-81c142b5448b@gmail.com
Commit 61461a300c accidentally misspelled the PGcancelConn struct
using the PQ prefix (which admittedly is a very easy typo to make).
Reported off-list.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
The documentation said that you need to pick a suitable LC_COLLATE
setting in addition to setting the DETERMINISTIC flag. This would
have been correct if the libc provider supported nondeterministic
collations, but since it doesn't, you actually need to set the LOCALE
option.
Reviewed-by: Kashif Zeeshan <kashi.zeeshan@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/a71023c2-0ae0-45ad-9688-cf3b93d0d65b%40eisentraut.org
When testing pg_upgrade against an old server, ignore failures on the
check to upgrade invalid databases. This is necessary because old
servers don't know to raise the appropriate error of the database being
invalid.
This change causes no reduction in coverage, because such old versions
don't know to mark databases invalid when a drop is interrupted; but
testing against such old servers is useful in some circumstances.
Backpatch to 16, where it cherry-picks with minimal conflicts.
On 16, perltidy 20230309 chooses to change an unrelated line. I let it
do that because that's the version we document as preferred for that
branch, even though it would make other changes to many other files in
the tree.
Discussion: https://postgr.es/m/202404181539.lh42llaesnv3@alvherre.pgsql
A parallel worker's buffer usage is accumulated to its pgBufferUsage
and then is accumulated into the leader's one at the end of the
parallel vacuum. However, since the leader process used to use
dedicated VacuumPage{Hit, Miss, Dirty} globals for the buffer usage
reporting, the worker's buffer usage was not included, leading to an
incorrect buffer usage report.
To fix the problem, this commit makes vacuum use pgBufferUsage
instruments for buffer usage reporting instead of VacuumPage{Hit,
Miss, Dirty} globals. These global variables are still used by ANALYZE
command and autoanalyze.
This also fixes the buffer usage report of vacuuming on temporary
tables, since the buffers dirtied by MarkLocalBufferDirty() were not
tracked by the VacuumPageDirty variable.
Parallel vacuum was introduced in 13, but the buffer usage reporting
for VACUUM command with the VERBOSE option was implemented in
15. So backpatch to 15.
Reported-by: Anthonin Bonnefoy
Author: Anthonin Bonnefoy
Reviewed-by: Alena Rybakina, Masahiko Sawada
Discussion: https://postgr.es/m/CAO6_XqrQk+QZQcYs_C6nk0cMfHuUWk85vT9CrcA1NffFbAVE2A@mail.gmail.com
Backpatch-through: 15
As an optimization, we store "name" columns as cstrings in btree
indexes.
Here we modify it so that Index Only Scans convert these cstrings back
to names with NAMEDATALEN bytes rather than storing the cstring in the
tuple slot, as was happening previously.
Bug: #17855
Reported-by: Alexander Lakhin
Reviewed-by: Alexander Lakhin, Tom Lane
Discussion: https://postgr.es/m/17855-5f523e0f9769a566@postgresql.org
Backpatch-through: 12, all supported versions
If the bootstrap superuser's name requires quoting, regroleout
will supply double quotes ... but the result of CURRENT_USER
is just the literal name. Apply quote_ident() to ensure a match.
Per Andrew Dunstan's off-list investigation of buildfarm member
prion's failures.
This commit makes new partitions created by ALTER TABLE ... SPLIT PARTITION
and ALTER TABLE ... MERGE PARTITIONS commands inherit the paret table access
method.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/84ada05b-be5c-473e-6d1c-ebe5dd21b190%40gmail.com
Reviewed-by: Pavel Borisov
This commit implements psql tab completion for ALTER TABLE ... SPLIT PARTITION
and ALTER TABLE ... MERGE PARTITIONS commands.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/5dee3937-8e9f-cca4-11fb-737709a92b37%40gmail.com
Author: Dagfinn Ilmari Mannsåker, Pavel Borisov
Replace "salesman" with "salesperson", "salesmen" with "salespeople". The
names are both gramatically correct and gender-neutral.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/fdaa003e-919c-cbc9-4f0c-e4546e96bd65%40gmail.com
Reviewed-by: Robert Haas, Pavel Borisov
Currently, the error message is produced by a system of complex substitutions
making it quite untranslatable and hard to read. This commit splits this into
4 plain error messages suitable for translation.
Reported-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20240408.152402.1485994009160660141.horikyota.ntt%40gmail.com
Reviewed-by: Pavel Borisov
The createPartitionTable() function is responsible for creating new partitions
for ALTER TABLE ... MERGE PARTITIONS, and ALTER TABLE ... SPLIT PARTITION
commands. It emulates the behaviour of CREATE TABLE ... (LIKE ...), where
new table persistence should be specified by the user. In the table
partitioning persistent of the partition and its parent must match. So, this
commit makes createPartitionTable() copy the persistence of the parent
partition.
Also, this commit makes createPartitionTable() recheck the persistence after
the new table creation. This is needed because persistence might be affected
by pg_temp in search_path.
This commit also changes the signature of createPartitionTable() making it
take the parent's Relation itself instead of the name of the parent relation,
and return the Relation of new partition. That doesn't lead to
complications, because both callers have the parent table open and need to
open the new partition.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/dbc8b96c-3cf0-d1ee-860d-0e491da20485%40gmail.com
Author: Dmitry Koval
Reviewed-by: Alexander Korotkov, Robert Haas, Justin Pryzby, Pavel Borisov
The name collision happens when the name of the new partition is the same as
the name of one of the merging partitions. Currently, ATExecMergePartitions()
first gives the new partition a temporary name and then renames it when old
partitions are deleted. That negatively influences the naming of related
objects like indexes and constrains, which could inherit a temporary name.
This commit changes the implementation in the following way. A merging
partition gets renamed first, then the new partition is created with the
right name immediately. This resolves the issue of the naming of related
objects.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/edfbd846-dcc1-42d1-ac26-715691b687d3%40postgrespro.ru
Author: Dmitry Koval, Alexander Korotkov
Reviewed-by: Robert Haas, Justin Pryzby, Pavel Borisov
I'd not checked that this iteration of the test actually worked
with a bootstrap superuser not named 'postgres'. It didn't,
because the coercion rules for CASE caused us to try to cast
the 'postgres' literal to regrole. Mea culpa.
Per buildfarm (via Alexander Korotkov)
Discussion: https://postgr.es/m/CAPpHfdsV=iTvH6B858hnH1bLgewYH6cdTnO_eOOw9EOa8kehkA@mail.gmail.com
This had been disabled because the test "doesn't delete its user".
It doesn't seem like a great idea for the meson tests to act
differently from the makefile tests, though, and the makefiles
had no such exception (which is how come only copperhead noticed
the problem just fixed in 534287403). In any case, the premise
is false since 936e3fa37, so let's remove the restriction.
Discussion: https://postgr.es/m/2857513.1713733688@sss.pgh.pa.us
