Commit Graph

4388 Commits

Author SHA1 Message Date
Tomas Vondra 211d80c065 Fix stale comment about sample_frac adjustment
A comment was left behind referencing sample rate adjustment removed
from 8ad51b5f44. So clean that up. While at it also remove the sample
rate clamping which should not be necessary without the clamping, and
just check that with an assert.

Reported-by: Tom Lane
Discussion: https://postgr.es/m/951485.1672461744%40sss.pgh.pa.us
2023-01-06 14:47:23 +01:00
Bruce Momjian c8e1ba736b Update copyright for 2023
Backpatch-through: 11
2023-01-02 15:00:37 -05:00
Tom Lane 1fd3dd2048 Add bt_multi_page_stats() function to contrib/pageinspect.
This is like the existing bt_page_stats() function, but it can
report on a range of pages rather than just one at a time.

I don't have a huge amount of faith in the portability of the
new test cases, but they do pass in a 32-bit FreeBSD VM here.
Further adjustment may be needed depending on buildfarm results.

Hamid Akhtar, reviewed by Naeem Akhter, Bertrand Drouvot,
Bharath Rupireddy, and myself

Discussion: https://postgr.es/m/CANugjht-=oGMRmNJKMqnBC69y7vr+wHDmm0ZK6-1pJsxoBKBbA@mail.gmail.com
2023-01-02 13:02:29 -05:00
Tomas Vondra 8ad51b5f44 Sample postgres_fdw tables remotely during ANALYZE
When collecting ANALYZE sample on foreign tables, postgres_fdw fetched
all rows and performed the sampling locally. For large tables this means
transferring and immediately discarding large amounts of data.

This commit allows the sampling to be performed on the remote server,
transferring only the much smaller sample. The sampling is performed
using the built-in TABLESAMPLE methods (system, bernoulli) or random()
function, depending on the remote server version.

Remote sampling can be enabled by analyze_sampling on the foreign server
and/or foreign table, with supported values 'off', 'auto', 'system',
'bernoulli' and 'random'. The default value is 'auto' which uses either
'bernoulli' (TABLESAMPLE method) or 'random' (for remote servers without
TABLESAMPLE support).
2022-12-30 23:16:01 +01:00
Andrew Dunstan 203942243c Fix oversight in 7a05425d96
This patch was changed as a result of review but one line didn't get the
message. Mea Culpa.
2022-12-29 07:16:58 -05:00
Andrew Dunstan 7a05425d96 Convert contrib/ltree's input functions to report errors softly
Reviewed by Tom Lane and Amul Sul

Discussion: https://postgr.es/m/49e598c2-cfe8-0928-b6fb-d0cc51aab626@dunslane.net
2022-12-28 10:00:12 -05:00
Andrew Dunstan 3b76622e04 Convert contrib/intarray's bqarr_in() to report errors softly
Reviewed by Tom Lane and Amul Sul

Discussion: https://postgr.es/m/49e598c2-cfe8-0928-b6fb-d0cc51aab626@dunslane.net
2022-12-28 10:00:12 -05:00
Tom Lane 197f98a848 Convert hstore_in to report errors softly.
The error reporting here was not only old and crufty, but untested.
I took the opportunity to bring the messages into some sort of
compliance with our message style guidelines.

Discussion: https://postgr.es/m/6B6A5C77-60AD-4A71-9F3A-B2C026A281A6@dunslane.net
2022-12-27 14:50:56 -05:00
Andrew Dunstan 361ec4368b Fix thinko in 720e0327bc 2022-12-25 04:36:58 -05:00
Andrew Dunstan 720e0327bc Convert contrib/isn's input functions to report errors softly 2022-12-24 15:28:13 -05:00
Andrew Dunstan 878ce16056 Convert contrib/seg's input function to report errors softly
Reviewed by Tom Lane

