The documentation for log_parameter_max_length said it affects messages
generated by log_duration. However, log_duration alone does not log bind
parameter values, so this is misleading.
This commit updates the documentation to reference log_min_duration_statement,
which can log bind parameters, to better reflect actual behavior.
Backpatch to all supported versions.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Surya Poondla <suryapoondla4@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwGnCVMVz8-LU9F8Sh57bkQX3jMZzx7age7M0LFEz5=Fog@mail.gmail.com
Backpatch-through: 14
Heap WAL records that clear bits on the visibility map (like inserts and
deletes) did not register the visibility map blocks they modified.
Because the WAL summarizer only records registered blocks, an
incremental backup taken over such operations would omit the changed VM
pages. On restore, the VM would retain stale all-visible/all-frozen
bits, which can cause wrong results from index-only scans and incorrect
relfrozenxid advancement due to vacuum page skipping.
Not registering the VM buffer also meant we never emitted FPIs of VM
pages when clearing bits. A torn VM page won't raise an error because
the VM is read with ZERO_ON_ERROR; with checksums on, it would be
detected and zeroed, but with checksums off, it is accepted as-is and
can lead to data corruption.
Fix this by registering the VM buffer in the WAL record when clearing VM
bits. The VM buffer must now be locked throughout the critical section
that modifies the VM and heap pages and emits the WAL record. This can
slow down operations that clear the VM, since the VM lock is held longer
and VM FPIs may be emitted, but it is required for correctness.
Note that this fix does not repair existing incremental backups.
Bumps XLOG_PAGE_MAGIC. Though it is late in the cycle (post-beta 2) to
be doing so, that seemed better than maintaining the backwards
compatability code in yet another branch.
Author: Melanie Plageman <melanieplageman@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/flat/CAAKRu_bn%2Be7F4yPFBgFbnP%2BsyJRKyNK092bjD2LKvZW7O4Svag>
Backpatch-through: 17
When registering a buffer with the WAL machinery, the caller assigns it a
block reference ID, and replay must read each block back by that same ID.
Today these IDs are bare integers assigned by convention (0, 1, 2, ...),
which is easy to follow when a record registers a single block, or when the
blocks are handled during replay in their registration order.
An upcoming bug fix registers up to two visibility map blocks in addition
to the heap block(s) when clearing the VM, and these are not handled during
replay in a straightforward 1:1, in-registration-order fashion. Relying on
bare integers for the block IDs in that case is error-prone.
Introduce macros naming the block reference IDs for the heap record types
that the upcoming commit extends to register visibility map blocks, so the
registration and replay sites refer to the same block by a meaningful name.
Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/66mqpfyti3qhfttcsv6r2lbvqqd32rrmpn6i47ovrsnvguts46%40gou54xc>
Backpatch through: 17
When bulk-extending a relation, we add the newly-added blocks that we
won't immediately use to the free space map and then call
FreeSpaceMapVacuumRange() to propagate that free space up the FSM tree,
so other backends can find and reuse it.
However, the end block argument to FreeSpaceMapVacuumRange() is
exclusive, and we passed the number of the last added block (since
00d1e02be2). If that block was the first one covered by a new FSM page,
its free space wasn't propagated up the tree and was therefore invisible
to FSM searches until the next FSM vacuum.
Fix by passing the block number one past the last added block, so the
full range is vacuumed.
Author: Jingtang Zhang <mrdrivingduck@gmail.com>
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://postgr.es/m/flat/CAPsk3_Bx_vdybN%3D-DZu8HLStf%2BXnuFUBkLwxouONSMkWuO9oug%40mail.gmail.com
Backpatch-through: 16
Code elsewhere in the system assumes that fork numbers and chunk sizes
are within bounds, so the code that reads those quantities from disk
should validate that they are. Without these additional checks, a
corrupted file can cause us to index off the end of fork number or chunk
entry arrays, potentially resulting in a crash.
Reported-by: oxsignal <awo@kakao.com> (chunk sizes)
Reported-by: Robert Haas <rhaas@postgresql.org> (fork numbers)
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: http://postgr.es/m/CA+TgmoYP8RKoBGosS7C6Fdr-GNCfyz_W1zmK=Tx1Fe0ZvzGh0g@mail.gmail.com
Backpatch-through: 17
pg_clear_attribute_stats() checks its required arguments manually
because the function is not strict. Previously, when schemaname or
relname was passed as NULL, the error incorrectly reported the
argument name as "relation" in both cases:
ERROR: argument "relation" must not be null
This was misleading, especially for schemaname, and inconsistent with
the function's SQL-visible argument names.
The cause is that cleararginfo[] in attribute_stats.c used
"relation" for both the schema-name and relation-name arguments.
This commit fixes the issue by using "schemaname" and "relname" instead,
matching the function's declared argument names so that the error reports
the correct argument name.
Backpatch to v18, where pg_clear_attribute_stats() was introduced.
Author: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/4bf66c5e-8dd7-4ef3-8691-db67ecff6f16@tantorlabs.com
Backpatch-through: 18
'ALTER SUBSCRIPTION ... REFRESH SEQUENCES' can race with an already
running sequence synchronization worker. If a second refresh request
resets the synchronization state while the worker has already fetched
sequence values from the publisher but has not yet applied them to the
subscriber, the worker can overwrite the subscriber with stale values
and mark the synchronization as complete.
Avoid this race by rejecting 'ALTER SUBSCRIPTION ... REFRESH SEQUENCES'
when a sequence synchronization worker is already running for the
subscription. The command reports an error asking the user to rerun it
after the current synchronization completes.
Also add a wait for the re-added 'regress_s4' sequence to finish
synchronizing in 036_sequences.pl, so the subsequent test does not race
against its sequencesync worker.
Reported-by: Noah Misch <noah@leadboat.com>
Author: vignesh C <vignesh21@gmail.com>
Reviewed-by: Shveta Malik <shveta.malik@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Backpatch-through: 19, where it was introduced
Discussion: https://postgr.es/m/20260710045217.f0.noahmisch@microsoft.com
Contrary to the from_serialized_data callback used by the pgstats reads
at startup, the to_serialized_data callback used for the pgstats writes
matched with pgstat_write_statsfile(), by not returning a boolean
status, expecting a ferror() failure to deal with the discard of the
stats file should an error happen while writing the stats. This was
slightly confusing designed this way.
Things are changed in this commit with:
- to_serialized_data now returns a boolean status on a write failure.
pgstat_write_statsfile() detects that and switches to failure mode
instead of continuing to process the entries to write, speeding up the
shutdown.
- pgstat_write_statsfile() now uses STATS_DISCARD if a failure happens,
to let the registered callbacks directly know that something is wrong,
and that things need to be cleaned up. This gives a better error path
detection for custom stats kinds. For example, they do not have to rely
solely on the expectation of an ferror() for an auxiliary file.
This new set of behaviors matches with what is already done in
pgstat_read_statsfile() for the finish() callback (DISCARD on failure,
READ on success) and the from_serialized_data with a status returned.
Author: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/CAA5RZ0sMgOvuhpb2P=KSJOjgjC6AfUu+GYcu9mHar-y_Xtd=Pg@mail.gmail.com
Backpatch-through: 19
equalPolicy() is used in the relation cache to check if two policy
definitions are equivalent, but missed to check for polpermissive.
ALTER POLICY cannot switch a policy to be PERMISSIVE or RESTRICTIVE, so
this would need a dropped and then re-created policy, which would
trigger a relcache invalidation. Anyway, there is no harm in being
consistent in the check, and if one decides to add an ALTER POLICY to
switch PERMISSIVE or RESTRICTIVE, we would be silently in trouble.
Author: Andreas Lind <andreaslindpetersen@gmail.com>
Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at>
Discussion: https://postgr.es/m/CAMxA3rv1CS6R7JR5ojz-3CmCEnZEFrqu+XXTnGbLRWrjJRH7sA@mail.gmail.com
Backpatch-through: 14
Commit 9a60f295b stripped the stale PlaceHolderVars left behind by
left-join removal from the surviving rels' baserestrictinfo and from
EquivalenceClass member expressions, but it overlooked join clauses.
A PlaceHolderVar embedded in a join clause can likewise retain the
removed rel and join in its phrels, since remove_rel_from_query()
fixes up the RestrictInfo's own relid sets but not the PHVs inside its
expression.
As before, this is normally harmless, because later processing
consults those relid sets rather than the embedded PHVs. However, a
restriction clause derived from such an OR join clause inherits the
stale PlaceHolderVar, and when the derived clause is translated for an
appendrel child, pull_varnos() recomputes its relids and folds the
removed relation back in. The rebuilt clause then references a
no-longer-existent relation, tripping an assertion during path
generation.
Fix by also stripping the removed relation from the PlaceHolderVars in
the surviving rels' join clauses, including the sub-clauses of any OR
clause.
Like 9a60f295b, this is only reachable on v18 and later, where
match_index_to_operand() began ignoring PlaceHolderVars.
Author: Arne Roland <arne.roland@malkut.net>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/27a44087-3d65-473e-8d88-7c12228e0d7e@malkut.net
Backpatch-through: 18
This reverts commit ed823da128, that has made pgstat_write_chunk() and
pgstat_read_chunk() available for public use. These routines do not
have a symmetric API definition across reads and writes, with the write
part returning a void status, deferring an error detection once all the
stats entries have been processed with an ferror(), and the read part
returning a boolean status.
These routines are just tiny wrappers around fread() and fwrite(), and
extensions can just define they own routines instead of relying on the
same facilities as the core pgstat.c. This commit removes their
declaration from the public headers, to reduce the confusion.
test_custom_stats is updated to use its own read/write routines.
Perhaps something better could be designed in the future; trying to do
so for v19 is not feasable during beta.
Reported-by: Peter Eisentraut <peter@eisentraut.org>
Author: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/a4a8e9af-3eaf-4bbf-9b21-21620f3fc434@eisentraut.org
Backpatch-through: 19
Commit 62c3b4cd9d taught postgres_fdw to push down ArrayCoerceExpr, but
foreign_expr_walker() only recursed into the input array expression and
never examined elemexpr, the per-element conversion that gives the
coercion its semantics. deparseArrayCoerceExpr() then shipped a bare
"arg::resulttype" cast, or nothing at all for an implicit-format
coercion, leaving the remote server to re-resolve the element conversion
against its own catalogs and session state.
This produced wrong results or remote errors whenever the element
conversion was not a plain relabeling, and it was inconsistent with how
postgres_fdw treats the equivalent scalar coercions. An ArrayCoerceExpr
was shipped even when its elemexpr was a cast function (whose
shippability was never checked), a CoerceViaIO (e.g. float8out or
byteaout, which depend on extra_float_digits / bytea_output that
postgres_fdw sets differently on the remote session), or a
CoerceToDomain (which pushes domain enforcement to the remote catalog).
By contrast, a scalar CoerceViaIO is never shipped, and a scalar cast
function is shipped only when it is shippable.
Restrict pushdown to element coercions that are a plain relabeling, that
is, elemexpr is a RelabelType or a bare CaseTestExpr. Any other element
coercion is now evaluated locally. This keeps the common
binary-coercible case pushed down, including "col = ANY($1)" with a
varchar[]-to-text[] relabeling, which is the case 62c3b4cd9d set out to
optimize.
Pushing down shippable element cast functions, to reach parity with the
scalar case, is left out here for simplicity.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260711024234.43.noahmisch%40microsoft.com
Backpatch-through: 19
The pg_attribute_always_inline macro name is so long it forces pgindent
to format the code in strange ways. Which may incentivize patch authors
to either structure the code in strange ways (e.g. reorder prototypes),
use shorter names, etc. Neither is very desirable for code readability.
This shortens the name by removing the _attribute_ part. It also makes
it more consistent with pg_noinline, which does not have the _attribute_
part either.
Backpatched to all supported branches, to prevent conflicts when
backpatching other fixes. The backbranches however keep both the old and
new macro name, so that existing code keeps working.
Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Peter Geoghegan <pg@bowt.ie>
Reviewed-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/bqqdehahpoa36igpictuqyn2s2mexk3t3ehidh2ffd2slb35e5@rzgksuiszgbg
Backpatch-through: 14
In c75ebc657f I (Andres) introduced UnlockBufHdrExt() which can set and
clear bits in the buffer state using CAS. Unfortunately I added bits before
subtracting them, which means that a bit that was both removed and set would
remain unset. Fix the order of operations.
The only known case where that is a problem is that BM_IO_ERROR would not
actually remain set.
It's unfortunately not trivial to add a decent, race-free, test to verify that
BM_IO_ERROR remains set. That's therefore left for the 20 cycle.
Reported-by: Yura Sokolov <y.sokolov@postgrespro.ru>
Discussion: https://postgr.es/m/ab0dcc9e-aba0-44e3-ac23-8d74c48888e6@postgrespro.ru
Backpatch-through: 19, where c75ebc657f went in
When doing a whole database repack, we build a list of tables to process
taking a lock on each. But because it's a regular transaction-scoped
lock, it's automatically released immediately after building the list
anyway, which makes it not very useful. (Also, we have three ways to
obtain a list of tables to repack, and only one of them acquired this
lock.) Remove that lock acquisition, as it's useless and inconsistent.
We acquire a lock properly afterwards (and recheck that the table can
still be repacked as indicated), so we don't need to do anything other
than drop that initial lock acquisition and harden the code in
repack_is_permitted_for_relation() against possible concurrent drops.
This is similar to how vacuum does it in get_all_vacuum_rels().
In order for this to work reliably, also change
repack_is_permitted_for_relation() to cope with the possibility of the
table going away partway through. Similarly, in ExecRepack(), be
prepared for what we believed to be a table or matview to now be
something else, and skip it without erroring out, by changing
try_table_open() to try_relation_open() and testing the relkind
separately.
While at it, replace one relation_close() call in get_tables_to_repack()
with table_close() to match the table_open() that opened the catalog.
Author: ChangAo Chen <cca5507@qq.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/tencent_9F290B256A3F52B66542F1140E32ECC64309@qq.com
When building the list of temporary relations to wait for, the code
previously included temporary relations without storage, such as
temporary views, even though they are irrelevant to checksum
processing. As a result, enabling data checksums could wait for a
long-lived session that owned only a temporary view.
This commit fixes the issue by filtering temporary relations with storage
only, matching the existing behavior for non-temporary relations.
Also, when enabling data checksums online, the launcher assigns the
first worker to process shared catalogs and prevents later workers from
doing so. Previously, if that worker's database was dropped after it
had been selected for processing but before checksum processing began,
the worker failed without processing the shared catalogs, yet they were
still marked as processed. As a result, later workers skipped them, and
checksum enabling could complete successfully even though the shared
catalogs had never been processed.
This commit fixes the issue by marking shared catalogs as processed
only after a worker completes successfully.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CAHGQGwGDHAQw=bmpRzk+EmKzVtxZiD5YDurMUffBMwr6WXugQA@mail.gmail.com
Backpatch-through: 19
pg_stat_progress_data_checksums uses -1 as a sentinel value that is
displayed as NULL for progress counters. However, after
pgstat_progress_start_command() initialized all progress counters to
zero, data checksum progress did not reset those counters to -1.
As a result, some counters could incorrectly appear as zero instead
of NULL. For example, workers could report zero database counters,
and the disabling launcher could report zero relation and block counters.
Also, blocks_done was not reset when a worker started processing
a new relation fork. As a result, it could temporarily exceed blocks_total
or report a stale value for an empty relation fork.
Fix this by initializing the data checksum progress counters to -1
when progress reporting starts for both launcher and worker processes.
Also reset blocks_done together with blocks_total when starting each
relation fork.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CAHGQGwEOQyEzW2cqrHEzvwbcsAsuH8MEe7MMidFOFxECy0E1_Q@mail.gmail.com
Backpatch-through: 19
Previously, this function imported remote statistics by executing SQL
functions like pg_restore_relation_stats and pg_restore_attribute_stats
via SPI (in read-write mode). As the SQL functions take a schema name
and a relation name as two separate arguments, rather than a single OID
argument, if the containing schema was concurrently renamed, the
callback function would throw an error like this:
ERROR: schema "foo" does not exist
To fix, 1) provide new interface functions to import remote statistics
that are directly callable from FDWs and take a single OID, and 2)
modify the callback function to use the interface functions instead when
importing remote statistics.
For #1, this commit does a bit of refactoring to
relation_statistics_update and attribute_statistics_update, which are
the workhorse functions for pg_restore_relation_stats and
pg_restore_attribute_stats respectively: since they also take a schema
name and a relation name, separate the guts of them into new functions
so that they take a single OID and are callable not only from the
workhorse functions but from the interface functions introduced by #1.
Oversight in commit 28972b6fc.
Reported-by: Robert Haas <robertmhaas@gmail.com>
Suggested-by: Robert Haas <robertmhaas@gmail.com>
Author: Corey Huinker <corey.huinker@gmail.com>
Co-authored-by: Etsuro Fujita <etsuro.fujita@gmail.com>
Discussion: https://postgr.es/m/CA%2BTgmoYqMtWb4zLUkT98oFnEkJ%3DWz0Pw-ggDJrp9wnSXPzUaeQ%40mail.gmail.com
Backpatch-through: 19
Check explicitly for pqsecure_read() returning an error. It shouldn't
fail, and we would've caught it in the check for a short read, but
better to be explicit so that the error message is more informative.
We also shouldn't update 'inEnd' when the read fails, although that
too is just pro forma as we will bail out and close the connection on
error.
Reported-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://www.postgresql.org/message-id/34844e8c-267c-4daf-b1e0-f26059a4a7d3@eisentraut.org
Backpatch-through: 14
The new partitions built for ALTER TABLE ... SPLIT PARTITION and
ALTER TABLE ... MERGE PARTITIONS are created at the explicit request of
the user, just like a plain CREATE TABLE. createPartitionTable() passes
is_internal=true to heap_create_with_catalog(), while createTableConstraints()
does the same to StoreAttrDefault() and AddRelationNewConstraints().
Pass is_internal=false in all these places instead, so that object-access
hooks treat them as user-requested objects. The is_internal flag is intended
for objects created as internal implementation details, such as a transient
heap built during CLUSTER.
While at it, pass 0 rather than PERFORM_DELETION_INTERNAL to the
performDeletionCheck() calls that pre-check the drop eligibility of the
old partitions, to match the subsequent performDeletion(). The flag has
no functional effect on performDeletionCheck(), but change this for code
consistency.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260707185751.f9.noahmisch@microsoft.com
Backpatch-through: 19
When executing an UPDATE with a RETURNING clause on a table with a
BEFORE UPDATE row trigger, the computation of the OLD values in the
RETURNING list was incorrect if the target tuple was concurrently
updated by another session, at isolation level READ COMMITTED.
The problem was that the trigger code would lock the target tuple,
waiting for the other session to commit, and then fetch the updated
target tuple, but ExecUpdate() would not realise that the target tuple
had changed, and use the outdated target tuple for computing OLD
values. Fix by having ExecUpdate() check the TM_FailureData from
trigger execution and re-fetch the target tuple if necessary.
Re-fetching the target tuple like this is a little inefficient, but
probably negligible compared to the trigger execution and update. A
better long-term fix might be to move the EPQ code out of trigger.c,
and let ExecUpdate() handle it, like ExecMergeMatched() does, but that
would likely mean changing the trigger API, which seems a bit much for
back-patching.
Backpatch to v18, where support for RETURNING OLD/NEW was added.
Bug: #19536
Reported-by: Jonas Boberg <bobergj@gmail.com>
Diagnosed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Author: Dean Rasheed <dean.a.rasheed@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/19536-73ce5847e6c0e7b1@postgresql.org
Backpatch-through: 18
WAIT_FOR_WAL_FLUSH, WAIT_FOR_WAL_REPLAY, and WAIT_FOR_WAL_WRITE were
placed in the WaitEventClient class. But WaitEventClient is about
waiting for a socket to become readable or writable, while these events
have other delay sources as well: local fsync and local replay, which
may be disk- or CPU-bound. WaitEventIPC is a better fit, so move them
there.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260706012642.f9.noahmisch@microsoft.com
Backpatch-through: 19
Sequence synchronization reports insufficient privileges on publisher
and subscriber sequences, but the warnings do not indicate which role
needs which privilege. This makes common configuration mistakes harder
to diagnose.
Add HINT messages for these warnings. Publisher-side warnings suggest
granting SELECT to the role used for the replication connection.
Subscriber-side warnings suggest granting UPDATE to the subscription
owner when run_as_owner is enabled. Otherwise, the worker runs as the
sequence owner, so no useful GRANT hint can be provided.
Suggested-by : Amit Kapila <amit.kapila16@gmail.com>
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/CAA4eK1JOo0aJRhFHNWpj3hMwaTtNOopY34f1Lh_QD=z=+DrzWQ@mail.gmail.com
Backpatch-through: 19
The documentation previously said that pg_get_sequence_data() returns
a row of NULL values if the sequence does not exist or if the current
user lacks privileges on it. This was incomplete and could be misleading.
A nonexistent relation name is rejected during regclass input conversion,
while the function returns NULLs for a nonexistent relation OID and
several other cases.
This commit clarifies that the function returns NULLs when the specified
relation OID does not exist, the relation is not a sequence, the current
user lacks SELECT privilege on the sequence, the sequence belongs to
another session's temporary schema, or it is an unlogged sequence on
a standby server.
Author: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAA4eK1JOo0aJRhFHNWpj3hMwaTtNOopY34f1Lh_QD=z=+DrzWQ@mail.gmail.com
Backpatch-through: 19
When a string literal is provided as a property expression, the data
type of the property was set to "unknown", which may lead to various
failures when the property is used in GRAPH_TABLE or when its data
type is compared against other properties with the same name. To fix
this, call resolveTargetListUnknowns() on the targetlist of new
properties being added to resolve unknown type literals.
Reported-by: Noah Misch <noah@leadboat.com>
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/20260630173053.51.noahmisch%40microsoft.com
replace_property_refs() called expression_tree_mutator() with the root
of the expression tree as the input node. But
expression_tree_mutator() does not call the mutator function on the
root node, so the root node remains unchanged. If the root node is a
property reference or a lateral reference -- the two node kinds that
replace_property_refs_mutator() rewrites -- it is returned unchanged.
Modules after the rewriter do not know about property reference nodes,
resulting in "ERROR: unrecognized node type: 63". Since varlevelsup
of lateral references is not incremented, they are not resolved
correctly in the planner, leading to many different symptoms. Fix
this by calling replace_property_refs_mutator() directly from
replace_property_refs(), similar to how other mutator functions do.
The only case when a property reference or a lateral reference can be
the root of a GRAPH_TABLE expression tree is when it is a bare
property reference or a bare lateral reference in the WHERE clause.
The COLUMNS clause is passed to replace_property_refs() as a
targetlist. Every other expression has at least one expression node
covering the property reference or a lateral reference in the
expression tree. That explains why this bug was not seen so far.
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://www.postgresql.org/message-id/flat/20260630173053.51.noahmisch%40microsoft.com
Commit aa606b9316 disallowed generated columns in COPY FROM WHERE
expressions, and commit 21c69dc73f disallowed system columns.
However, the COPY reference page still mentions only the restriction
on subqueries.
Update the documentation to also list generated columns and system
columns as unsupported in COPY FROM WHERE expressions.
Backpatch the generated-column documentation change to all supported
versions. Backpatch the system-column documentation change to v19,
where that restriction was introduced.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwEgxErc54yVOAVWCsr1O=8pgw4oKRPuEQ9mfhkoYGR_XA@mail.gmail.com
Backpatch-through: 14
Commit dcb0049523 accidentally changed the final expanded query's
condition to > 2 while rewriting the example into SQL operator notation.
The original query and the preceding rewritten forms all use >= 2,
and view expansion should preserve that qualification. This commit
changes the final condition from > 2 to >= 2.
Backpatch to all supported versions.
Reported-by: Yaroslav Saburov <y.saburov@gmail.com>
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/178248467618.108999.9966122434342474006@wrigleys.postgresql.org
Backpatch-through: 14
