Currently, when the ON EMPTY clause is not present, the ON ERROR
clause (implicit or explicit) dictates the behavior when jsonpath
evaluation in ExecEvalJsonExprPath() results in an empty sequence.
That is an oversight in the commit 6185c9737c.
This commit fixes things so that a NULL is returned instead in that
case which is the default behavior when the ON EMPTY clause is not
present.
Reported-by: Markus Winand
Discussion: https://postgr.es/m/F7DD1442-265C-4220-A603-CB0DEB77E91D%40winand.at
Previously, GetJsonPathVar() allowed a jsonpath expression to
reference any prefix of a PASSING variable's name. For example, the
following query would incorrectly work:
SELECT JSON_QUERY(context_item, jsonpath '$xy' PASSING val AS xyz);
The fix ensures that the length of the variable name mentioned in a
jsonpath expression matches exactly with the name of the PASSING
variable before comparing the strings using strncmp().
Reported-by: Alvaro Herrera (off-list)
Discussion: https://postgr.es/m/CA+HiwqFGkLWMvELBH6E4SQ45qUHthgcRH6gCJL20OsYDRtFx_w@mail.gmail.com
In cost_memoize_rescan(), when calculating the hit_ratio using the calls
and ndistinct estimations, if the value that was set in
MemoizePath.calls had not been processed through clamp_row_est(), then it
was possible that it was set to some non-integer value which could result
in ndistinct being 1 higher than calls due to estimate_num_groups()
performing clamp_row_est() on its input_rows. This could result in
hit_ratio values slightly below 0.0, which would cause an Assert failure.
The value of MemoizePath.calls comes from the final parameter in the
create_memoize_path() function, of which we only have one true caller of.
That caller passes outer_path->rows. All the core code I looked at
always seems to call clamp_row_est() on the Path.rows, so there might
have been no issues with any core Paths causing troubles here. The bug
report was about a CustomPath with a non-clamped row estimated.
The misbehavior as a result of this seems to be mostly limited to the
Assert() failing. Aside from that, it seems the Memoize costs would
just come out slightly higher than they should have, which is likely
fairly harmless.
Reported-by: Kohei KaiGai <kaigai@heterodb.com>
Discussion: https://postgr.es/m/CAOP8fzZnTU+N64UYJYogb1hN-5hFP+PwTb3m_cnGAD7EsQwrKw@mail.gmail.com
Reviewed-by: Richard Guo
Backpatch-through: 14, where Memoize was introduced
Reconstruction of an SP-GiST index by REINDEX CONCURRENTLY may
insert some REDIRECT tuples. This will typically happen in
a transaction that lacks an XID, which leads either to assertion
failure in spgFormDeadTuple or to insertion of a REDIRECT tuple
with zero xid. The latter's not good either, since eventually
VACUUM will apply GlobalVisTestIsRemovableXid() to the zero xid,
resulting in either an assertion failure or a garbage answer.
In practice, since REINDEX CONCURRENTLY locks out index scans
till it's done, it doesn't matter whether it inserts REDIRECTs
or PLACEHOLDERs; and likewise it doesn't matter how soon VACUUM
reduces such a REDIRECT to a PLACEHOLDER. So in non-assert builds
there's no observable problem here, other than perhaps a little
index bloat. But it's not behaving as intended.
To fix, remove the failing Assert in spgFormDeadTuple, acknowledging
that we might sometimes insert a zero XID; and guard VACUUM's
GlobalVisTestIsRemovableXid() call with a test for valid XID,
ensuring that we'll reduce such a REDIRECT the first time VACUUM
sees it. (Versions before v14 use TransactionIdPrecedes here,
which won't fail on zero xid, so they really have no bug at all
in non-assert builds.)
Another solution could be to not create REDIRECTs at all during
REINDEX CONCURRENTLY, making the relevant code paths treat that
case like index build (which likewise knows that no concurrent
index scans can be happening). That would allow restoring the
Assert in spgFormDeadTuple, but we'd still need the VACUUM change
because redirection tuples with zero xid may be out there already.
But there doesn't seem to be a nice way for spginsert() to tell that
it's being called in REINDEX CONCURRENTLY without some API changes,
so we'll leave that as a possible future improvement.
In HEAD, also rename the SpGistState.myXid field to redirectXid,
which seems less misleading (since it might not in fact be our
transaction's XID) and is certainly less uninformatively generic.
Per bug #18499 from Alexander Lakhin. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/18499-8a519c280f956480@postgresql.org
In the wake of the previous commit, we're not doing anything
with that argument. Hence, revert the portions of 534287403
that added that argument and taught the callers to pass it.
Passing the ownerId requires additional syscache lookups in
some code paths, which'd be fine if we were doing anything
useful with the info, but it seems inadvisable if we're not.
Committed separately since there's some thought that we might
want to un-revert this in future, in case it's decided that
storing the original owner ID explicitly in pg_init_privs
is worth doing.
Discussion: https://postgr.es/m/CAMT0RQSVgv48G5GArUvOVhottWqZLrvC5wBzBa4HrUdXe9VRXw@mail.gmail.com
Commit 534287403 invented SHARED_DEPENDENCY_INITACL entries in
pg_shdepend, but installed them only for non-owner roles mentioned
in a pg_init_privs entry. This turns out to be the wrong thing,
because there is nothing to cue REASSIGN OWNED to go and update
pg_init_privs entries when the object's ownership is reassigned.
That leads to leaving dangling entries in pg_init_privs, as
reported by Hannu Krosing. Instead, install INITACL entries for
all roles mentioned in pg_init_privs entries (except pinned roles),
and change ALTER OWNER to not touch them, just as it doesn't
touch pg_init_privs entries.
REASSIGN OWNED will now substitute the new owner OID for the old
in pg_init_privs entries. This feels like perhaps not quite the
right thing, since pg_init_privs ought to be a historical record
of the state of affairs just after CREATE EXTENSION. However,
it's hard to see what else to do, if we don't want to disallow
dropping the object's original owner. In any case this is
better than the previous do-nothing behavior, and we're unlikely
to come up with a superior solution in time for v17.
While here, tighten up some coding rules about how ACLs in
pg_init_privs should never be null or empty. There's not any
obvious reason to allow that, and perhaps asserting that it's
not so will catch some bugs. (We were previously inconsistent
on the point, with some code paths taking care not to store
empty ACLs and others not.)
This leaves recordExtensionInitPrivWorker not doing anything
with its ownerId argument, but we'll deal with that separately.
catversion bump forced because of change of expected contents
of pg_shdepend when pg_init_privs entries exist.
Discussion: https://postgr.es/m/CAMT0RQSVgv48G5GArUvOVhottWqZLrvC5wBzBa4HrUdXe9VRXw@mail.gmail.com
DeleteInitPrivs did not get the memo about how, when dropping a
whole object (with subid == 0), you should drop entries relating
to its sub-objects too. This is visible in the test_pg_dump test
case if one drops the extension at the end: the entry for
GRANT SELECT(col1) ON regress_pg_dump_table TO public;
was still present in pg_init_privs afterwards, although it was
pointing to a dangling table OID.
Noted while fooling with a fix for REASSIGN OWNED for pg_init_privs
entries. This bug is aboriginal in the pg_init_privs feature
though, and there seems no reason not to back-patch the fix.
Commit 667e65aac3 changed both num_dead_tuples and max_dead_tuples
columns to dead_tuple_bytes and max_dead_tuple_bytes columns,
respectively. But as per discussion, the number of dead tuples
collected still provides meaningful insights for users.
This commit reintroduces the column for the count of dead tuples,
renamed as num_dead_item_ids. It avoids confusion with the number of
dead tuples removed by VACUUM, which includes dead heap-only tuples
but excludes any pre-existing LP_DEAD items left behind by
opportunistic pruning.
Bump catalog version.
Reviewed-by: Peter Geoghegan, Álvaro Herrera, Andrey Borodin
Discussion: https://postgr.es/m/CAD21AoBL5sJE9TRWPyv%2Bw7k5Ee5QAJqDJEDJBUdAaCzGWAdvZw%40mail.gmail.com
The manual says clearly that punctuation in the input of
websearch_to_tsquery() is ignored, except for the special cases
of dashes and quotes. However, this failed for cases like
"(foo bar) or something", or in general an ISOPERATOR character
in front of the "or". We'd switch back to WAITOPERAND state,
then ignore the operator character while remaining in that state,
and then reach the "or" in WAITOPERAND state which (intentionally)
makes us treat it as data.
The fix is simple enough: if we see an ISOPERATOR character while in
WAITOPERATOR state, we have to skip it while staying in that state.
(We don't need to worry about other punctuation characters: those will
be consumed as though they were words, but then rejected by lexizing.)
In v14 and up (since commit eb086056f) we can simplify the code a bit
more too, because there is no longer a reason for the WAITOPERAND
state to distinguish between quoted and unquoted operands.
Per bug #18479 from Manos Emmanouilidis. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/18479-d9b46e2fc242c33e@postgresql.org
The purpose of the function is to reduce the effective
autovacuum_multixact_freeze_max_age if the multixact members SLRU is
approaching wraparound, to make multixid freezing more aggressive.
The returned value should therefore never be greater than plain
autovacuum_multixact_freeze_max_age.
Reviewed-by: Robert Haas
Discussion: https://www.postgresql.org/message-id/85fb354c-f89f-4d47-b3a2-3cbd461c90a3@iki.fi
Backpatch-through: 12, all supported versions
Make sure that function declarations use names that exactly match the
corresponding names from function definitions in a few places. These
inconsistencies were all introduced during Postgres 17 development.
pg_bsd_indent still has a couple of similar inconsistencies, which I
(pgeoghegan) have left untouched for now.
This commit was written with help from clang-tidy, by mechanically
applying the same rules as similar clean-up commits (the earliest such
commit was commit 035ce1fe).
infer_arbiter_indexes failed to renumber varnos in index expressions
or predicates that it got from the catalogs. This escaped detection
up to now because the stored varnos in such trees will be 1, and an
INSERT's result relation is usually the first rangetable entry,
so that that was fine. However, in cases such as inserting through
an updatable view, it's not fine, leading to failure to match the
expressions to the query with ensuing "there is no unique or exclusion
constraint matching the ON CONFLICT specification" errors.
Fix by copy-and-paste from get_relation_info().
Per bug #18502 from Michael Wang. Back-patch to all supported
versions.
Discussion: https://postgr.es/m/18502-545b53f5b81e54e0@postgresql.org
When a partition is being detached in concurrent mode, it is possible
for find_inheritance_children_extended() to return that partition in the
list, and immediately after that receive an invalidation message that
sets its relpartbound to NULL just before we read it. (This can happen
because table_open() reads invalidation messages.) Currently we raise
an error
ERROR: missing relpartbound for relation %u
about the situation, but that's bogus because the table is no longer a
partition, so we shouldn't be complaining about it. A better reaction
is to retry the find_inheritance_children_extended call to get a new
list, which will no longer have the partition being detached.
Noticed while investigating bug #18377.
Backpatch to 14, where DETACH CONCURRENTLY appeared.
Discussion: https://postgr.es/m/202405201616.y4ht2qe5ihoy@alvherre.pgsql
Commit e0b2eed047 assumed that the confirmed_flush LSN can't go backward.
However, it is possible that confirmed_flush LSN can go backward
temporarily when the client acknowledges a prior value of flush location.
This can happen when the client (subscriber in this case) acknowledges an
LSN it doesn't have to do anything for (say for DDLs) and thus didn't
store persistently. After restart, the client sends the prior value of
flush LSN which it had stored persistently and the server updates the
confirmed_flush LSN with that value.
The fix is to remove the assumption and not allow the prior value of
confirmed_flush LSN to be flushed to the disk.
Author: Vignesh C
Reviewed-by: Amit Kapila, Shlok Kyal
Discussion: https://postgr.es/m/CALDaNm3hgow2+oEov5jBk4iYP5eQrUCF1yZtW7+dV3J__p4KLQ@mail.gmail.com
Commit 210622c6 accidentally zeroed out pages even if they were found in
the buffer pool. It should always lock the page, but it should only
zero pages that were not already valid. Otherwise, concurrent readers
that hold only a pin could see corrupted page contents changing under
their feet.
While here, rename ZeroAndLockBuffer() to match the RBM_ flag name.
Also restore a some useful comments lost by 210622c6's refactoring, and
add some new ones to clarify why we need to use the BM_IO_IN_PROGRESS
infrastructure despite not doing I/O.
Reported-by: Noah Misch <noah@leadboat.com>
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> (earlier version)
Reviewed-by: Robert Haas <robertmhaas@gmail.com> (earlier version)
Discussion: https://postgr.es/m/20240512171658.7e.nmisch@google.com
Discussion: https://postgr.es/m/7ed10231-ce47-03d5-d3f9-4aea0dc7d5a4%40gmail.com
Normally this case isn't even reachable by non-superusers, since
permissions checks prevent naming such a table. However, it is
possible to make it happen by altering a parent table whose child
is another session's temp table.
We definitely can't support any such ALTER that requires modifying
the contents of such a table, since we lack access to the other
session's temporary-buffer pool. But there seems no good reason
to allow it even if it'd only require changing catalog contents.
One reason not to allow it is that we'd rather not expose the
implementation-dependent behavior of whether a specific ALTER
requires touching the table contents. Another is that there may
be (in future, even if not today) optimizations that assume that
a session's own temp tables won't be modified by other sessions.
Hence, add a RELATION_IS_OTHER_TEMP() check to all the places
where ALTER TABLE currently does CheckTableNotInUse(). (I looked
through all other callers of CheckTableNotInUse(), and they seem
OK already.)
Per bug #18492 from Alexander Lakhin. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/18492-c7a2634bf4968763@postgresql.org
If the CALL is within an atomic context (e.g. there's an outer
transaction block), _SPI_execute_plan should acquire a fresh snapshot
to execute any such functions with. We failed to do that and instead
passed them the Portal snapshot, which had been acquired at the start
of the current SQL command. This'd lead to seeing stale values of
rows modified since the start of the command.
This is arguably a bug in 84f5c2908: I failed to see that "are we in
non-atomic mode" needs to be defined the same way as it is further
down in _SPI_execute_plan, i.e. check !_SPI_current->atomic not just
options->allow_nonatomic. Alternatively the blame could be laid on
plpgsql, which is unconditionally passing allow_nonatomic = true
for CALL/DO even when it knows it's in an atomic context. However,
fixing it in spi.c seems like a better idea since that will also fix
the problem for any extensions that may have copied plpgsql's coding
pattern.
While here, update an obsolete comment about _SPI_execute_plan's
snapshot management.
Per report from Victor Yegorov. Back-patch to all supported versions.
Discussion: https://postgr.es/m/CAGnEboiRe+fG2QxuBO2390F7P8e2MQ6UyBjZSL_w1Cej+E4=Vw@mail.gmail.com
Floris Van Nee has reported a bug in the pgstats facility where a stats
entry already dropped would get again dropped. This case should not
happen, still the error generated did not offer any details about the
stats entry getting dropped.
This commit improves the error message generated to inform about the
stats entry kind, database OID, object OID and refcount, which should
help to debug more the problem reported. Bertrand Drouvot has been
independently able to reach this error path while writing a new feature,
and more details about the failure would have been helpful for
debugging.
Author: Andres Freund, Bertrand Drouvot
Discussion: https://postgr.es/m/20240505160915.6boysum4f34siqct@awork3.anarazel.de
Discussion: https://postgr.es/m/ZkM30paAD8Cr/Bix@ip-10-97-1-34.eu-west-3.compute.internal
Backpatch-through: 15
ReorderBufferImmediateInvalidation() executes invalidation messages in
an aborted transaction. However, RelationFlushRelation sometimes
required a valid resource owner, to temporarily increment the refcount
of the relache entry. Commit b8bff07daa worked around that in the main
subtransaction abort function, AbortSubTransaction(), but missed this
similar case in ReorderBufferImmediateInvalidation().
To fix, introduce a separate function to invalidate a relcache
entry. It does the same thing as RelationClearRelation(rebuild==true)
does when outside a transaction, but can be called without
incrementing the refcount.
Add regression test. Before this fix, it failed with:
ERROR: ResourceOwnerEnlarge called after release started
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://www.postgresql.org/message-id/e56be7d9-14b1-664d-0bfc-00ce9772721c@gmail.com
0452b461bc made optimizer explore alternative orderings of group-by pathkeys.
It eliminated preprocess_groupclause(), which was intended to match items
between GROUP BY and ORDER BY. Instead, get_useful_group_keys_orderings()
function generates orderings of GROUP BY elements at the time of grouping
paths generation. The get_useful_group_keys_orderings() function takes into
account 3 orderings of GROUP BY pathkeys and clauses: original order as written
in GROUP BY, matching ORDER BY clauses as much as possible, and matching the
input path as much as possible. Given that even before 0452b461b,
preprocess_groupclause() could change the original order of GROUP BY clauses
we don't need to consider it apart from ordering matching ORDER BY clauses.
This commit restores preprocess_groupclause() to provide an ordering of
GROUP BY elements matching ORDER BY before generation of paths. The new
version of preprocess_groupclause() takes into account an incremental sort.
The get_useful_group_keys_orderings() function now takes into 2 orderings of
GROUP BY elements: the order generated preprocess_groupclause() and the order
matching the input path as much as possible.
Discussion: https://postgr.es/m/CAPpHfdvyWLMGwvxaf%3D7KAp-z-4mxbSH8ti2f6mNOQv5metZFzg%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Andrei Lepikhov, Pavel Borisov
0452b461bc made optimizer explore alternative orderings of group-by pathkeys.
The PathKeyInfo data structure was used to store the particular ordering of
group-by pathkeys and corresponding clauses. It turns out that PathKeyInfo
is not the best name for that purpose. This commit renames this data structure
to GroupByOrdering, and revises its comment.
Discussion: https://postgr.es/m/db0fc3a4-966c-4cec-a136-94024d39212d%40postgrespro.ru
Reported-by: Tom Lane
Author: Andrei Lepikhov
Reviewed-by: Alexander Korotkov, Pavel Borisov
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
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.
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 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 4908c58720.
Author: Xing Guo
Discussion: https://postgr.es/m/CACpMh+BM-VgKeO7suPG-VHTtpzJ+zsbDPwVHu42PLp-iTk0z+A@mail.gmail.com
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 509199587d.
Reported-by: Ranier Vilela
Author: Ashutosh Bapat
Discussion: https://postgr.es/m/CAEudQAqh_RZqoFcYKso5d9VhF-Vd64_ZodfQ_2zSusszkEmyRg@mail.gmail.com
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
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 7204f35919,
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 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)
After further review, we want to move in the direction of always
quoting GUC names in error messages, rather than the previous (PG16)
wildly mixed practice or the intermittent (mid-PG17) idea of doing
this depending on how possibly confusing the GUC name is.
This commit applies appropriate quotes to (almost?) all mentions of
GUC names in error messages. It partially supersedes a243569bf6 and
8d9978a717, which had moved things a bit in the opposite direction
but which then were abandoned in a partial state.
Author: Peter Smith <smithpb2250@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAHut%2BPv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w%40mail.gmail.com
There were a few typedefs that were never used to define a variable or
field. This had the effect that the buildfarm's typedefs.list would
not include them, and so they would need to be re-added manually to
keep the overall pgindent result perfectly clean.
We can easily get rid of these typedefs to avoid the issue. In a few
cases, we just let the enum or struct stand on its own without a
typedef around it. In one case, an enum was abused to define flag
bits; that's better done with macro definitions.
This fixes all the remaining issues with missing entries in the
buildfarm's typedefs.list.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/1919000.1715815925@sss.pgh.pa.us
This enum was used to determine the first ID to use when assigning a
custom wait event for extensions, which is always 1. It was kept so
as it would be possible to add new in-core wait events in the category
"Extension". There is no such thing currently, so let's remove this
enum until a case justifying it pops up. This makes the code simpler
and easier to understand.
This has as effect to switch back autoprewarm.c to use PG_WAIT_EXTENSION
rather than WAIT_EVENT_EXTENSION, on par with v16 and older stable
branches.
Thinko in c9af054653.
Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/195c6c45-abce-4331-be6a-e87724e1d060@eisentraut.org
Specifically, it terminates a background worker even if the caller
couldn't terminate the background worker with pg_terminate_backend().
Commit 3a9b18b309 neglected to update
this. Back-patch to v13, which introduced DROP DATABASE FORCE.
Reviewed by Amit Kapila. Reported by Kirill Reshke.
Discussion: https://postgr.es/m/20240429212756.60.nmisch@google.com
04e72ed617 pushed the skip fetch optimization (allowing bitmap heap
scans to operate like index-only scans if none of the underlying data is
needed) into heap AM implementations of bitmap table scan callbacks.
04e72ed617 added an assert that all tuples in blocks eligible for the
optimization had been NULL-filled and emitted by the end of the scan.
This assert is incorrect when not all tuples need be scanned to execute
the query; for example: a join in which not all inner tuples need to be
scanned before skipping to the next outer tuple.
Remove the assert and reset the field on which it previously asserted to
avoid incorrectly emitting NULL-filled tuples from a previous scan on
rescan.
Author: Melanie Plageman
Reviewed-by: Tomas Vondra, Michael Paquier, Alvaro Herrera
Reported-by: Melanie Plageman
Reproduced-by: Tomas Vondra, Richard Guo
Discussion: https://postgr.es/m/CAMbWs48orzZVXa7-vP9Nt7vQWLTE04Qy4PePaLQYsVNQgo6qRg%40mail.gmail.com
