The tests added by this commit cover a large ground in terms of the
introduction of oid8 in TOAST tables, with the following areas covered,
with all the gaps found during the development of the feature:
- Tables with oid8 TOAST tables in general, under strings.sql.
- ALTER TABLE with attribute manipulations in transactions and different
toast_table_value settings.
- Relation rewrites, not affecting the TOAST relation after initial
creation with VACUUM FULL, CLUSTER, REPACK.
- pg_column_compression().
- test_decoding, for external and extended TOAST data.
- amcheck, external and extended storage with heap checked.
- UPDATE with out-of-line datums that belongs to another TOAST table.
This offers coverage for toast_tuple_init(), where external vartags
could be mixed once multiple on-disk external TOAST pointers can
co-exist. This pattern can be triggered with asan/ubsan, with an
out-of-bound memcpy when unpatched.
These tests are added before the new vartag_external, mostly as a matter
of avoiding dead code in the tree.
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Greg Burd <greg@burd.me>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/af19kUjwjhaoUTLn@paquier.xy
This commit adds the possibility to define TOAST tables with oid8 as
their chunk_id, based on the reloption toast_value_type. All the
external TOAST pointers still rely on varatt_external_oid and a single
vartag. The values inserted in the oid8 TOAST tables are fed from the
OID8 value generator, casted to OID for now, as we do not have a
vartag_external able to store Oid8 values yet.
An upcoming commit will add support for a new vartag_external and its
associated structures, with the code being able to use a different
on-disk external TOAST pointer depending on the attribute type of
chunk_id defined in TOAST relations.
All the changes done here are mechanical, mostly around the TOAST code
that needs to be able to do chunk_id lookups based on the two types now
supported.
Bump catalog version.
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Greg Burd <greg@burd.me>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/af19kUjwjhaoUTLn@paquier.xy
To import attribute statistics, postgres_fdw sent a query wity COLLATE
"C" to remote servers without checking their version. Since COLLATE is
only supported since v9.1, this could break the import against older
servers; fix by modifying postgres_fdw to check their version and if
older, fall back to sampling. (This commit still allows importing only
relation statistics from such an older server.)
Also, expand the documentation to mention that attribute statistics are
stored into the local server without applying the foreign table columns'
n_distinct option.
Oversight in commit 28972b6fc.
Reported-by: Fujii Masao <masao.fujii@gmail.com>
Reported-by: Osama Abdul Qader <osamaabdulqader.cs@gmail.com>
Author: Etsuro Fujita <etsuro.fujita@gmail.com>
Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwH8%2BuDTwfohpck9v_9L3nwSWACbmC36JYRyqHd5xQN_OA%40mail.gmail.com
Backpatch-through: 19
This is a mechanical update to change most of the TOAST code paths that
assume TOAST value IDs to be Oid variables to become Oid8 variables,
easing an the introduction of an upcoming patch to allow larger TOAST
values, at 8 bytes.
The areas touched by the change are related to table AM, amcheck and
logical decoding's reorder buffer. A good chunk of the changes involve
switching printf() markers from %u to OID8_FORMAT.
For the sake of the release notes in terms of compatibility (perhaps
worth mentioning, or not), note that this changes some callback
definitions in the table AM section, upgrading some TOAST callbacks to
use Oid8 instead of Oid when referring to a TOAST value.
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Greg Burd <greg@burd.me>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/af19kUjwjhaoUTLn@paquier.xy
The replica identity determination was "flawed": it fell back to the
primary key if the marked index was dropped, which is a defensible
choice, but logical decoding has a differing opinion and fails to
provide usable tuple identity data in this case. "Fix" REPACK by
refusing to use the primary key in that case.
This is arguably broken behavior in RelationGetIndexList, but I'll
refrain from changing that at this stage.
Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Author: Matthias van de Meent <boekewurm+postgres@gmail.com>
Author: Ewan Young <kdbase.hack@gmail.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/apCBRKCH8jwKiaSY@nathan
The general nature of this interaction has been understood (at least
by me) since before the original commit, but I failed to explicitly
document it. I also left behind an XXX in the README which I intended
to remove before commit. Fix those things.
Reported-by: Noah Misch <noah@leadboat.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
Commit 0ee83dd4a9 allowed postgres_fdw to push down inner joins between
foreign tables and functions in FROM clauses to the remote server.
However, planning such a join with use_remote_estimate enabled could
cause a segmentation fault. For example, the following query crashed
when ft had use_remote_estimate set to true:
EXPLAIN SELECT * FROM generate_series(1, 3) g JOIN ft ON ft.i = g;
The crash occurred because init_func_stub_fpinfo() copied the connection
information and options from the foreign side to the function-side stub,
but omitted the user mapping, leaving it NULL. When the function was the
outer side of the join, the join inherited this NULL mapping and passed it
to GetConnection() when requesting a remote estimate, causing a
segmentation fault.
Fix this by having init_func_stub_fpinfo() also copy the user mapping
from the foreign side.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
Discussion: https://postgr.es/m/CAHGQGwGE9xPzCke0C0r4omTkh-hU08aBd2j9oSA0FYmJLCf9jA@mail.gmail.com
Without CHECK_FOR_INTERRUPTS(), backends might not respond to query
cancellations in a timely fashion, especially for JOIN_ORDER()
specifications, where matching against each join can take O(n^2) time
in the length of the join order list in degenerate cases.
Recursive functions should check_stack_depth().
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
Backpatch-through: 19
Commit 86f7c82cf1 reported the user catalog table restriction as a
detail, but the other checks in check_concurrent_repack_requirements()
were still a mix: some were hints, and the details repeated
"REPACK (CONCURRENTLY)", which the primary message already names.
Report them as details with the same "This operation ..." wording. The
one actionable hint, ALTER TABLE ... REPLICA IDENTITY USING INDEX,
stays.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Suggested-by: Alvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/apBIFWzHYOaG0auN%40nathan
Backpatch-through: 19
Previously, running REPACK (CONCURRENTLY) on a materialized view failed
with a confusing "has no identity index" error.
REPACK (CONCURRENTLY) replays changes decoded from WAL, but a
materialized view produces none, since it cannot be modified by DML and
REFRESH rewrites it through a transient heap that is not logically
decoded.
Fix this by reporting a clear error for materialized views upfront,
alongside the existing checks for system catalogs and TOAST tables, and
document the limitation.
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Kiran Kaki <itskkpg@gmail.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CALj2ACXPvy4_LkQ0nHKgbZjAGK9fkDqRYUvXA3hgwxPm_X7Eng@mail.gmail.com
REPACK (CONCURRENTLY) is not an MVCC-safe operation; it doesn't
preserve the visibility information, which logical decoding needs
because it reads user catalog tables under a historic snapshot.
Disallow REPACK (CONCURRENTLY) on user catalog tables. Removing this
check requires making it MVCC-safe and logical rewrite mappings.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/apBIFWzHYOaG0auN%40nathan
Backpatch-through: 19
There's not really any good reason to store an empty advice string
in an advice stash, but it's entirely possible that some people might
do it by accident. If they do that without this fix, restarting the
system will result in failure while reloading pg_stash_advice.tsv.
The root cause is that I (rhaas) did not think clearly enough when
defining pgsa_next_tsv_field(). After returning each field on the
line, it set *cursor to the start of the next field, and after
returning all of them, it set *cursor to the trailing NUL byte. But
when the last field on the line is empty, this is ambiguous: if we're
pointing at the trailing NUL byte, it could be either because it's the
start of the zero-length field or because we're out of fields. To
remove the ambiguity, this commit decides that *cursor will instead
be set to NULL when there are no more fields.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260827171830.68.noahmisch@microsoft.com
Backpatch-through: 19
This fixes issues with the same variable name being used in the same
function for two different variables. (In some cases, one of the uses
is a function argument.) The fix is in most cases to rename one or
both of them. The individual choice depends on conventions in the
surrounding code. Often, making both variable names more specific is
the best choice. In a few cases, the fix is to move one of the
variables to a lower scope, so that it no longer conflicts with the
other.
Since these are all local-variable conflicts, the changes in different
files in this patch are independent of each other.
These issues correspond to the gcc warning option -Wshadow=local,
which is not currently used but could be activated after this.
Reviewed-by: Peter Geoghegan <pg@bowt.ie>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/84b6f128-91f5-480e-8a9e-7d0e8f538cea%40eisentraut.org
List of commits reverted:
2f094e7ac6 SQL Property Graph Queries (SQL/PGQ)
cd8844e7db Make some tests more stable by adding more explicit ordering
182cdf5aea pg_dump: Add appropriate version check
c9babbc881 Dump labels in reproducible order
040a56be4b Cleanup users and roles in graph_table_rls test
5282bf535e Fix some typos and make small stylistic improvements
720f0f89d6 Reject consecutive element patterns of same kind
288ae96872 Add a graph pattern variable only once
c5b3253b8a Property references are preferred over regular column references
a0dd0702e4 Fix cross variable references in graph pattern causing segfault
2ff289d039 Check for stack overflow when rewriting graph queries
9082680c34 Fix typos and grammar in graph table rewrite code
ac3bcc041c Fix collation of expressions in GRAPH_TABLE COLUMNS clause
9d2979dd68 pg_get_viewdef() and lateral references in COLUMNS of GRAPH_TABLE
891a57c739 Do not define type for a property graph
dc9e7c9ed9 Handle nodes that may appear in GraphPattern expression trees
f6edd8ed70 Add ORDER BY to test query to stabilize test
1190f858ea doc: Small synopsis wording change for consistency
d0eac3cafb Make spelling consistent
6827de95ee Simplify code in objectaddress.c for some property graph objects
5778fb3eaf Fix typo in error message
72498a8698 Handle element label and label property objects in object address functions
4cb2a9863d Fix LATERAL references in GRAPH_TABLE with multi-label pattern
9d8cdcbe0c Record dependencies on graph labels and properties
2a7e95b659 Readable identity strings for property graph objects
7afa11feca Prevent dropping the last label from a property graph element
96418a6da9 Fix handling of dropping a property not associated with the given label
efd7d8d7d4 Resolve unknown-type literals in GRAPH_TABLE COLUMNS
e0ff7fd9aa Make property graph object descriptions better translatable
0e4f0827f6 Fix properties orphaned by dropping a label
73dfe79fd6 Remove apparent support for SECURITY LABEL ON PROPERTY GRAPH
592de8bd21 Fix pg_dump ACL minimization for PROPERTY GRAPH.
16a4b3ef8e Fix replace_property_refs() ignoring the root of expression tree
57f93af36f Resolve unknown-type literals in property expressions
2e6578292a Prohibit locking clauses on GRAPH_TABLE
2521f9e75e doc: Fix SQL quoting in example
0316593146 Restore the ability to use | and -> as prefix operators.
780fa49746 Fix missing space before WHERE in GRAPH_TABLE deparse
fa62d4f6ea Prohibit GRANT ... ON TABLE on a property graph
63c3bbb39f doc: Add PROPERTY GRAPH to the access privilege tables
0a51a06cdb More tab-completion for DROP PROPERTY GRAPH
f585671055 Disallow aggregates, window functions, and SRFs in GRAPH_TABLE COLUMNS
9b917a9311 Report duplicate property and label names with a proper error
a82e33a83d Make generate_queries_for_path_pattern_recurse() interruptible
3c982c9bf1 Add test for view referencing labels shared by vertex and edge tables
8646d1d590 Fix inferred property graph keys with INCLUDE columns
0417bf5296 Fix pg_event_trigger_ddl_commands for GRANT ON PROPERTY GRAPH
a20d3e35f2 doc: reformat GRAPH_TABLE examples
792094a5ce Coerce GRAPH_TABLE pattern WHERE clauses to boolean
plus typedefs.list changes.
Discussion: https://www.postgresql.org/message-id/CAAKRu_bEtjWYWhYxSo0o_t3DaZYRQd5PkC0abA0g9Mp4%2BovH0w%40mail.gmail.com
When analyzing a foreign table pointing to a partitioned or inherited
table on the remote server, this method just imported relpages and
reltuples for the parent table from the remote pg_class, causing a
discrepancy or an error in results compared to the sampling method.
* In the partitioning case, while the sampling method sets relpages=0,
this method set relpages=-1. (For reltuples, the results of both
methods match.)
* In the inheritance case, while the sampling method sets
reltuples=(# of rows in the parent and child tables), this method set
reltuples=(# of rows in the parent table). (For relpages, the results
of both methods match.)
As relpages and reltuples are used for costing foreign paths, these
would lead to incorrect plans or plan changes depending on the method.
To fix, do the following:
* For the partitioning case, modify postgres_fdw to set relpages=0 to
match the sampling method.
* For the inheritance case, we could also modify it to set the correct
reltuples value by fetching reltuples for the child tables as well,
but 1) the change isn't small, and 2) table inheritance hasn't been
used that much these days, so fix by disabling this method.
While at it, adjust comments and docs slightly.
Oversight in commit 28972b6fc.
Author: Etsuro Fujita <etsuro.fujita@gmail.com>
Discussion: https://postgr.es/m/CAPmGK173b2Xvyoeq%3DY7L09H%2BajgMgxC2fYe%3D7CCunZz8tGTfFA%40mail.gmail.com
Backpatch-through: 19
This refactoring eases a little bit a follow-up change to add support
for 8-byte TOAST value IDs, as the maximum chunk size allowed for a
single chunk of TOASTed data depends on the size of varatt_external, its
value ID being part of it.
It is worth noting that TOAST_MAX_CHUNK_SIZE resolves as a uintptr_t, so
the previous calculations were in 64-bit unsigned arithmetic. These are
now done in int32, which still works the same way due to an external
TOAST value being bounded to 1GB.
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/CALj2ACVnO6jjs5aRx3dSOBy9LV7=iakPBR1eOvoeB8rL8Q47zg@mail.gmail.com
This reverts commit cc9a8eb112, after more discussion with Etsuro
Fujita. We are still not sure about the interface that should be used
for the portion related to the statistics import, and what should be
exposed for FDW code that may want to rely on these two functions:
- import_attribute_statistics()
- import_relation_statistics()
Discussion: https://postgr.es/m/CAPmGK15HUuNvEZk4b+K_x57+sg31B9LDwB7DBgVNaCpLnafhPA@mail.gmail.com
Commit 0ee83dd4a9 detected the mixed foreign x function-RTE cases in one
if/else chain that only set a pair of bool flags, then acted on those flags in
a second chain. fpinfo_o and fpinfo_i are each assigned in one chain and read
in the other, so proving them initialized requires correlating the flags with
the assignments. gcc 13 with -Og does not manage that and reports fpinfo_i as
possibly uninitialized; clang's -Wconditional-uninitialized likewise reports
both variables.
The flags served no purpose beyond deferring the work, so merge the two chains
and read both fdw_private pointers up front, unconditionally. A function rel
never has an fdw_private of its own, so the pointer is simply NULL until the
stub replaces it, and each branch now assigns and uses the two variables in one
place.
Reported-by: Karina Litskevich <litskevichkarina@gmail.com>
Reported-by: Marina Polyakova <m.polyakova@postgrespro.ru>
Reviewed-by: Marina Polyakova <m.polyakova@postgrespro.ru>
Discussion: https://postgr.es/m/CACiT8iamoL-%3D792e6JK2uCVvwDhfCg7DXHhvqN0P33jhGg2-8A%40mail.gmail.com
The statistics import and restore paths built fake FunctionCallInfos
with LOCAL_FCINFO() and InitFunctionCallInfoData(), never calling
anything through them. The only data relevantly used was the values
carried around as NullableDatum arrays, read back with PG_GETARG_*()
macros.
import_relation_statistics() and import_attribute_statistics() were
passing a set of NullablePointer pointers, leading to more round-trip
manipulations with the fake FunctionCallInfos. These are reworked so as
the values pushed to pg_class (for relation stats) and pg_statistic (for
attribute stats) are passed around in dedicated structures with all the
values assigned based on positional indexes (import code cares about
less values than the restore code), simplifying their signatures and
reducing the number of code lines required to pass the data around.
The stats_check_*() functions now take the values and the attribute
names rather than a StatsArgInfo with positional indexes, as this code
is shared between the stats restore and import. This removes the
dependency between the StatsArgInfo bits and the import code.
StatsArgInfo is used only for the restore code, for argument mapping.
No behavioral change should be introduced in this commit; this is
(should be) pure refactoring.
Reported-by: Robert Haas <robertmhaas@gmail.com>
Author: Corey Huinker <corey.huinker@gmail.com>
Author: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CADkLM=eo7MtuCE=YjovW+=ASw1=q39qQ3qarrsw+EKfU901ztA@mail.gmail.com
FOREIGN_JOIN target sublists must contain at least two relation
identifiers. However, the parser checked only for sublists with
exactly one identifier, so FOREIGN_JOIN(()) was accepted.
Reject sublists with fewer than two relation identifiers, and add
regression coverage.
Author: Chao Li <lic@highgo.com>
Discussion: http://postgr.es/m/BEDC04E0-6732-4310-95BA-6EC34BC1442C@gmail.com
It's reasonable to specify that the input string passed to these
functions isn't modified.
In each case, the input string is passed down to CStringGetDatum(),
which already takes a const char *, so this change just pushes that
API specification one level up.
A couple of callers benefit from this by no longer having to cast away
a const qualifier.
A few more functions that call input functions internally are changed
so that they can now offer a const qualified input string to their
callers.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/flat/459217ca-3840-4452-8cd1-8f9b47e8d479%40eisentraut.org
The pg_class fields relpages, relallvisible, and relallfrozen are of
type int32, but the statistics restoration code and some statistics
code in postgres_fdw internally dealt with them as uint32 for a little
bit, after which they would turn back into int32. This doesn't make
sense, so fix it to use int32 consistently throughout.
In passing, change a float to float4 to similarly make the C type
consistent with the SQL type.
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/db993f34-e3e1-4e6e-85c3-4f248d2ff29b%40eisentraut.org
Clearly define case mapping upper bounds that cover all providers,
locales, and encodings: PG_MAX_CASEMAP_MBCHARS,
PG_MAX_CASEMAP_EXPANSION, and PG_CASEMAP_BUFSZ.
Also redefine UTF8_CASEMAP_BUFSZ to include room for a NUL for
consistency with PG_CASEMAP_BUFSZ.
Use PG_CASEMAP_BUFSZ for the static buffer in ltree/crc32.c.
Remove macros UNICODE_CASEMAP_LEN (which was redundant with
UNICODE_MAX_CASEMAP_CODEPOINTS from unicode_limits.h), and
UNICODE_CASEMAP_BUFSZ which was not as descriptive or helpful.
Suggested-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Rithvika Devisetti <devisettirithvika@gmail.com>
Discussion: https://postgr.es/m/2cfbc37ae8deccd3825e36b7f31c391cbf8ff9b8.camel@j-davis.com
Explicitly make space for the NUL when casefolding.
No known bug in the previous code, because the previous buffer (size
12) was more than large enough for folding any codepoint with enough
room left for a NUL. The builtin provider's limit is 7; ICU's limit
seems to be 7 also; and libc always does 1:1 mappings so the real
limit is MAX_MULTIBYTE_CHAR_LEN + 1 (size 5).
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Rithvika Devisetti <devisettirithvika@gmail.com>
Discussion: https://postgr.es/m/2cfbc37ae8deccd3825e36b7f31c391cbf8ff9b8.camel@j-davis.com
Backpatch-through: 18
The unnest() test introduced in 0ee83dd4a9 did not produce a
deterministic EXPLAIN plan output.
The two sides of this test's nested loop join cost about the same, so
which one ended up as the outer wasn't stable and could flip between
platforms.
Use an equality predicate on ft1.c3 instead of a range one. That
restricts ft1 to a single row, so it's clearly the outer of the nested
loop and the Materialize on the inner side goes away, giving a stable
plan.
Author: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/CAA5RZ0uC%3DGOihOKcF7o3JVETF3e-Fkc4OUO5NbqMEKCTeSN-dQ%40mail.gmail.com
Up to now, if you passed a "tied" object to a PL/Perl operation,
it would most likely get treated as "undef", since we failed to
perform get magic on values we're passed. Tied hashes worked
partially, in that you could read out their keys, but the values
always read as undef.
To fix, use hv_iterval() not HeVAL() to fetch values from perl
hashes, and be careful to apply plperl_materialize_sv() before
testing SvOK() or SvROK() on any perl SV.
Add regression test cases exercising some typical usage scenarios
for tied objects.
Arguably this is a bug fix, but in view of the lack of field
complaints to date, let's treat it as a new feature instead.
The risk/reward ratio for back-patching isn't attractive.
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/569769.1786901901@sss.pgh.pa.us
A foreign join planning hook now considers a (foreign-table x function-RTE)
INNER join as a push-down candidate when the function expression is
IMMUTABLE and otherwise shippable. The remote query absorbs the function
call as a FROM-list item (e.g. unnest(...) AS f<rti>(c1, c2, ...)), so the
foreign side returns only rows that match the function-produced set and the
join executes entirely on the remote.
An IMMUTABLE function gives the same result on any server, so the same
function RTE can be a push-down candidate for several distinct foreign
servers without semantic risk. To keep the planner state consistent
across those independent attempts, the per-call stub fpinfo for the
function side lives on the joinrel's PgFdwRelationInfo (new
outer_func_fpinfo / inner_func_fpinfo), never on the function rel itself,
and the function side is detected via rtekind rather than fdw_private.
set_foreign_rel_properties() propagates fdwroutine onto a joinrel that
pairs a foreign rel with an RTE_FUNCTION rel so GetForeignJoinPaths gets
called; the FDW retains full control over whether to actually generate a
path. deparseRangeTblRef and deparseColumnRef gain a FUNCTION-RTE branch
that emits the function expression and resolves Vars to the generated
column aliases.
Discussion: https://postgr.es/m/e3af56f9b2f1bd9fdf12aff6ca25b18d%40postgrespro.ru
Author: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
Reviewed-by: Solaimurugan Vellaipandiyan <drsolaimurugan.v@gmail.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Make hek2cstr() available in plperl.h, so that it can be used
in hstore_plperl and jsonb_plperl. Those modules were previously
using different coding techniques that probably don't get
conversion from Perl strings to the database encoding quite right.
(I'd prefer to make hek2cstr() non-inline, but cross-extension calls
are messy and plperl has avoided them up to now, so stick with the
existing approach.)
Consistently use hv_iternext, hek2cstr, and HeVAL for hash iterations,
with one exception in plperl_trusted_init: there, hv_iternextsv is
fine since we don't actually care about the hash keys. (A later
patch will remove the HeVAL calls again, but for now we just want
consistency.)
Remove unnecessary extra calls of hv_iterinit.
Remove duplicative pstrdup's in plperl_to_hstore.
The meat of this change is to use hek2cstr() in the contrib modules,
which makes a user-visible change in encoding conversion behavior.
While it's certainly a bug fix, we've had no field complaints about
those modules, so I'm hesitant to make this change in the back
branches. Hence, apply to master only.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Discussion: https://postgr.es/m/569769.1786901901@sss.pgh.pa.us
Tied hashes, and probably tied arrays, are capable of returning Perl
value pointers that are actually NULL, not the usual pointer to an
undef SV. We were not defending against that everywhere, leading
to possible SIGSEGV. Fix the code to consistently treat a null
pointer returned from hv_iternext or av_fetch like a !SvOK one.
(Note that the large diff in SV_to_JsonbValue is actually quite
trivial, but it required reindenting a chunk of existing code.)
Claude Code found the instance in hstore_plperl, and I found the
others by code auditing. Perhaps the other instances aren't
actually reachable, but I see little reason to assume that.
The known test cases for these errors require perl's Tie modules,
which may not be present, so it doesn't seem worth the trouble
to create regression test cases that would cover them.
Reported-by: Claude Code (via Noah Misch)
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/569769.1786901901@sss.pgh.pa.us
Backpatch-through: 14
Virtual generated columns are stored in tuples as null values, so
heap_getattr() in tuple_to_stringinfo() always returns null for them.
test_decoding printed such a column as null, which is
indistinguishable from a column that genuinely holds null.
This commit skips virtual generated columns in tuple_to_stringinfo(),
the only place that prints a tuple. Stored generated columns continue
to be printed, since their values do live in the heap tuple.
This is a behavior change rather than a bug fix, and it changes
test_decoding's output for tables that have virtual generated columns,
which could affect consumers that parse it. Therefore it is not
back-patched.
Author: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
Co-authored-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Euler Taveira <euler@eulerto.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAHg+QDfTh3UbB-Ed--o2Bd=SBDJoEiG-qp3C0+ETDibF63y=dw@mail.gmail.com
Commit 072ee847ad taught logical decoding to discard the changes of a
transaction that is already known to be aborted when it is picked for
eviction. That path reuses ReorderBufferTruncateTXN(), which marks
every subtransaction that still has in-memory changes as
streamed. Since nothing is streamed in that path, and the top-level
transaction is never marked, a subtransaction ends up flagged as
streamed even though the output plugin has never seen it. Decoding the
subsequent abort record then makes ReorderBufferAbort() invoke the
stream_abort callback for that subtransaction.
For pgoutput this sends a Stream Abort ('A') message to a subscriber
that requested streaming = off, and it does so regardless of the
negotiated protocol version, so even a client speaking a version that
predates transaction streaming receives a message it cannot
parse. test_decoding dereferences a NULL pointer and crashes, since it
allocates its per-transaction state in the begin or stream start
callback, neither of which runs for a transaction discarded as
aborted.
This commit fixes this by marking a subtransaction as streamed only
when it has changes and its top-level transaction is already marked as
streamed. All streaming call sites mark the top-level transaction
before truncating it, so their behavior is unchanged, while the
abort-discard path never marks the top-level transaction and therefore
now leaves its subtransactions unmarked.
Backpatch to v18, where commit 072ee847ad was introduced.
Bug: #19616
Reported-by: Tyler Smart <tyler@smarts.io>
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/19616-f6153af509910853@postgresql.org
Backpatch-through: 18
hstore has been relying on a set of int/int32 to count the total length
of all the keys and values constructed, relying on the maximum values of
the arguments rather than proper bound checks. As proved in the
reported thread, these counters could overflow and wrap, leading to
incorrect allocations.
This commit replaces the int/int32 values with a set of Size values,
with add_size() and mul_size() in charge of checking for overflows when
constructing hstore values before doing any allocation.
The changes that matter in terms of the report are mostly in
hstore_io.c. hstore_compat.c is adjusted for consistency, to keep the
allocation maths uniform across the module.
This is an old bug. A backpatch should perhaps be done but I cannot get
excited about a change like that in stable branches.
Reported-by: Man Zeng <zengman@halodbtech.com>
Author: Tender Wang <tndrwang@gmail.com>
Author: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAHewXNmfD6-9MDxgpRfTzjVj0mhBw3JMoNOHeTiPOjZGhNzd=g@mail.gmail.com
plperl_func_handler()'s stanza for handling an arrayref result in
a SETOF function could loop forever (or at least till OOM) when
given a tied array, since av_fetch won't necessarily ever return
a null pointer in that case. Be consistent with the other places
where we traverse a perl array: call av_len() once and use len+1
as the loop limit, silently ignoring any null pointers we get back
from that range of subscripts.
But actually, Perl's preferred locution for this seems to be to
use av_count() not av_len()+1. av_count() seems better since
there's less risk of forgetting to add 1. Also, both of those
functions return Size_t (or SSize_t) not int, creating at least
a theoretical overflow hazard. While we're modernizing this,
let's use the correct variable type where we can, and include an
overflow check where we can't.
Reported-by: Claude Code (via Noah Misch)
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Discussion: https://postgr.es/m/569769.1786901901@sss.pgh.pa.us
Backpatch-through: 14
This is part of a more global effort to modernize the code tree with
safer allocation functions, which offer more protection in terms of:
- stronger type safety guarantees
- size overflow checks, particulary for arrays.
This set of updates includes allocation changes based on
palloc_object(), palloc_array() and repalloc_array(). This work is
similar to 1b105f9472, expanding more the new APIs in the tree in an
incremental fashion.
Author: Tristan Partin <tristan@partin.io>
Discussion: https://postgr.es/m/DKJB5887LGX9.359DV1UMZ1URK@partin.io
Simple EXISTS subplans can be converted to hashed ANY subplans as
an alternative implementation. Previously, both the original EXISTS
subplan and the converted ANY subplan used names with the
exists_ prefix, making EXPLAIN output harder to read and
pg_plan_advice targets less clear.
Use the exists_to_any_ prefix for converted ANY subplans so that
their names distinguish them from the original EXISTS alternative.
This only changes the names shown by EXPLAIN and used by plan advice;
it does not affect planner behavior.
Author: Yugo Nagata <nagata@sraoss.co.jp>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: solai v <solai.cdac@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/20260605165641.3950f99ace0aad8f807abe96@sraoss.co.jp
Backpatch-through: 19
Commit 13b935cd changed hash_get_num_entries to return int64 instead
of long. This fixes a few more users of hash_get_num_entries which
were missed in the original commit.
PGSS_FILE_HEADER is left unchanged since this only affects as of yet
unreleased versions of PostgreSQL. Backpatch to v19 where the int64
change was performed.
Author: Karina Litskevich <litskevichkarina@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CACiT8iYTkc33YWaA2D3t51Y5s=GqBO7T1zX7bkpSmet2njcLLw@mail.gmail.com
Backpatch-through: 19
