This commit introduces invariants checking of generated orderings
in get_useful_group_keys_orderings() for assert-enabled builds.
Discussion: https://postgr.es/m/a663f0f6-cbf6-49aa-af2e-234dc6768a07%40postgrespro.ru
Reported-by: Tom Lane
Author: Andrei Lepikhov
Reviewed-by: Alexander Korotkov, Pavel Borisov
0452b461bc made get_eclass_for_sort_expr() always set
EquivalenceClass.ec_sortref if it's not done yet. This leads to an asymmetric
situation when whoever first looks for the EquivalenceClass sets the
ec_sortref. It is also counterintuitive that get_eclass_for_sort_expr()
performs modification of data structures.
This commit makes make_pathkeys_for_sortclauses_extended() responsible for
setting EquivalenceClass.ec_sortref. Now we set the
EquivalenceClass.ec_sortref's needed to explore alternative GROUP BY ordering
specifically during building pathkeys by the list of grouping clauses.
Discussion: https://postgr.es/m/17037754-f187-4138-8285-0e2bfebd0dea%40postgrespro.ru
Reported-by: Tom Lane
Author: Andrei Lepikhov
Reviewed-by: Alexander Korotkov, Pavel Borisov
Concurrent activity around replication slot creation and drop could
cause a replication slot to use a stats entry it should not have used
when created, triggering an assertion failure when retrieving an
inconsistent entry from the dshash table used by the stats facility.
The issue is that pgstat_drop_replslot() calls pgstat_drop_entry()
without checking the result. If pgstat_drop_entry() cannot free the
entry related to the object dropped, pgstat_request_entry_refs_gc()
should be called. AtEOXact_PgStat_DroppedStats() and surrounding
routines dropping stats entries already do that.
This is documented in pgstat_internal.h, but let's add a comment at the
top of pgstat_drop_entry() as that can be easy to miss.
Reported-by: Alexander Lakhin
Author: Floris Van Nee
Analyzed-by: Andres Freund
Discussion: https://postgr.es/m/17947-b9554521ad963c9c@postgresql.org
Backpatch-through: 15
The documentation for POSIX semaphores is missing a reference to
max_wal_senders. This commit fixes that in the same way that
commit 4ebe51a5fb fixed the same issue in the documentation for
System V semaphores.
Discussion: https://postgr.es/m/20240517164452.GA1914161%40nathanxps13
Backpatch-through: 12
They were under "File Locations", which doesn't make sense. Move them
to Resource Usage / Memory, which matches their categorization in the
source code and in the documentation.
This HBA entry was using "local" while specifying an address, which was
incorrect. While in it, this adjusts the format of the entry to be
aligned with the surroundings.
Oversight in 8fea86830e1d.
Reported-by: Stéphane Schildknecht
Reviewed-by: David G. Johnston
Discussion: https://postgr.es/m/44662001-54c4-4bfd-be93-35e01ca25fa1@gmail.com
Backpatch-through: 16
In a procedure or function returning tuple, we use that function to
parse the Tcl script's result, which is supposed to be a Tcl list.
If it isn't, you get an error. Commit 26abb50c4 incautiously
supposed that we could use throw_tcl_error() to report such an error.
That doesn't actually work, because low-level functions like
Tcl_ListObjGetElements() don't fill Tcl's errorInfo variable.
The result is either a null-pointer-dereference crash or emission
of misleading context information describing the previous Tcl error.
Back off to just reporting the interpreter's result string, and
improve throw_tcl_error()'s comment to explain when to use it.
Also, although the similar code in pltcl_trigger_handler() avoided
this mistake, it was using a fairly confusing wording of the
error message. Improve that while we're here.
Per report from A. Kozhemyakin. Back-patch to all supported
branches.
Erik Wienhold and Tom Lane
Discussion: https://postgr.es/m/6a2a1c40-2b2c-4a33-8b72-243c0766fcda@postgrespro.ru
Commit faff8f8e47 allowed integer literals to contain underscores, but
failed to update the lexer's "numericfail" rule. As a result, a
decimal integer literal containing underscores would fail to parse, if
used in an integer range with no whitespace after the first number,
such as "1_001..1_003" in a PL/pgSQL FOR loop.
Fix and backpatch to v16, where support for underscores in integer
literals was added.
Report and patch by Erik Wienhold.
Discussion: https://postgr.es/m/808ce947-46ec-4628-85fa-3dd600b2c154%40ewie.name
The VS and MinGW Windows images have been merged, to reduce the space needed
for images. Before 98811323c8e the split helped boot performance, but now that
we are using VMs that doesn't appear to be the case anymore.
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/CAN55FZ2kWYjPd7uUC5QswrB3tfVJDiURqC%2BMGM6a3oeev%3DVgOA%40mail.gmail.com
Backpatch: 15-, where CI was added
The assertion used at the beginning of mdwritev(), that is not enabled
except by defining -DCHECK_WRITE_VS_EXTEND as mdnblocks() is costly,
forgot about the total number of blocks to write at location specified
by the caller. The calculation is fixed to count for that, and uses
casts to uint64 to ensure a proper check should the number of blocks
overflow.
Using a cast is a suggestion from Tom Lane.
Oversight in 4908c5872059.
Author: Xing Guo
Discussion: https://postgr.es/m/CACpMh+BM-VgKeO7suPG-VHTtpzJ+zsbDPwVHu42PLp-iTk0z+A@mail.gmail.com
The formulas for SEMMNI and SEMMNS do not include the archiver
process, which was converted to an auxiliary process in v14, and
the WAL summarizer process, which was introduced in v17. This
commit corrects these formulas and adds a missing reference to
max_wal_senders nearby. Since this section of the documentation
tends to be incorrect quite often, we should likely give up on
documenting the exact formulas in favor of something less fragile,
but that is left as a future exercise.
Reported-by: Sami Imseih
Reviewed-by: Sami Imseih
Discussion: https://postgr.es/m/20240517164452.GA1914161%40nathanxps13
Backpatch-through: 12
The OSSP uuid.h file includes unistd.h, so to use it with MSVC we need to
include the postgres include directories so it picks up our version of
that in src/include/port/win32_msvc. Adjust the meson test accordingly.
The function invokes SearchSysCacheAttNum() and SearchSysCacheAttName().
They may respectively return 0 for the attribute number or NULL for
the attribute name if the attribute does not exist, without any kind of
error handling. The common practice is to check that the data retrieved
from the syscache is valid. There is no risk of NULL pointer
dereferences currently, but let's stick to the practice of making sure
that this data is always valid, to catch future inconsistency mistakes.
The code is switched to use get_attnum() and get_attname(), and adds
some error handling.
Oversight in 509199587df7.
Reported-by: Ranier Vilela
Author: Ashutosh Bapat
Discussion: https://postgr.es/m/CAEudQAqh_RZqoFcYKso5d9VhF-Vd64_ZodfQ_2zSusszkEmyRg@mail.gmail.com
* Don't forget to pfree() the right page when it's to be ignored.
* Report error on unexpected non-leaf right page even if this page is not
to be ignored. This restores the logic which was unintendedly changed
in 97e5b0026f.
Reported-by: Pavel Borisov
System catalog tables are subject to modification by parallel tests. This
is the source of instability when querying them without explicit ORDER BY.
This commit adds explicit ORDER BY to system catalog queries in
partition_split.sql to stabilize the result.
Reported-by: Tom Lane
Discussion: https://postgr.es/m/695264.1716578979%40sss.pgh.pa.us
This test was failing when using wal_debug=on and -DWAL_DEBUG because of
additional log entries that made the test grab an LSN not mapping with
the error expected in the test.
Previously the test would look for the first matching line to get the
LSN to skip up to. This is improved by having the test scan the logs
with a regexp that checks for the expected ERROR string, ensuring that
the wanted LSN comes from the correct context.
Backpatch down to 15 where this test has been introduced.
Author: Ian Ilyasov
Discussion: https://postgr.es/m/GV1P251MB100415F17E6B2FDD7188777ECDE32@GV1P251MB1004.EURP251.PROD.OUTLOOK.COM
Backpatch-through: 15
Coverity complains that ECPGdebug is accessing debugstream without
holding debug_mutex, which is a fair complaint: we should take
debug_mutex while changing the settings ecpg_log looks at.
In some branches it also complains about unlocked use of simple_debug.
I think it's intentional and safe to have a quick unlocked check of
simple_debug at the start of ecpg_log, since that early exit will
always be taken in non-debug cases. But we should recheck
simple_debug after acquiring the mutex. In the worst case, calling
ECPGdebug concurrently with ecpg_log in another thread could result
in a null-pointer dereference due to debugstream transiently being
NULL while simple_debug isn't 0.
This is largely hypothetical, since it's unlikely anybody uses
ECPGdebug() at all in the field, and our own regression tests
don't seem to be hitting the theoretical race conditions either.
Still, if we're going to the trouble of having mutexes here, we ought
to be using them in a way that's actually safe not just almost safe.
Hence, back-patch to all supported branches.
Parameter column_name must be an existing column because ALTER
MATERIALIZED VIEW cannot add new columns. The old description was
likely copied from ALTER TABLE.
Author: Erik Wienhold
Discussion: https://postgr.es/m/6880ca53-7961-4eeb-86d5-6bd05fc2027e@ewie.name
Backpatch-through: 12
When MERGE/SPLIT created new partitions, it was cloning the extended
statistics of the parent table.
However, extended stats on partitioned tables don't behave like
indexes on partitioned tables (which exist only to create physical
indexes on child tables). Rather, extended stats on a parent 1) cause
extended stats to be collected and computed across the whole partition
hierarchy, and 2) do not cause extended stats to be computed for the
individual partitions.
"CREATE TABLE ... PARTITION OF" command doesn't copy extended
statistics. This commit makes createPartitionTable() behave
consistently.
Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/ZiJW1g2nbQs9ekwK%40pryzbyj2023
Author: Alexander Korotkov, Justin Pryzby
Both MERGE and SPLIT partition operations support the case when the name of the
new partition matches the name of the existing partition to be merged/split.
But the name collision detection doesn't always work as intended. The SPLIT
partition operation finds the namespace to search for an existing partition
without taking into account the parent's persistence. The MERGE partition
operation checks for the name collision with simple equal() on RangeVar's
simply ignoring the search_path.
This commit fixes this behavior as follows.
1. The SPLIT partition operation now finds the namespace to search for
an existing partition according to the parent's persistence.
2. The MERGE partition operation now checks for the name collision similarly
to the SPLIT partition operation using
RangeVarGetAndCheckCreationNamespace() and get_relname_relid().
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/86b4f1e3-0b5d-315c-9225-19860d64d685%40gmail.com
Author: Dmitry Koval, Alexander Korotkov
This is a very unlikely condition during checking a B-tree unique constraint,
meaning that the index structure is violated badly, and we shouldn't continue
checking to avoid endless loops, etc. So it's worth immediately throwing an
error.
Reported-by: Peter Geoghegan
Discussion: https://postgr.es/m/CAH2-Wzk%2B2116uOXdOViA27SHcr31WKPgmjsxXLBs_aTxAeThzg%40mail.gmail.com
Author: Pavel Borisov
5ae2087202 implemented a cross-page unique constraint check by loading
the right sibling to the BtreeCheckState.target variable. This is wrong,
because bt_target_page_check() shouldn't change the target page. Also,
BtreeCheckState.target shouldn't be changed alone without
BtreeCheckState.targetblock.
However, the above didn't cause any visible bugs for the following reasons.
1. We do a cross-page unique constraint check only for leaf index pages.
2. The only way target page get accessed after a cross-page unique constraint
check is loading of the lowkey.
3. The only place lowkey is used is bt_child_highkey_check(), and that applies
only to non-leafs.
The reasons above don't diminish the fact that changing BtreeCheckState.target
for a cross-page unique constraint check is wrong. This commit changes this
check to temporarily store the right sibling to the local variable.
Reported-by: Peter Geoghegan
Discussion: https://postgr.es/m/CAH2-Wzk%2B2116uOXdOViA27SHcr31WKPgmjsxXLBs_aTxAeThzg%40mail.gmail.com
Author: Pavel Borisov
This commit introduces a new data structure BtreeLastVisibleEntry comprising
information about the last visible heap entry with the current value of key.
Usage of this data structure allows us to avoid passing all this information
as individual function arguments.
Reported-by: Alexander Korotkov
Discussion: https://www.postgresql.org/message-id/CAPpHfdsVbB9ToriaB1UHuOKwjKxiZmTFQcEF%3DjuzzC_nby31uA%40mail.gmail.com
Author: Pavel Borisov, Alexander Korotkov
Commit 3e1a373e2 missed teaching DecodeTimeOnly the same "ptype"
manipulations it added to DecodeDateTime. While likely harmless
at the time, it became a problem after 5b3c59535 added an error check
that ptype must be zero once we exit the parsing loop (that is, there
shouldn't be any unused prefixes). The consequence was that we'd
reject time or timetz input like T12:34:56 (the "extended" format
per ISO 8601-1:2019), even though that still worked in timestamp
input.
Since this is clearly under-tested code, add test cases covering all
the ISO 8601 time formats. (Note: although 8601 allows just "Thh",
we have never accepted that, and this patch doesn't change that.
I'm content to leave that as-is because it seems too likely to be
a mistake rather than intended input. If anyone wants to allow
that, it should be a separate patch anyway, and not back-patched.)
Per bug #18470 from David Perez. Back-patch to v16 where we
broke it.
Discussion: https://postgr.es/m/18470-34fad4c829106848@postgresql.org
transformTableLikeClause believed that it could process extended
statistics immediately because "the representation of CreateStatsStmt
doesn't depend on column numbers". That was true when extended stats
were first introduced, but it was falsified by the addition of
extended stats on expressions: the parsed expression tree is fed
forward by the LIKE option, and that will contain Vars. So if the
new table doesn't have attnums identical to the old one's (typically
because there are some dropped columns in the old one), that doesn't
work. The CREATE goes through, but it emits invalid statistics
objects that will cause problems later.
Fortunately, we already have logic that can adapt expression trees
to the possibly-new column numbering. To use it, we have to delay
processing of CREATE_TABLE_LIKE_STATISTICS into expandTableLikeClause,
just as for other LIKE options that involve expressions.
Per bug #18468 from Alexander Lakhin. Back-patch to v14 where
extended statistics on expressions were added.
Discussion: https://postgr.es/m/18468-f5add190e3fa5902@postgresql.org
This reverts commit 7204f35919b7e021e8d1bc9f2d76fd6bfcdd2070,
thus restoring 66c0185a3 (Allow planner to use Merge Append to
efficiently implement UNION) as well as the follow-on commits
d5d2205c8, 3b1a7eb28, 7487044d6.
Per further discussion on pgsql-release, we wish to ship beta1 with
this feature, and patch the bug that was found just before wrap,
rather than shipping beta1 with the feature reverted.
This reverts commit e1e83a71715f1c2f0717bb66e630ca3bda79b9c1.
Per further discussion on pgsql-release, we wish to ship beta1 with
that feature, and patch the bug, rather than shipping beta1 with
that feature reverted.
This reverts 66c0185a3 (Allow planner to use Merge Append to
efficiently implement UNION) as well as the follow-on commits
d5d2205c8, 3b1a7eb28, 7487044d6. In addition to those, 07746a8ef
had to be removed then re-applied in a different place, because
66c0185a3 moved the relevant code.
The reason for this last-minute thrashing is that depesz found a
case in which the patched code creates a completely wrong plan
that silently gives incorrect query results. It's unclear what
the cause is or how many cases are affected, but with beta1 wrap
staring us in the face, there's no time for closer investigation.
After we figure that out, we can decide whether to un-revert this
for beta2 or hold it for v18.
Discussion: https://postgr.es/m/Zktzf926vslR35Fv@depesz.com
(also some private discussion among pgsql-release)