Discussion: https://postgr.es/m/a8dc5700-c341-3ba8-0507-cc09881e6200@dunslane.net
2022-12-23 09:17:24 -05:00
David Rowley bbfdf7180d Fix bug in translate_col_privs_multilevel
Fix incorrect code which was trying to convert a Bitmapset of columns at
the attnums according to a parent table and transform them into the
equivalent Bitmapset with same attnums according to the given child table.
This code is new as of a61b1f748 and was failing to do the correct
translation when there was an intermediate parent table between 'rel' and
'top_parent_rel'.

Reported-by: Ranier Vilela
Author: Richard Guo, Amit Langote
Discussion: https://postgr.es/m/CAEudQArohfB_Gy%2BhcH2-bANUkxgjJiP%3DABq01_LgTNTbcNijag%40mail.gmail.com
2022-12-24 00:58:34 +13:00
Tom Lane 701c881f78 Fix contrib/seg to be more wary of long input numbers.
seg stores the number of significant digits in an input number
in a "char" field.  If char is signed, and the input is more than
127 digits long, the count can read out as negative causing
seg_out() to print garbage (or, if you're really unlucky,
even crash).

To fix, clamp the digit count to be not more than FLT_DIG.
(In theory this loses some information about what the original
input was, but it doesn't seem like useful information; it would
not survive dump/restore in any case.)

Also, in case there are stored values of the seg type containing
bad data, add a clamp in seg_out's restore() subroutine.

Per bug #17725 from Robins Tharakan.  It's been like this
forever, so back-patch to all supported branches.

Discussion: https://postgr.es/m/17725-0a09313b67fbe86e@postgresql.org
2022-12-21 17:51:50 -05:00
Michael Paquier 22e3b55805 Switch some system functions to use get_call_result_type()
This shaves some code by replacing the combinations of
CreateTemplateTupleDesc()/TupleDescInitEntry() hardcoding a mapping of
the attributes listed in pg_proc.dat by get_call_result_type() to build
the TupleDesc needed for the rows generated.

get_call_result_type() is more expensive than the former style, but this
removes some duplication with the lists of OUT parameters (pg_proc.dat
and the attributes hardcoded in these code paths).  This is applied to
functions that are not considered as critical (aka that could be called
repeatedly for monitoring purposes).

Author: Bharath Rupireddy
Reviewed-by: Robert Haas, Álvaro Herrera, Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACV23HW5HP5hFjd89FNS-z5X8r2jNXdMXcpN2BgTtKd87w@mail.gmail.com
2022-12-21 10:11:22 +09:00
Andrew Dunstan 8284cf5f74 Add copyright notices to meson files
Discussion: https://postgr.es/m/222b43a5-2fb3-2c1b-9cd0-375d376c8246@dunslane.net
2022-12-20 07:54:39 -05:00
Etsuro Fujita 594f8d3776 Allow batching of inserts during cross-partition updates.
Commit 927f453a9 disallowed batching added by commit b663a4136 to be
used for the inserts performed as part of cross-partition updates of
partitioned tables, mainly because the previous code in
nodeModifyTable.c couldn't handle pending inserts into foreign-table
partitions that are also UPDATE target partitions.  But we don't have
such a limitation anymore (cf. commit ffbb7e65a), so let's allow for
this by removing from execPartition.c the restriction added by commit
927f453a9 that batching is only allowed if the query command type is
CMD_INSERT.

In postgres_fdw, since commit 86dc90056 changed it to effectively
disable cross-partition updates in the case where a foreign-table
partition chosen to insert rows into is also an UPDATE target partition,
allow batching in the case where a foreign-table partition chosen to
do so is *not* also an UPDATE target partition.  This is enabled by the
"batch_size" option added by commit b663a4136, which is disabled by
default.

This patch also adjusts the test case added by commit 927f453a9 to
confirm that the inserts performed as part of a cross-partition update
of a partitioned table indeed uses batching.

