Commit 2f094e7ac6 added a mention of SECURITY LABEL ON PROPERTY GRAPH
to the SECURITY LABEL reference page, and it added support to psql tab
completion. However, security labels on property graphs are not
actually supported (per SecLabelSupportsObjectType()). The syntax
does work, but that is just a result of how gram.y is factored. We
don't document or tab-complete the syntax of SECURITY LABEL for other
object types that are not actually supported, so it was inconsistent
to do this for property graphs. Thus, remove this.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://www.postgresql.org/message-id/flat/20260704221210.08.noahmisch%40microsoft.com
With virtual generated columns there is no column to assign to, and we
shouldn't assign directly to stored generated columns either. (Once
we have PERIODs, we will allow a stored generated column here, but we
will assign to its start/end inputs.)
We can't do this in parse analysis, because views haven't yet been
rewritten, so they mask generated columns.
Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/agOOykf2HV26yVfU%40nathan
The planner has two optimizations that move a qual clause across a
grouping boundary: subquery_planner transfers HAVING clauses to WHERE
so they can be evaluated before aggregation, and qual_is_pushdown_safe
pushes outer restriction clauses into a subquery past its DISTINCT,
DISTINCT ON, window PARTITION BY, or set-operation grouping layer.
Both produce wrong results when the moved clause's equivalence
relation disagrees with the grouping's, since the clause then filters
rows the grouping would have merged.
The disagreement has two forms. A type may belong to multiple btree
opfamilies whose equality operators disagree (e.g. record_ops vs
record_image_ops); or the grouping may use a nondeterministic
collation, where comparing the column under a different collation, or
wrapping it in a function or operator, can distinguish values the
collation considers equal. Because we cannot prove an arbitrary
expression preserves that equality, a grouping column with a
nondeterministic collation is safe to push only as a direct operand of
a comparison under its own collation.
Fix both call sites through a shared walker parameterized by a
callback that maps each Var to the grouping equality operator for its
column (or InvalidOid for non-grouping Vars). For HAVING, the
callback recovers the SortGroupClause's eqop via the GROUP Var's
varattno, which requires running before flatten_group_exprs while
havingQual still contains GROUP Vars. For subquery pushdown, the
callback recovers the eqop from subquery->distinctClause, a window's
partitionClause, or any grouping node in the SetOperationStmt tree.
The walker fires only when there is an equivalence boundary to cross,
gated by either the existing UNSAFE_NOTIN_DISTINCTON_CLAUSE and
UNSAFE_NOTIN_PARTITIONBY_CLAUSE flags or by a recursive check for any
grouping node in the set-op tree.
Back-patch to v18 only. The HAVING half relies on the RTE_GROUP
mechanism introduced in v18 (commit 247dea89f), which is what lets us
identify grouping expressions via GROUP Vars on pre-flatten
havingQual. Pre-v18 branches lack that machinery, so a back-patch
there would need a different approach. Given the absence of field
reports of these bugs on back branches, the risk of carrying a
different fix on stable branches is not justified.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Thom Brown <thom@linux.com>
Reviewed-by: Florin Irion <irionr@gmail.com>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Chengpeng Yan <chengpeng_yan@outlook.com>
Discussion: https://postgr.es/m/CAMbWs4-QLZpn3UVOpeG2fOxxhdnkDNMZ_3Zcm3dqJwRAphz68g@mail.gmail.com
Backpatch-through: 18
When io_min_workers is set strictly higher than io_max_workers, the
minimum has no effect since the pool will never grow past
io_max_workers. Previously this was silently accepted, which could
be confusing for users expecting at least io_min_workers workers to
always be running.
In order to avoid noise in the server logs, the following restrictions
are in place:
- The only process printing the WARNING is the IO worker with ID 0, on
startup and reload, which is we know the only process always running
when using IO workers.
- At reload, the message shows only if one of the bounds has changed.
Note that this commit reuses a log message updated by 7905416eef.
Author: Baji Shaik <baji.pgdev@gmail.com>
Reviewed-by: Tristan Partin <tristan@partin.io>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/CA+fm-RO_O7-XThg2qjj=ir35x9nOFbZYu07gttqAbM5T88QB4Q@mail.gmail.com
pgstat_register_kind() did not validate that required callbacks are
set, which could lead to NULL pointer dereferences when trying to
register a stats kind. This adds a couple of checks:
- Fox fixed-sized kinds, init_shmem_cb, reset_all_cb, and snapshot_cb
are required.
- For variable-sized kinds, flush_pending_cb is called when there is
pending data, pending_size being required.
These issues should be easy to notice for someone developing an
extension that relies on the custom pgstats APIs. No backpatch is done
as it is mainly a life improvement.
Author: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/CAA5RZ0uNoe=xT7QsU1K0mMRg-QAwPtupPWZ2J3weM2PjVL2tiA@mail.gmail.com
"prev_tuple" was overwritten with a new tuple coming from
CopyIndexTuple() on each loop, leaking memory for every tuple processed
on entry tree pages. The function uses a dedicated memory context, but
this could leave unused large areas of memory while processing a large
GIN index, the larger the worse.
Oversight in 14ffaece0f.
Author: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://postgr.es/m/CALdSSPjTS6TYe5=5NfMUBYZyQu5cn=ABL6K5_OZjzGWqnwXeBw@mail.gmail.com
Backpatch-through: 18
psql decided whether to use the pager in expanded output without
accounting for possible wrapping of column values. This could
allow it to not use the pager in cases where it should do so.
To fix, move the IsPagerNeeded decision in print_aligned_vertical()
down until after the wrapped data width is known. Then, if we're in
wrapped mode, prepare a width_wrap array specifying that width (which,
in vertical mode, is the same for all columns).
This is fixing an omission in 27da1a796, so back-patch to v19
where that came in.
Author: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Erik Wienhold <ewie@ewie.name>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/A44110E7-6A03-4C67-95AD-527192A6C768@gmail.com
Backpatch-through: 19
There's no need to create and free a temporary copy of the input,
since str_tolower() is already able to cope with not-certainly-
nul-terminated input. (Before v18, copying was needed because
this code used lowerstr(), but now we can do without.)
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19525-b0be8e4eb7dbaf07@postgresql.org
AlterPropGraph() cleans up pg_propgraph_property entries that are
orphaned by dropping an element or by dropping properties associated
with an element. But it did not clean up pg_propgraph_property
entries that are orphaned by dropping labels associated with an
element. Fix this missing case.
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Author: zengman <zengman@halodbtech.com>
Discussion: https://www.postgresql.org/message-id/flat/tencent_76F6ACA2364EAA1E5DBD7A47%40qq.com
ON SELECT rules must be named "_RETURN", while other kinds of rules
must not be; this ancient restriction is depended on by various client
code. We successfully enforced this convention in most places, but
ALTER RULE allowed renaming a non-SELECT rule to "_RETURN". Notably,
that would break dump/restore, since the eventual CREATE RULE command
would reject the name.
While at it, remove DefineQueryRewrite's hack to substitute "_RETURN"
for the convention that was used before 7.3. We dropped other
server-side code that supported restoring pre-7.3 dumps some time ago
(notably in e58a59975 and nearby commits), but this bit was missed.
Bug: #19543
Reported-by: Adam Pickering <adamkpickering@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19543-461228e77f3b32fc@postgresql.org
Backpatch-through: 14
getObjectDescription() currently constructs property graph-related
object descriptions incrementally with appendStringInfo(). This
effectively fixes the word order in English, which makes the messages
difficult to translate naturally into languages such as Japanese.
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/20260528.121622.1662808269492494574.horikyota.ntt%40gmail.com
gbt_var_node_cp_len() contained logic to ensure that its choice of
a common prefix length didn't truncate away part of a multibyte
character. However, that was really dead code, because we have not
allowed truncation of text-string data types since ef770cbb6, and
it seems unlikely that that behavior could ever get resurrected.
The code is still reachable via gbt_var_penalty, but for that
usage it hardly matters if we break in the middle of a multibyte
character: we're just calculating a small correction factor that
is arguably bunkum anyway in non-C locales.
Hence, delete said code. That actually removes all need for
gbtree_vinfo.eml, which allows const-ification of the gbtree_vinfo
structs in which we were changing it, which removes one headache
for future attempts to thread-ify the backend.
(Curiously, all this infrastructure was itself added by ef770cbb6.
Not sure why Teodor didn't see the contradiction.)
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/AH*AvQCYKhQGVvPWi1GiU4oY.8.1781609375063.Hmail.3020001251@tju.edu.cn
Truncating an internal node's upper bound can cause it to compare
less than some values that in fact are included in the represented
leaf page. So we need a hack to make sure it looks large enough
to include all values that could be on the page. But there's no
equivalent issue for the lower bound. The fact that the code did
fuzzy comparisons for the lower bound too seems to be the result of
fuzzy thinking. Or maybe there was a desire to not assume too much
about what the datatype's comparison rule is; but we've already
fully bought into the premise that internal keys compare like bytea.
Hence, remove the useless check against the key's lower bound in
gbt_var_node_pf_match. The comparable check in gbt_var_penalty may
also be useless, but I'm not quite sure. In any case that seems
negligible from a performance standpoint, so I left it alone.
Also, in the strategy cases in gbt_var_consistent that only
require comparisons to the lower bound, there's no need to call
gbt_var_node_pf_match at all. Refactor that logic by inventing
macros lower_is_below_query and upper_is_above_query to directly
express what we need to test. I also took this opportunity to flip
all the tests around to be "indexkey OP query" rather than mostly
being the reverse: IMO this makes the code less confusing since the
tests now match the names of the strategies.
Also, in the name of consistency, make gbt_num_consistent look
like that too. There's no functional change there, but this
should be more readable going forward.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/AH*AvQCYKhQGVvPWi1GiU4oY.8.1781609375063.Hmail.3020001251@tju.edu.cn
For some odd reason we pass the strategy number to gbt_num_consistent
as "const StrategyNumber *strategy". There's no reason for that:
it almost certainly costs more at both callers and callee to pass a
pointer than to pass a small integer value. And it's inconsistent
with gbt_var_consistent(), so fix it.
gbt_var_consistent() had its own infelicity, which was not marking
the input "key" value const. Fix that too while we're here.
This is primarily cosmetic, so I see no need to backpatch.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/AH*AvQCYKhQGVvPWi1GiU4oY.8.1781609375063.Hmail.3020001251@tju.edu.cn
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
gbt_var_consistent() handled the <> (BtreeGistNotEqual) strategy without
distinguishing leaf from internal pages, unlike every other strategy.
In particular, it tried to apply the datatype-specific f_eq method,
which is completely wrong since internal keys might not have the same
representation as leaf keys. This led to OOB reads and potentially
crashes, and most likely to wrong query results as well.
On leaf pages we can apply the inverse of what the Equal strategy does.
On internal pages, use a correct implementation of what the previous
code intended: we can descend if the query value equals both bounds,
*so long as the bounds aren't truncated*. With truncated bounds we
don't quite know the range of what's below, so we must always descend.
Adjust the code in gbt_num_consistent() to look similar, too. This
fixes a performance buglet in that there's no need to do two comparisons
on a leaf entry, but the main point is just to keep code consistency.
Reported-by: 王跃林 <violin0613@tju.edu.cn>
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/AH*AvQCYKhQGVvPWi1GiU4oY.8.1781609375063.Hmail.3020001251@tju.edu.cn
Backpatch-through: 14
There are a number of rather subtle points about the behavior of
this code, which its original authors did not deign to document.
Try to improve that. In particular, explain how internal and leaf
keys can differ and what the restrictions are on that.
This work arose from trying to fix some bugs, and in the process
I believe I've identified some more, but this patch does not attempt
to fix anything, only document it. I did make a few purely cosmetic
code changes, such as removing dead (and confusing!) initializations
of variables and choosing more appropriate types for some pointers.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/AH*AvQCYKhQGVvPWi1GiU4oY.8.1781609375063.Hmail.3020001251@tju.edu.cn
If we're dealing with leaf entries, the function to call is bitcmp
not byteacmp. Using byteacmp didn't lead to any obvious failure,
but it did result in sorting the entries in a way not matching the
datatype's actual sort order. Hence the constructed index would be
less efficient than one would expect, and in particular worse than
what you got before this code was added in v18 (by commit e4309f73f).
We might want to recommend that users reindex btree_gist indexes
on bit/varbit columns.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/AH*AvQCYKhQGVvPWi1GiU4oY.8.1781609375063.Hmail.3020001251@tju.edu.cn
Backpatch-through: 18
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
A foreign scan can target a single relation while still reaching the
fs_relids branch of pgpa_build_scan() -- for example, when postgres_fdw
pushes an aggregate down over one foreign table. In that case, no
advice should be emitted.
Author: Mahendra Singh Thalor <mahi6run@gmail.com>
Co-authored-by: Robert Haas <rhaas@postgresql.org>
Discussion: http://postgr.es/m/CAKYtNAofuAJBz6++SeikpCb=Y=MO1QgEuZNJ+KZOP2johF1r4Q@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
The float4 and float8 btree_gist opclasses compared keys with raw C
operators (==, <, >). IEEE 754 makes every comparison involving NaN
false, so GiST disagreed with the regular float comparison operators
and with the btree opclass, which uses float[4|8]_cmp_internal()
(so that all NaNs are equal and NaN sorts after every non-NaN value).
In addition, the penalty and distance functions were not careful
about NaNs, and the penalty functions could also misbehave for IEEE
infinities. Wrong answers from the penalty functions would probably
do no more than make the index non-optimal, but the distance mistakes
were visible from SQL.
To fix, make the comparison functions rely on the same NaN-aware
comparison functions the core code uses, and rewrite the penalty
and distance functions to follow the rules that NaNs are equal
but maximally far away from non-NaNs. The penalty_num() code was
formerly shared between integral and float cases, but I chose to make
two copies so that the integral cases are not saddled with the extra
logic for NaNs and infinities/overflows. I also rewrote it as static
inline functions instead of an unreadable and uncommented macro.
The float penalty functions were previously unreached by the
regression tests, so add new test cases to exercise them.
There's no on-disk format change, but users who have NaN entries
in a btree_gist index would be well advised to reindex it.
Bug: #19501
Bug: #19524
Reported-by: Man Zeng <zengman@halodbtech.com>
Reported-by: Yuelin Wang <3020001251@tju.edu.cn>
Author: Bill Kim <billkimjh@gmail.com>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19501-3bff3bbc97f1e7c9@postgresql.org
Discussion: https://postgr.es/m/19524-9559d302c8455664@postgresql.org
Discussion: https://postgr.es/m/CAMQXxcgbtD2LXfX0tpgvOizxP-XxrCHV2ZDy4By_TZnJMsxXWQ@mail.gmail.com
Backpatch-through: 14
