We were skipping a bunch of things that are mostly unnecessary for
REPACK. However, one thing that seems would be better to pass closer to
truth, is the updatedCols bitmapset in the range table entry for the
repacked table. Cons up an RTE and install it into the EState.
This only has an effect on btree indexes, because certain operations are
optimized in the case of unchanged columns; and even then, correctnesss
is not being compromised.
The values we pass after this commit are not fully trustworthy either,
because we simply say "all columns were updated" for all insert/updates,
regardless of whether their values were actually modified or not.
However, this way we err to the side of caution rather than to the
opposite direction as we were originally doing. This could be refined
in the future, but there's a trade-off: determining whether the column
was in fact updated could be expensive.
Author: Antonin Houska <ah@cybertec.at>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/18222.1782126731@localhost
Commit ce146621 ensures that ERROR is raised if a session tries to read
pages of another session's temp table. But there is a corner case where
the other session's temp table is empty -- in this case the INSERT
command bypasses our checks and executes without any errors.
Such behavior is inconsistent and erroneous: it leaves an invalid buffer
in the temp buffers pool. Since the buffer was created for another
session's temp table, we get an error "no such file or directory" when
trying to flush it.
This commit fixes it by adding a RELATION_IS_OTHER_TEMP check in the
relation-extension path.
Backpatch to 16, because it is the first release after 31966b151e, which
introduced a separate local relation extension function
ExtendBufferedRelLocal(), which lacks of RELATION_IS_OTHER_TEMP() check.
As this fix introduces more checks to 013_temp_obj_multisession.pl, backpatch
the whole test script to 16.
Discussion: https://postgr.es/m/CAJDiXgiX2XZBHDNo%2BzBbvku%2BtchrUurvPRaN1_40mEQ1_sG90g%40mail.gmail.com
Author: Daniil Davydov <3danissimo@gmail.com>
Reviewed-by: Jim Jones <jim.jones@uni-muenster.de>
Reviewed-by: Imran Zaheer <imran.zhir@gmail.com>
Reviewed-by: ZizhuanLiu X-MAN <44973863@qq.com>
Backpatch-through: 16
When dropping a property by name from a label, the code checked only
whether the property existed in the graph's property catalog. It did
not verify that the property was actually associated with the given
label, resulting in passing InvalidOid to performDeletion(). Fix it
by explicilty checking the label property association.
While at it also rearrange the code so as to avoid multiple ereport
calls for the same error in the same block.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/1DA5D52A-4AFA-426E-83F7-42ED974D682B%40gmail.com
BackendKeyData length was increased from 4 bytes to a variable-length
length (up to 256 bytes) in a460251f0a. However, pqTrace still traces
it as a 4 bytes key, leading to a "mismatched message length" warning
message. The same issue impacts the tracing of CancelRequest.
This patch fixes the issue by using pqTraceOutputNchar instead of
pqTraceOutputInt32 in both cases.
Author: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>
Discussion: https://www.postgresql.org/message-id/CAO6_Xqo6gTv9=76H=k2qDRFU+KHuBiY2S=bQynEr6J8gS7L6xA@mail.gmail.com
Backpatch-through: 18
In order to replay concurrent changes, REPACK CONCURRENTLY needs the
pg_attrdef tuples for the transient table to be there, in case a tuple
is modified concurrently with REPACK and requires to store the value
from the generated column (which, with the current arrangements, means
all tuples concurrently updated or inserted). Fix by creating a copy of
them from the original table. Add a test that tickles the bug.
Author: Antonin Houska <ah@cybertec.at>
Reported-by: Ewan Young <kdbase.hack@gmail.com>
Diagnosed-by: Ewan Young <kdbase.hack@gmail.com>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CAON2xHMrELwx9vKg6niSf8fMBA=-MGXmG=MPQU6+vMVhGjF8kQ@mail.gmail.com
Per SQL/PGQ standard, every graph element must have at least one
label. When dropping a label from a graph element, ensure that there
exists at least one other label on the element. If the label being
dropped is the only label on the element, raise an error.
We hold a ShareRowExclusiveLock when modifying a property graph.
Hence the label will not be dropped even when multiple labels are
being dropped concurrently.
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Author: Satyanarayana Narlapuram <satyanarlapuram@gmail.com>
Reported-by: Satyanarayana Narlapuram <satyanarlapuram@gmail.com>
Discussion: https://www.postgresql.org/message-id/CAHg+QDeP=mTHTV48R23zKMy1SBmCKZ_L7-z5zKnYyw+K0x-gCg@mail.gmail.com
Buildfarm member prion reported a failure in the test added by commit
c8bd8387c2 to verify that the server logs an empty statement body
when log_statement_max_length = 0.
The test assumed that "statement:" would appear immediately after
"LOG:" in the logged statement message. However, prion runs with
log_error_verbosity = verbose, which inserts the SQLSTATE between
"LOG:" and the message text. As a result, the test failed even though
the server behaved correctly.
Per buildfarm member prion.
Discussion: https://postgr.es/m/CAHGQGwFiQKwvLVG+U0WWNo2kgkQ88FVGhYH_MBZu9Y0SJ8BjDw@mail.gmail.com
Commit fb421231da extended \df to include procedures, but its tab
completion continued not to show procedures.
Update \df tab completion to include procedures as well.
Backpatch to all supported versions.
Author: Erik Wienhold <ewie@ewie.name>
Reviewed-by: Surya Poondla <suryapoondla4@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/10fbfdfe-80f6-4ef9-b8b3-f7be0eb53a50@ewie.name
Backpatch-through: 14
GetBufferDescriptor() and GetLocalBufferDescriptor() took a uint32
buffer index, but every real caller derives the index from a Buffer:
- Unsigned value for shared buffers.
- Signed value for local buffers.
Both routines now take in input a signed number, GetBufferDescriptor()
gaining an assertion checking that the input value is in the range
allowed by the GUC shared_buffers. This work is a follow-up of
e18b0cb734, where we found that passing down a value for a local
buffer was undetected and finished outside the range of NBuffers.
While monitoring all the existing callers of *BufferDescriptor(), the
only consumer that passes does an unsigned value is ClockSweepTick(),
whose result is always a module of NBuffers.
Suggested-by: Andres Freund <andres@anarazel.de>
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAExHW5uzRMYVZsXXS3HXXT0fG_sNrpUhUqwP4NorhaCqH9JDhA@mail.gmail.com
Previously, MultiXactId wraparound hints suggested dropping stale
replication slots. While that advice is appropriate for transaction ID
wraparound, where replication slots can hold back XID horizons,
it was misleading for MultiXactId wraparound. Following it could lead
users to drop replication slots unnecessarily without helping resolve
the MultiXactId wraparound condition.
MultiXact cleanup is not directly delayed by replication slots.
Instead, it depends on whether old MultiXactIds can still be seen
as live by running transactions.
This commit removes the replication slot advice from MultiXactId
wraparound hints, and documents that stale replication slots are
normally not relevant to resolving MultiXactId wraparound problems.
Backpatch to all supported branches.
BUG #18876
Reported-by: Haruka Takatsuka <harukat@sraoss.co.jp>
Author: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/18876-0d0b53bad5a1f4c1@postgresql.org
Backpatch-through: 14
Very large statements can make server logs grow unexpectedly. This is
particularly painful when applications accidentally or intentionally
send huge literal values and statement logging is enabled: the full
statement text may be written to the log even when DBA sees only
its leading part is useful for normal operations.
This commit adds log_statement_max_length GUC that limits the number
of bytes of statement text emitted by statement logging. The setting
applies to statements logged by log_statement, log_min_duration_statement,
log_min_duration_sample, and log_transaction_sample_rate. A positive
value truncates the logged statement body to at most that many bytes,
zero logs an empty statement body, and the default value -1 preserves
the existing behavior of logging statements in full.
Truncation is byte-based, matching the GUC unit, but it clips only
at multibyte character boundaries so that the log output remains valid.
This setting does not affect statements logged because of
log_min_error_statement; handling error-statement logging can be
considered separately.
Author: Jim Jones <jim.jones@uni-muenster.de>
Author: Kirill Gavrilov <diphantxm@gmail.com>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Reviewed-by: Maxym Kharchenko <maxymkharchenko@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CA+E0NR4S+NC6+QHyY_vUuQZMzLhKqczMx-jJVqtjAxF6+=JwAA@mail.gmail.com
For ease of review, and to be able to put the indentation changes
in .git-blame-ignore-revs, I did not fix the indentation of the
last 3 commits. Do that now.
Discussion: https://postgr.es/m/adZ4j88Dq9r8y9_9%40nathan
Per discussion, it seems like a good time to bump the minimum
supported version for various applications. Our current policy is
to support at least 10 previous major versions, so this bumps the
minimum to v10 for the v20 release. For reference, the minimum was
last bumped to v9.2 in 2021 for v15 (see commits 30e7c175b8,
e469f0aaf3, cf0cab868a, and 492046fa9e).
For ease of review, and to be able to put the indentation changes
in .git-blame-ignore-revs, I did not fix the indentation in this
patch. I'll push a separate pgindent commit after these changes
are applied.
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/adZ4j88Dq9r8y9_9%40nathan
Per discussion, it seems like a good time to bump the minimum
supported version for various applications. Our current policy is
to support at least 10 previous major versions, so this bumps the
minimum to v10 for the v20 release. For reference, the minimum was
last bumped to v9.2 in 2021 for v15 (see commits 30e7c175b8,
e469f0aaf3, cf0cab868a, and 492046fa9e).
For ease of review, and to be able to put the indentation changes
in .git-blame-ignore-revs, I did not fix the indentation in this
patch. I'll push a separate pgindent commit after these changes
are applied.
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/adZ4j88Dq9r8y9_9%40nathan
Per discussion, it seems like a good time to bump the minimum
supported version for various applications. Our current policy is
to support at least 10 previous major versions, so this bumps the
minimum to v10 for the v20 release. For reference, the minimum was
last bumped to v9.2 in 2021 for v15 (see commits 30e7c175b8,
e469f0aaf3, cf0cab868a, and 492046fa9e). As in previous changes of
this sort, we aren't removing pg_restore's ability to read older
archive files, though it's fair to wonder how that might be tested
nowadays.
For ease of review, and to be able to put the indentation changes
in .git-blame-ignore-revs, I did not fix the indentation in this
patch. I'll push a separate pgindent commit after these changes
are applied.
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/adZ4j88Dq9r8y9_9%40nathan
The int2, oid, and oid8 "fastcmp" comparators are functionally
equivalent to the ssup_datum_int32_cmp (for int2) and
ssup_datum_unsigned_cmp (for oid, oid8) functions added by commit
697492434, so simplify by using the latter instead. This has the
added benefit of making these types eligible for radix sort.
Author: Baji Shaik <baji.pgdev@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA+fm-RMyLC94NfrxCh273+dKs44U0ZJjRczznvzvgw=KtpPNVw@mail.gmail.com
Previously, test_custom_var_stats and test_custom_fixed_stats silently
skipped pgstat_register_kind() when not loaded via
shared_preload_libraries, behavior inherited from injection_points.
This left the SQL functions callable without the kind registered,
leading to various issues on the backend side.
This code is not designed to work without the pgstats kinds registered.
pgstat_register_kind() gets now called when these libraries are loaded,
with or without shared_preload_libraries, letting the registration fail
if loading the modules at a later step than startup. test_custom_rmgrs
does the same thing.
Oversight in 31280d96a6.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://postgr.es/m/akS/ldidWeqG1FWk@bdtpg
Backpatch-through: 19
Multiplying by the constant 0.001 can produce trailing-digit noise in
displayed values (for example 0.009000000000000001 instead of 0.009,
with default extra_float_digits) because 0.001 cannot be represented
exactly in binary floating point. Use division by 1000.0 instead,
matching code elsewhere in the tree.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/akIYkMK4bHe9qX/N@bdtpg
The existing comment explained that a user-created slot could reuse the
same shared memory as 'local_slot' during the window between selecting a
slot to drop and locking its database, and that we therefore recheck
before dropping. It did not, however, spell out the fuller consequence:
because local_slot points to a reusable slot-array entry, its fields may
already describe a replacement slot, so the earlier drop decision and the
slot_database used for locking could relate to an unrelated slot/database.
Expand the comment to describe this, and note that the recheck prevents
us from dropping a user-created replacement slot while the residual risk
(such as briefly locking an unrelated database) is confined to the cycle
and is acceptable given the race is rare and non-fatal.
No functional change.
Author: Fujii Masao <masao.fujii@gmail.com>
Author: Xuneng Zhou <xunengzhou@gmail.com>
Author: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwGGyEDL3dh7uJ6qPsGvnq4QK_R8+U=12CaprnzwrwaLGA@mail.gmail.com
Discussion: https://postgr.es/m/CAHGQGwHqQ1PPVFfYKVxLfRyC-byRdwSN0NeaHj9SLYV97oO5cw@mail.gmail.com
The jsonpath .decimal(precision[, scale]) method built its numeric
typmod by calling numerictypmodin() through DirectFunctionCall1(), which
can throw a hard error for an incorrect set of precision and/or scale
vaulues. This breaks the silent mode supported by this function, that
should not fail.
Most of the jsonpath code uses the soft error reporting to bypass
errors, which is what this fix does by avoiding a direct use of
numerictypmodin(). Its code is refactored to use a new routine called
make_numeric_typmod_safe(), able to take an error context in input.
numerictypmodin() sets no context, mapping to its previous behavior.
The jsonpath code sets or not a context depending on the use of the
silent mode. This result leads to some nice simplifications:
numerictypmodin() feeds on an array, we can now pass directly values for
the scale and precision.
Oversight in 66ea94e8e6.
Author: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://postgr.es/m/CAON2xHMaigKABiyPBBq3Sjd3gp7uWMJXnnMHt=s85V1ij3KP1w@mail.gmail.com
Backpatch-through: 17
Until now, logical replication conflicts were only written as plain text
to the server log, which is hard to query, analyze, or feed into external
monitoring and automation.
This commit adds a conflict_log_destination option to CREATE SUBSCRIPTION
and ALTER SUBSCRIPTION that controls where conflicts are recorded. It
accepts 'log' (the existing behavior), 'table', or 'all'.
When table logging is enabled ('table' or 'all'), an internal log table
named pg_conflict_log_<subid> is created automatically in a dedicated,
system-managed pg_conflict namespace. Using a separate namespace avoids
collisions with user table names and lets the table be shielded from
direct modification. The table is tied to the subscription through an
internal dependency, so it is dropped automatically when the subscription
is removed.
The conflict details, including the local and remote tuples, are stored in
JSON columns, so a single table layout can accommodate rows from tables
with different schemas. The table also records the local and remote
transaction IDs, LSNs, commit timestamps, and the conflict type, providing
a complete record for post-mortem analysis.
A per-subscription table was chosen over a single global log because it
aligns table ownership with the subscription lifecycle. This keeps
permission management simple: the subscription owner can perform the
permitted maintenance operations without the security concerns or
Row-Level Security that a shared table would require.
Because the table is system-managed, it is protected from direct
manipulation: DDL (such as ALTER, DROP, CREATE INDEX, and adding a
trigger, rule, policy, or extended statistics), use as an inheritance
parent or a foreign-key target, and manual INSERT, UPDATE, MERGE, or row
locking are all rejected. Only DELETE and TRUNCATE are permitted, so that
users can prune old conflict rows.
Conflict log tables are also excluded from publications, even those
defined with FOR ALL TABLES or FOR TABLES IN SCHEMA.
This commit only establishes the conflict log table along with its
creation, cleanup, and protection; recording the conflicts detected
during apply into the table will be handled in a follow-up commit.
Author: Dilip Kumar <dilipbalaut@gmail.com>
Author: Nisha Moond <nisha.moond412@gmail.com>
Author: Amit Kapila <akapila@postgresql.org>
Reviewed-by: Shveta Malik <shveta.malik@gmail.com>
Reviewed-by: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: Shlok Kyal <shlok.kyal.oss@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAFiTN-u5D5o_AGNbHRZHaOqAMWkxLf%2BhSk_r9X3gv6HbLOB5%2Bg%40mail.gmail.com
This commit adds support for pg_stat_kind_info, that exposes at SQL
level data about the statistics kinds registered into a backend:
- Meta-data of a stats kind (built-in or custom, some properties).
- Number of entries, if tracking is enabled.
We have discussed the possibility of more fields (like shared memory
size for a single entry); this adds the minimum agreed on.
This is in spirit similar to pg_get_loaded_modules() for custom stats
kinds, this view providing detailed information about the stats kinds
when registered through shared_preload_libraries.
Bump catalog version.
Author: Tristan Partin <tristan@partin.io>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/DI6OFGHJ1B69.25YVDEP3BABRH@partin.io
The uuid type already has a full set of comparison operators and a
btree operator class, so it is totally ordered. min() and max() were
the only common aggregates missing for it. Add the uuid_larger() and
uuid_smaller() support functions and register the min(uuid) and
max(uuid) aggregates that use them.
uuid values are compared lexicographically over their 128 bits. For
UUIDv7, whose most significant bits encode a Unix timestamp, this
coincides with chronological order, so min() and max() return the
oldest and newest values.
Bump catalog version.
Author: Tristan Partin <tristan@partin.io>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/DJGML0T9FCDV.3VA29JLODXEHZ@partin.io
Some platforms provide a definition of unreachable() in standard C
headers, leading to a conflict with unreachable() in the new
timezone code. It seems best for our purposes to conform to what
pg_unreachable() does, so #undef away the platform version.
Reported-by: Tristan Partin <tristan@partin.io>
Discussion: https://postgr.es/m/DJNDN9UQS9GP.11L4NJ1HHE1ZJ@partin.io
If the user doesn't specify a name for an index, it's generated
based on the names chosen for the index columns (which the user
has no direct control over). For index columns that are just
columns of the base relation, the index column name is the same as
the base column name; but for index columns that are expressions,
it's less clear what to do. Up to now, what we have done is
equivalent to the heuristics used to choose SELECT output column
names, except that we fall back to "expr" not "?column?" in the
numerous cases that FigureColname doesn't know what to do with.
This is not tremendously helpful. More, it frequently leads to
collisions of generated index names, which we can handle but
only at the cost of user confusion; also there's some risk of
concurrent index creations trying to use the same name.
Let's try to do better.
Messing with the FigureColname heuristics would have a very
large blast radius, since that affects the column headings
that applications see. That doesn't seem wise, but fortunately
SQL queries are seldom directly concerned with index names.
So we should be able to change the index-name generation rules
as long as we decouple them from FigureColname.
The method used in this patch is to dig through the expression,
extract the names of Vars, the string representations of Consts,
and the names of functions, and run those together with underscores
between. Other expression node types are ignored but descended
through. We could work harder by handling more node types, but
it seems like this is likely to be sufficient to arrive at unique
index names in many cases.
Notably, this rule ignores the names of operators, for example
both "a + b" and "a * b" will be rendered as "a_b". This choice
was made to reduce the probability of having to double-quote
the index name.
I've also chosen to strip Const representations down to only
alphanumeric characters (plus non-ASCII characters, which our
parser treats as alphabetic anyway). So for example "x + 1.0"
would be represented as "x_10". This likewise avoids possible
quoting problems. I also considered limiting how many characters
we'd take from each Const, but didn't do that here.
We might tweak these rules some more after we get some experience
with this patch. It's being committed at the start of a
development cycle to provide as much time as possible to gather
feedback.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/876799.1757987810@sss.pgh.pa.us
Discussion: https://postgr.es/m/18959-f63b53b864bb1417@postgresql.org
This was moderately tedious, because upstream has been busy
since we last did this in 2020.
Notably, they changed the signatures of both tzload() and tzparse(),
which we'd unwisely exposed as API for callers to use. I concluded
that the best answer was to change them both back to "static" and
instead expose a new API function of our own choosing, pg_tzload().
That change may be a sufficient reason not to back-patch this update,
as I'd normally want to do. There's probably not a good reason for
extensions to be calling those functions, but on the other hand
there are few pressing reasons for a back-patch. The one bug we have
run into (a Valgrind uninitialized-data complaint about zic) appears
to have no field-visible consequences.
A few of the files generated by this version of zic are not
byte-for-byte the same as before, but "zdump -v" avers that
they represent the same sets of transitions.
Discussion: https://postgr.es/m/2294297.1780270682@sss.pgh.pa.us
Previously, if a base backup failed after it had started streaming
files, pg_stat_progress_basebackup could continue to show a stale
progress entry even though the backup was no longer running. This could
be observed when the client kept the replication connection open after
the error. It is normally not observable when using pg_basebackup,
because the client disconnects after the error.
The problem was that progress reporting was cleared only after
successful completion.
This commit moves the progress reporting cleanup into the progress
sink's cleanup callback so that it is cleared after both successful
and failed backups.
Backpatch to v15. v14 has the same issue, but the fix does not apply
cleanly because it lacks the base backup sink infrastructure. Since
the bug does not affect the backup itself and is normally not
observable when using pg_basebackup, skip the v14 backpatch.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/EA1A6CD2-EFA6-462B-9A02-03003555AB4A@gmail.com
Backpatch-through: 15
Commit bc60ee860 added a connection warning after successful MD5
authentication, but only for the md5 authentication method. A role with
an MD5-encrypted password can also authenticate via the password method,
which left that path without the same deprecation warning.
Emit the MD5 deprecation connection warning after successful
password authentication as well, when the stored password is
MD5-encrypted.
Backpatch to v19, where the MD5 connection warning was introduced.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Japin Li <japinli@hotmail.com>
Discussion: https://postgr.es/m/CAHGQGwGkWfn5rtHzvdRbVk+PCefQU3gun3hc7QnaMXHFa5Bu3w@mail.gmail.com
Backpatch-through: 19
In fe_memutils.h, we have various allocation functions beginning with
either pg_ or p. The pg_ functions have a matching pg_free() for
freeing memory, while the p functions use pfree(). In some cases, we
were allocating memory with one set of functions while using the wrong
deallocation functions. This creates a tiny bit of mental overhead
when reading code. Matching up allocation and deallocation functions
makes it easier to analyze memory handling in a code path.
Author: Tristan Partin <tristan@partin.io>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Discussion: https://www.postgresql.org/message-id/flat/DIBZE2B6SVF2.28R3EQTYJSWIG@partin.io
Fixup for commit c05dee1911. It fits better with the style and APIs
to print separate primary and a detail messages instead of one
multiline message.
Reviewed-by: Euler Taveira <euler@eulerto.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/CAHut+PsvQJQnQO0KT0S2oegenkvJ8FUuY-QS5syyqmT24R2xFQ@mail.gmail.com
off_t is most likely a 64-bit integer, so casting it to a 32-bit type
for output could lose data. There are more issues like this in the
tree, but this is an instance where this could actually happen in
practice, since base backups are routinely larger than 4 GB. So this
is separated out as a bug fix.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/flat/20ce62fa-47fc-457b-b504-12f3c1651726%40eisentraut.org
Replace pg_attribute_aligned with C11 alignas, for consistency with
current conventions.
(These new uses were added by commit fbc57f2bc2, which was developed
concurrently with the switch from pg_attribute_aligned to C11 standard
alignas, and it ended up being committed with the "old" style.)
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Discussion: https://postgr.es/m/CANWCAZaKhE+RD5KKouUFoxx1EbUNrNhcduM1VQ=DkSDadNEFng@mail.gmail.com
A UNION (not UNION ALL) removes duplicates, so its output has no more
rows than its input. The planner did not account for this: it set the
set-op relation's row count to the total size of the appended input,
as though dedup removed nothing. That inflated estimate then
propagated to every node above the UNION, leading to poor plan choices
such as a hash join with a full table scan where an index nested loop
would have been cheaper.
This patch estimates the number of distinct output rows as the sum of
the per-child distinct-group estimates instead. This relies on the
fact that:
distinct(A union B) <= distinct(A) + distinct(B)
that is, the union cannot have more distinct rows than its children do
in total. And because each child's distinct-group estimate never
exceeds that child's row-count estimate, this sum is never larger than
the old estimate, so it only tightens the previous over-estimate.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Chengpeng Yan <chengpeng_yan@outlook.com>
Discussion: https://postgr.es/m/CAMbWs48Fu1nhGXPa60oc+adj7ge4dn0nHhqngqKvOVVQP61duA@mail.gmail.com
MultiXactOffsetStorageSize() and GetMultiXactInfo() are called to gather
the information reported by the function, but were wasteful for the case
where a role does not have the privileges of pg_read_all_stats, where we
return a set of NULLs. These calls are moved to the code path where
their results are used.
Author: Ranier Vilela <ranier.vf@gmail.com>
Discussion: https://postgr.es/m/CAEudQAonQh7be=wOR-CJFW=bgMBz5wW_bv4t0OFxbgn-794JCQ@mail.gmail.com
Backpatch-through: 19
Commit 4035cd5d4 added LZ4 compression for full-page writes in WAL, and
retained "on" as a backward-compatible way to specify the builtin PGLZ
method. Document this meaning of "on" and update postgresql.conf.sample
to make the equivalence clear.
Author: Christoph Berg <myon@debian.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/akJDHRtXwGLTppsQ@msg.df7cb.de
Backpatch-through: 15
Commit a1643d40b removed RADIUS authentication, but apparently
overlooked initdb's list of accepted authentication methods. As a
result, initdb still accepted radius for --auth, --auth-host, and
--auth-local, allowing it to create a pg_hba.conf that the server could
not load.
Remove radius from initdb's local and host authentication method lists.
Backpatch-through: 19
Author: Chao Li <lic@highgo.com>
Discussion: https://postgr.es/m/983F946B-A7CE-4C93-B5F0-665616F72254%40gmail.com
We previously allowed this, but it leads to odd behaviors, basically
because putting a SRF there is inconsistent with the principle that a
window function doesn't change the number of rows in the query result.
There doesn't seem to be a strong reason to try to make such cases
behave consistently. Users should put their SRFs in lateral FROM
clauses instead.
This issue has been sitting on the back burner for multiple years
now, partially because it didn't seem wise to back-patch such a
change. Let's squeeze it into v19 before it's too late.
Bug: #17502
Bug: #19535
Reported-by: Daniel Farkaš <daniel.farkas@datoris.com>
Reported-by: Qifan Liu <imchifan@163.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/17502-281a7aaacfaa872a@postgresql.org
Discussion: https://postgr.es/m/19535-376081d7cc07c86d@postgresql.org
Backpatch-through: 19
In various places we depend on compiler-defined macros like __x86_64__
to guard CPU-type-specific code. However, those macros aren't very
well standardized; in particular, it emerges that MSVC doesn't define
any of the ones gcc does, but has its own. We were not coping with
that consistently, with the result that we're missing some useful
CPU-dependent optimizations in MSVC builds. There are also some
places that are checking randomly-different spellings that may
have been the only ones recognized by some old compilers, but we
weren't doing that consistently either.
Let's standardize on using gcc's long-form spellings (with trailing
underscores), after putting a stanza into c.h that ensures that these
spellings are defined even when the compiler provides some other one.
I put an "#else #error" branch into the c.h addition so that we'll
get an error if the compiler provides none of the symbols we're
expecting. That might be best removed in the end, since it might
annoy people trying to port to some new CPU type. But for testing
this it seems like a good idea, in case we've missed some common
variant spelling.
In addition to enabling some optimizations we previously missed on
MSVC, this cleans up a thinko. Several places used "_M_X64" in the
apparent belief that that's MSVC's equivalent to __x86_64__, but
it's not: it will also get defined on some but not all ARM64 builds.
Also, guard the x86_feature_available() stuff in pg_cpu.[hc] with
#if defined(__x86_64__) || defined(__i386__)
which seems like a more natural way of specifying what it applies to.
This builds on some previous work by Thomas Munro, but it requires
much less code churn because it re-uses gcc's names for the CPU-type
macros instead of inventing our own.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA+hUKGL8Hs-phHPugrWM=5dAkcT897rXyazYzLw-Szxnzgx-rA@mail.gmail.com
Discussion: https://postgr.es/m/3035145.1780503430@sss.pgh.pa.us
This changes the argtypes argument of SPI_prepare(),
SPI_prepare_cursor(), SPI_cursor_open_with_args(), and
SPI_execute_with_args() from Oid *argtypes to const Oid *argtypes.
The underlying functions were already receptive to that, so this
doesn't require any significant changes beyond the function signatures
and some internal variables.
Commit 28972b6fc3 recently introduced a case where a const had to be
cast away before calling these functions. This is fixed here.
In passing, make a very similar const addition to SPI_modifytuple().
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/86b5162f-c472-40fa-997b-0450dece1dec%40eisentraut.org
Fixup for commit 8a27d418f8, which converted many functions to use
"const Datum *" instead of "Datum *", including some SPI functions.
For SPI_cursor_open(), the code was updated but not the documentation.
For SPI_cursor_open_with_args(), the documentation was updated but not
the code. (Possibly, these two were confused with each other.) Also,
SPI_execp() and SPI_modifytuple() were not updated, even though they
are closely related to the functions touched by the previous commit
and now look inconsistent. Fix all these.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/86b5162f-c472-40fa-997b-0450dece1dec%40eisentraut.org
This commit adds per-backend lock statistics, providing the same
information as pg_stat_lock. It is now possible to retrieve those stats
(lock wait counts, wait times, and fast-path exceeded count) on a
per-backend basis.
This data can be retrieved with a new system function called
pg_stat_get_backend_lock(), that returns one tuple per lock type based
on the PID provided in input. Like pg_stat_get_backend_io(), this is
useful if joined with pg_stat_activity to get a live picture of the
locks behavior for each running backend.
pgstat_flush_backend() gains a new flag value, able to control the flush
of the lock stats.
This commit is straight-forward, relying on the infrastructure provided
by 9aea73fc61 (backend-level pgstats).
Bump catalog version. No need to touch PGSTAT_FILE_FORMAT_ID as backend
statistics are never written to disk.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Tatsuya Kawata <kawatatatsuya0913@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Tristan Partin <tristan@partin.io>
Reviewed-by: Rui Zhao <zhaorui126@gmail.com>
Discussion: https://postgr.es/m/aiAzEY+cMQb/W8yu@bdtpg
This commit extracts the tuple-building logic from pg_stat_get_lock()
into a new static helper pg_stat_lock_build_tuples(). This is in
preparation for a follow-up patch, to add support for backend-level lock
stats, which will reuse the same helper.
This change follows the pattern established by pg_stat_io_build_tuples()
for IO stats and pg_stat_wal_build_tuple() for WAL stats.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Tatsuya Kawata <kawatatatsuya0913@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Tristan Partin <tristan@partin.io>
Reviewed-by: Rui Zhao <zhaorui126@gmail.com>
Discussion: https://postgr.es/m/aiAzEY+cMQb/W8yu@bdtpg
A placeholder %s is now used instead of the GUC names in the error
string of this routine. This is going to be useful for a follow-up
patch, where we will be able to reuse the same string, hence reducing
the translation work.
Based on a suggestion by me.
Author: Baji Shaik <baji.pgdev@gmail.com>
Discussion: https://postgr.es/m/ajnhfw84reaXgjfO@paquier.xyz
Other statistics views (pg_stat_io, pg_stat_database, etc.) use float8
for all measured-time columns, the new pg_stat_lock standing out as an
outlier by using bigint.
This commit aligns pg_stat_lock with the other stats views for
consistency. Like pg_stat_io, the time is stored in microseconds, and
is displayed in milliseconds with a conversion done when the view is
queried.
While on it, replace a use of "long" by PgStat_Counter, the former could
overflow for large wait times where sizeof(long) is 4 bytes (aka WIN32).
Bump catalog version.
Author: Tatsuya Kawata <kawatatatsuya0913@gmail.com>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAHza6qerEiQehrbW5xaXyxvR0qJe3KBX1R4kocDz1+7Ygu8x-g@mail.gmail.com
Backpatch-through: 19