Amit Langote, reviewed and/or tested by Georgios Kokolatos, Zhihong Yu,
Bharath Rupireddy, Hou Zhijie, Vignesh C, and me.

Discussion: http://postgr.es/m/CA%2BHiwqH1Lz1yJmPs%3DaD-pzd_HLLynLHvq5iYeT9mB0bBV7oJ6w%40mail.gmail.com
2022-12-20 19:05:00 +09:00
Bruce Momjian 59346209a8 C comment: fix wording
Backpatch-through: master
2022-12-16 12:15:54 -05:00
David Rowley 4a29eabd1d Remove pessimistic cost penalization from Incremental Sort
When incremental sorts were added in v13 a 1.5x pessimism factor was added
to the cost modal.  Seemingly this was done because the cost modal only
has an estimate of the total number of input rows and the number of
presorted groups.  It assumes that the input rows will be evenly
distributed throughout the presorted groups.  The 1.5x pessimism factor
was added to slightly reduce the likelihood of incremental sorts being
used in the hope to avoid performance regressions where an incremental
sort plan was picked and turned out slower due to a large skew in the
number of rows in the presorted groups.

An additional quirk with the path generation code meant that we could
consider both a sort and an incremental sort on paths with presorted keys.
This meant that with the pessimism factor, it was possible that we opted
to perform a sort rather than an incremental sort when the given path had
presorted keys.

Here we remove the 1.5x pessimism factor to allow incremental sorts to
have a fairer chance at being chosen against a full sort.

Previously we would generally create a sort path on the cheapest input
path (if that wasn't sorted already) and incremental sort paths on any
path which had presorted keys.  This meant that if the cheapest input path
wasn't completely sorted but happened to have presorted keys, we would
create a full sort path *and* an incremental sort path on that input path.
Here we change this logic so that if there are presorted keys, we only
create an incremental sort path, and create sort paths only when a full
sort is required.

Both the removal of the cost pessimism factor and the changes made to the
path generation make it more likely that incremental sorts will now be
chosen.  That, of course, as with teaching the planner any new tricks,
means an increased likelihood that the planner will perform an incremental
sort when it's not the best method.  Our standard escape hatch for these
cases is an enable_* GUC.  enable_incremental_sort already exists for
this.

This came out of a report by Pavel Luzanov where he mentioned that the
master branch was choosing to perform a Seq Scan -> Sort -> Group
Aggregate for his query with an ORDER BY aggregate function.  The v15 plan
for his query performed an Index Scan -> Group Aggregate, of course, the
aggregate performed the final sort internally in nodeAgg.c for the
aggregate's ORDER BY.  The ideal plan would have been to use the index,
which provided partially sorted input then use an incremental sort to
provide the aggregate with the sorted input.  This was not being chosen
due to the pessimism in the incremental sort cost modal, so here we remove
that and rationalize the path generation so that sort and incremental sort
plans don't have to needlessly compete.  We assume that it's senseless
to ever use a full sort on a given input path where an incremental sort
can be performed.

Reported-by: Pavel Luzanov
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/9f61ddbf-2989-1536-b31e-6459370a6baa%40postgrespro.ru
2022-12-16 15:22:23 +13:00
Peter Eisentraut 4cb65e1072 basebackup_to_shell: Add some const qualifiers for consistency 2022-12-13 10:39:44 +01:00
Tom Lane d02ef65bce Standardize error reports in unimplemented I/O functions.
We chose a specific wording of the not-implemented errors for
pseudotype I/O functions and other cases where there's little
value in implementing input and/or output.  gtsvectorin never
got that memo though, nor did most of contrib.  Make these all
fall in line, mostly because I'm a neatnik but also to remove
unnecessary translatable strings.

gbtreekey_in needs a bit of extra love since it supports
multiple SQL types.  Sadly, gbtreekey_out doesn't have the
ability to do that, but I think it's unreachable anyway.

Noted while surveying datatype input functions to see what we
have left to fix.
2022-12-10 18:26:43 -05:00
Tom Lane 2661469d86 Allow DateTimeParseError to handle bad-timezone error messages.
Pay down some ancient technical debt (dating to commit 022fd9966):
fix a couple of places in datetime parsing that were throwing
ereport's immediately instead of returning a DTERR code that could be
interpreted by DateTimeParseError.  The reason for that was that there
was no mechanism for passing any auxiliary data (such as a zone name)
to DateTimeParseError, and these errors seemed to really need it.
Up to now it didn't matter that much just where the error got thrown,
but now we'd like to have a hard policy that datetime parse errors
get thrown from just the one place.

Hence, invent a "DateTimeErrorExtra" struct that can be used to
carry any extra values needed for specific DTERR codes.  Perhaps
in the future somebody will be motivated to use this to improve
the specificity of other DateTimeParseError messages, but for now
just deal with the timezone-error cases.

This is on the way to making the datetime input functions report
parse errors softly; but it's really an independent change, so
commit separately.

Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru
2022-12-09 13:30:47 -05:00
Tom Lane ccff2d20ed Convert a few datatype input functions to use "soft" error reporting.
This patch converts the input functions for bool, int2, int4, int8,
float4, float8, numeric, and contrib/cube to the new soft-error style.
array_in and record_in are also converted.  There's lots more to do,
but this is enough to provide proof-of-concept that the soft-error
API is usable, as well as reference examples for how to convert
input functions.

This patch is mostly by me, but it owes very substantial debt to
earlier work by Nikita Glukhov, Andrew Dunstan, and Amul Sul.
Thanks to Andres Freund for review.

Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru
2022-12-09 10:14:53 -05:00
Andres Freund 3f0e786ccb meson: Add 'running' test setup, as a replacement for installcheck
To run all tests that support running against existing server:
$ meson test --setup running

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

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

Discussion: https://postgr.es/m/CAH2-Wz=XDQcmLoo7RR_i6FKQdDmcyb9q5gStnfuuQXrOGhB2sQ@mail.gmail.com
2022-12-07 12:13:35 -08:00
Alvaro Herrera a61b1f7482
Rework query relation permission checking
Currently, information about the permissions to be checked on relations
mentioned in a query is stored in their range table entries.  So the
executor must scan the entire range table looking for relations that
need to have permissions checked.  This can make the permission checking
part of the executor initialization needlessly expensive when many
inheritance children are present in the range range.  While the
permissions need not be checked on the individual child relations, the
executor still must visit every range table entry to filter them out.

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

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

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

Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqGjJDmUhDSfv-U2qhKJjt9ST7Xh9JXC_irsAQ1TAUsJYg@mail.gmail.com
2022-12-06 16:09:24 +01:00
Alvaro Herrera b5bbaf08ed
Fix typo 2022-12-06 16:08:30 +01:00
Alvaro Herrera 599b33b949
Stop accessing checkAsUser via RTE in some cases
A future commit will move the checkAsUser field from RangeTblEntry
to a new node that, unlike RTEs, will only be created for tables
mentioned in the query but not for the inheritance child relations
added to the query by the planner.  So, checkAsUser value for a
given child relation will have to be obtained by referring to that
for its ancestor mentioned in the query.

In preparation, it seems better to expand the use of RelOptInfo.userid
during planning in place of rte->checkAsUser so that there will be
fewer places to adjust for the above change.

Given that the child-to-ancestor mapping is not available during the
execution of a given "child" ForeignScan node, add a checkAsUser
field to ForeignScan to carry the child relation's RelOptInfo.userid.

Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqGFCs2uq7VRKi7g+FFKbP6Ea_2_HkgZb2HPhUfaAKT3ng@mail.gmail.com
2022-11-30 12:07:03 +01:00
Etsuro Fujita ffbb7e65a8 Fix handling of pending inserts in nodeModifyTable.c.
Commit b663a4136, which allowed FDWs to INSERT rows in bulk, added to
nodeModifyTable.c code to flush pending inserts to the foreign-table
result relation(s) before completing processing of the ModifyTable node,
but the code failed to take into account the case where the INSERT query
has modifying CTEs, leading to incorrect results.

Also, that commit failed to flush pending inserts before firing BEFORE
ROW triggers so that rows are visible to such triggers.

In that commit we scanned through EState's
es_tuple_routing_result_relations or es_opened_result_relations list to
find the foreign-table result relations to which pending inserts are
flushed, but that would be inefficient in some cases.  So to fix, 1) add
a List member to EState to record the insert-pending result relations,
and 2) modify nodeModifyTable.c so that it adds the foreign-table result
relation to the list in ExecInsert() if appropriate, and flushes pending
inserts properly using the list where needed.

While here, fix a copy-and-pasteo in a comment in ExecBatchInsert(),
which was added by that commit.

Back-patch to v14 where that commit appeared.

Discussion: https://postgr.es/m/CAPmGK16qutyCmyJJzgQOhfBq%3DNoGDqTB6O0QBZTihrbqre%2BoxA%40mail.gmail.com
2022-11-25 17:45:00 +09:00
Tom Lane aeaaf520f4 Mark pageinspect's disk-accessing functions as parallel restricted.
These functions have been marked parallel safe, but the buildfarm's
response to commit e2933a6e1 exposed the flaw in that thinking:
if you try to use them on a temporary table, and they run inside
a parallel worker, they'll fail with "cannot access temporary tables
during a parallel operation".

Fix that by marking them parallel restricted instead.  Maybe someday
we'll have a better answer and can reverse this decision.

Back-patch to v15.  To go back further, we'd have to devise variant
versions of pre-1.10 pageinspect versions.  Given the lack of field
complaints, it doesn't seem worth the trouble.  We'll just deem
this case unsupported pre-v15.  (If anyone does complain, it might
be good enough to update the markings manually in their DBs.)

Discussion: https://postgr.es/m/E1ox94a-000EHu-VH@gemulon.postgresql.org
2022-11-21 15:37:10 -05:00
Tom Lane e2933a6e11 Prevent instability in contrib/pageinspect's regression test.
pageinspect has occasionally failed on slow buildfarm members,
with symptoms indicating that the expected effects of VACUUM
FREEZE didn't happen.  This is presumably because a background
transaction such as auto-analyze was holding back global xmin.

We can work around that by using a temp table in the test.
Since commit a7212be8b, that will use an up-to-date cutoff xmin
regardless of other processes.  And pageinspect itself shouldn't
really care whether the table is temp.

Back-patch to v14.  There would be no point in older branches
without back-patching a7212be8b, which seems like more trouble
than the problem is worth.

Discussion: https://postgr.es/m/2892135.1668976646@sss.pgh.pa.us
2022-11-21 10:50:50 -05:00
Peter Eisentraut c727f511bd Refactor aclcheck functions
Instead of dozens of mostly-duplicate pg_foo_aclcheck() functions,
write one common function object_aclcheck() that can handle almost all
of them.  We already have all the information we need, such as which
system catalog corresponds to which catalog table and which column is
the ACL column.

There are a few pg_foo_aclcheck() that don't work via the generic
function and have special APIs, so those stay as is.

I also changed most pg_foo_aclmask() functions to static functions,
since they are not used outside of aclchk.c.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Discussion: https://www.postgresql.org/message-id/flat/95c30f96-4060-2f48-98b5-a4392d3b6066@enterprisedb.com
2022-11-13 09:02:41 +01:00
Peter Eisentraut afbfc02983 Refactor ownercheck functions
Instead of dozens of mostly-duplicate pg_foo_ownercheck() functions,
write one common function object_ownercheck() that can handle almost
all of them.  We already have all the information we need, such as
which system catalog corresponds to which catalog table and which
column is the owner column.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Discussion: https://www.postgresql.org/message-id/flat/95c30f96-4060-2f48-98b5-a4392d3b6066@enterprisedb.com
2022-11-13 08:12:37 +01:00
Noah Misch 30d98e14a8 If wait_for_catchup fails under has_wal_read_bug, skip balance of test.
Test files should now ignore has_wal_read_bug() so long as
wait_for_catchup() is their only known way of reaching the bug.  That's
at least five files today, a number expected to grow over time.  This
commit removes skip logic from three.  By doing so, systems having the
bug regain the ability to catch other kinds of defects via those three
tests.  The other two, 002_databases.pl and 031_recovery_conflict.pl,
have been unprotected.  Back-patch to v15, where done_testing() first
became our standard.

Discussion: https://postgr.es/m/20221030031639.GA3082137@rfd.leadboat.com
2022-11-12 11:19:50 -08:00
Tom Lane 042c9091f0 Produce more-optimal plans for bitmap scans on boolean columns.
The planner simplifies boolean comparisons such as "x = true" and
"x = false" down to "x" and "NOT x" respectively, to have a canonical
form to ease comparisons.  However, if we want to use an index on x,
the index AM APIs require us to reconstitute the comparison-operator
form of the indexqual.  While that works, in bitmap indexscans the
canonical form of the qual was emitted as a "filter" condition
although it really only needs to be a "recheck" condition, because
create_bitmap_scan_plan didn't recognize the equivalence of that
form with the generated indexqual.  booleq() is pretty cheap so that
likely doesn't make very much difference, but it's unsightly so
let's clean it up.

To fix, add a case to predicate_implied_by() to recognize the
equivalence of such clauses.  This is a relatively low-cost place to
add a check, and perhaps it will have additional use cases in future.

Richard Guo and Tom Lane, per discussion of bug #17618 from Sindy
Senorita.

Discussion: https://postgr.es/m/17618-7a2240bfaa7e84ae@postgresql.org
2022-11-08 10:36:04 -05: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
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
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
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
Amit Kapila 16b1fe0037 Fix assertion failures while processing NEW_CID record in logical decoding.
When the logical decoding restarts from NEW_CID, since there is no
association between the top transaction and its subtransaction, both are
created as top transactions and have the same LSN. This caused the
assertion failure in AssertTXNLsnOrder().

This patch skips the assertion check until we reach the LSN at which we
start decoding the contents of the transaction, specifically
start_decoding_at LSN in SnapBuild. This is okay because we don't
guarantee to make the association between top transaction and
subtransaction until we try to decode the actual contents of transaction.
The ordering of the records prior to the start_decoding_at LSN should have
been checked before the restart.

The other assertion failure is due to the reason that we forgot to track
that we have considered top-level transaction id in the list of catalog
changing transactions that were committed when one of its subtransactions
is marked as containing catalog change.

Reported-by: Tomas Vondra, Osumi Takamichi
Author: Masahiko Sawada, Kuroda Hayato
Reviewed-by: Amit Kapila, Dilip Kumar, Kuroda Hayato, Kyotaro Horiguchi, Masahiko Sawada
Backpatch-through: 10
Discussion: https://postgr.es/m/a89b46b6-0239-2fd5-71a9-b19b1f7a7145%40enterprisedb.com
Discussion: https://postgr.es/m/TYCPR01MB83733C6CEAE47D0280814D5AED7A9%40TYCPR01MB8373.jpnprd01.prod.outlook.com
2022-10-20 08:49:48 +05:30
Tom Lane 8bf66dedd8 Fix confusion about havingQual vs hasHavingQual in planner.
Preprocessing of the HAVING clause will reduce havingQual to NIL
if the clause is constant-TRUE.  This is one case where that
convention is rather unfortunate, because "HAVING TRUE" is not at all
the same as not having any HAVING clause at all.  (Per the SQL spec,
it still forces the query to be grouped.)  The planner deals with this
by having a boolean hasHavingQual that records whether havingQual was
originally nonempty; places that just want to check whether HAVING
was specified are supposed to consult that.

I found three places that got that wrong.  Fortunately, these could
only affect cost estimates not correctness.  It'd be hard even
to demonstrate the errors; for example, the one in allpaths.c would
only matter in a query that has HAVING TRUE but no GROUP BY and no
aggregates, which would require a completely variable-free SELECT
list, making the case probably of only academic interest.  Hence,
while these are worth fixing before someone copies the incorrect
coding somewhere more critical, they don't seem worth back-patching.
I didn't bother trying to devise regression tests, either.

Discussion: https://postgr.es/m/2503888.1666042643@sss.pgh.pa.us
2022-10-18 10:44:34 -04:00
Michael Paquier a19e5cee63 Rename SetSingleFuncCall() to InitMaterializedSRF()
Per discussion, the existing routine name able to initialize a SRF
function with materialize mode is unpopular, so rename it.  Equally, the
flags of this function are renamed, as of:
- SRF_SINGLE_USE_EXPECTED -> MAT_SRF_USE_EXPECTED_DESC
- SRF_SINGLE_BLESS -> MAT_SRF_BLESS
The previous function and flags introduced in 9e98583 are kept around
for compatibility purposes, so as any extension code already compiled
with v15 continues to work as-is.  The declarations introduced here for
compatibility will be removed from HEAD in a follow-up commit.

The new names have been suggested by Andres Freund and Melanie
Plageman.

Discussion: https://postgr.es/m/20221013194820.ciktb2sbbpw7cljm@awork3.anarazel.de
Backpatch-through: 15
2022-10-18 10:22:35 +09:00
Michael Paquier c68a183990 Fix calculation related to temporary WAL segment name in basic_archive
The file name used for its temporary destination, before renaming it to
the real deal, has been using a microseconds in a timestamp aimed to be
originally in milli-seconds.  This is harmless as this is aimed at being
a safeguard against name collisions (note MyProcPid in the name), but
let's be correct with the maths.

While on it, add a note in the module's makefile to document why
installcheck is not supported.

Author: Nathan Bossart
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/20221014044106.GA1673343@nathanxps13
Backpatch-through: 15
2022-10-17 11:40:14 +09:00
Alvaro Herrera cba4e78f35
Disallow MERGE cleanly for foreign partitions
While directly targetting a foreign table with MERGE was already
expressly forbidden, we failed to catch the case of a partitioned table
that has a foreign table as a partition; and the result if you try is an
incomprehensible error.  Fix that by adding a specific check.

Backpatch to 15.

Reported-by: Tatsuhiro Nakamori <bt22nakamorit@oss.nttdata.com>
Discussion: https://postgr.es/m/bt22nakamorit@oss.nttdata.com
2022-10-15 19:24:26 +02:00
Peter Eisentraut 1b11561cc1 Standardize format for printing PIDs
Most code prints PIDs as %d, but some code tried to print them as long
or unsigned long.  While this is in theory allowed, the fact that PIDs
fit into int is deeply baked into all PostgreSQL code, so these random
deviations don't accomplish anything except confusion.

Note that we still need casts from pid_t to int, because on 64-bit
MinGW, pid_t is long long int.  (But per above, actually supporting
that range in PostgreSQL code would be major surgery and probably not
useful.)

Discussion: https://www.postgresql.org/message-id/289c2e45-c7d9-5ce4-7eff-a9e2a33e1580@enterprisedb.com
2022-10-14 08:38:53 +02:00
Daniel Gustafsson ba185d318d Remove redundant memset call following palloc0
This is a follow-up commit to ca7f8e2 which removed the allocation
abstraction from pgcrypto and replaced px_alloc + memset calls with
palloc0 calls. The particular memset in this commit was missed in
that work though.

Author: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: Bruce Momjian <bruce@momjian.us>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/CALNJ-vT5qRucrFMPSzQyAWods1b4MnNPG-M=_ZUzh1SoTh0vNw@mail.gmail.com
2022-10-13 23:18:00 +02:00
Andres Freund 2589434ae0 pg_buffercache: Add pg_buffercache_summary()
Using pg_buffercache_summary() is significantly cheaper than querying
pg_buffercache and summarizing in SQL.

Author: Melih Mutlu <m.melihmutlu@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com>
Discussion: https://postgr.es/m/CAGPVpCQAXYo54Q%3D8gqBsS%3Du0uk9qhnnq4%2B710BtUhUisX1XGEg%40mail.gmail.com
2022-10-13 09:55:46 -07:00
Etsuro Fujita 97da48246d Allow batch insertion during COPY into a foreign table.
Commit 3d956d956 allowed the COPY, but it's done by inserting individual
rows to the foreign table, so it can be inefficient due to the overhead
caused by each round-trip to the foreign server.  To improve performance
of the COPY in such a case, this patch allows batch insertion, by
extending the multi-insert machinery in CopyFrom() to the foreign-table
case so that we insert multiple rows to the foreign table at once using
the FDW callback routine added by commit b663a4136.  This patch also
allows this for postgres_fdw.  It is enabled by the "batch_size" option
added by commit b663a4136, which is disabled by default.

When doing batch insertion, we update progress of the COPY command after
performing the FDW callback routine, to count rows not suppressed by the
FDW as well as a BEFORE ROW INSERT trigger.  For consistency, this patch
changes the timing of updating it for plain tables: previously, we
updated it immediately after adding each row to the multi-insert buffer,
but we do so only after writing the rows stored in the buffer out to the
table using table_multi_insert(), which I think would be consistent even
with non-batching mode, because in that mode we update it after writing
each row out to the table using table_tuple_insert().

Andrey Lepikhov, heavily revised by me, with review from Ian Barwick,
Andrey Lepikhov, and Zhihong Yu.

Discussion: https://postgr.es/m/bc489202-9855-7550-d64c-ad2d83c24867%40postgrespro.ru
2022-10-13 18:45:00 +09:00
Michael Paquier 56c19fee2d Add missing isolation test for test_decoding in meson build
Oversight in 7f13ac8, where catalog_change_snapshot was missing from the
list in meson.build.

Author: Hayato Kuroda
Discussion: https://postgr.es/m/TYAPR01MB58662C932F45A13C6F9BE352F5259@TYAPR01MB5866.jpnprd01.prod.outlook.com
2022-10-13 16:03:01 +09:00
Tom Lane 235eb4db98 Simplify our Assert infrastructure a little.
Remove the Trap and TrapMacro macros, which were nearly unused
and confusingly had the opposite condition polarity from the
otherwise-functionally-equivalent Assert macros.

Having done that, it's very hard to justify carrying the errorType
argument of ExceptionalCondition, so drop that too, and just
let it assume everything's an Assert.  This saves about 64K
of code space as of current HEAD.

Discussion: https://postgr.es/m/3928703.1665345117@sss.pgh.pa.us
2022-10-10 15:16:56 -04:00
Peter Eisentraut 357cfefb09 Use C library functions instead of Abs() for int64
Instead of Abs() for int64, use the C standard functions labs() or
llabs() as appropriate.  Define a small wrapper around them that
matches our definition of int64.  (labs() is C90, llabs() is C99.)

Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/4beb42b5-216b-bce8-d452-d924d5794c63%40enterprisedb.com
2022-10-10 09:01:17 +02:00
Peter Eisentraut e4c61bedcb Use fabsf() instead of Abs() or fabs() where appropriate
This function is new in C99.

Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/4beb42b5-216b-bce8-d452-d924d5794c63%40enterprisedb.com
2022-10-08 13:43:26 +02:00