| Age | Commit message (Collapse) | Author |
|---|
|
It wasn't in the documentation at all (even though we document all the
other debugging-like options). Also, change the --help output to show
that it exits after showing, similar to other options.
|
|
Files in common/ and fe_utils/ that contain translatable strings need
to be listed in the nls.mk files of the programs that use them. (Not
great, but that's the way it works for now.) This usually requires
some manual analysis which is done about once during each major
release beta period. This time, I wrote a hackish script that figures
some of this out more automatically, so this update is a bit larger as
it also includes some files that were missed in the past.
|
|
|
|
Make sure that function declarations use names that exactly match the
corresponding names from function definitions in pg_bsd_indent.
This commit was written with help from clang-tidy, by mechanically
applying the same rules as similar clean-up commits.
Discussion: https://postgr.es/m/CAH2-WzkaBS8w-vCbG5M5Bx7XikC0WhNLJV_+Z_YAWW9Kef6OBQ@mail.gmail.com
|
|
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).
|
|
Follow-up to 87d2801d4b: That commit restored some lost error
messages, but they ended up in a place where xgettext wouldn't find
them. Rather than elevating ENCRYPTION_NEGOTIATION_FAILED() to a
gettext trigger, it's easiest for now to put in some explicit
libpq_gettext() calls in the couple of call sites.
|
|
This one error message is just a workaround for a missing OpenSSL
error string. But OpenSSL does not have gettext support, so we don't
need to provide it in our workaround either. That way, the
user-facing behavior is consistent whether the user has a fixed
OpenSSL or not.
|
|
|
|
The do_set_block_offsets() and other functions accessing the tidstore
did not check if the tidstore was NULL. This led to a segmentation
fault when these functions are called without calling the
test_create().
This commit adds NULL checks in relevant functions of test_tidstore to
raise an error instead if the tidstore is not initialized.
Bug: #18483
Reported-by: Alexander Kozhemyakin
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/18483-30bfff42de238000%40postgresql.org
|
|
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
|
|
The nullingrels match checks are not limited to debugging builds.
Oversight in commit 867be9c07.
Author: Richard Guo
Reviewed-by: Alvaro Herrera, Tom Lane, Robert Haas
Discussion: https://postgr.es/m/CAMbWs4_SDsdYD7DdQw7RXc3jv3axbg+RGZ7aSi9GaqX=F8hNVw@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
|
|
test_predtest() neglected to consider the possibility that
SPI_plan_get_cached_plan would return NULL. This led to a core
dump if the input (incorrectly) contains more than one SQL
command.
While here, let's expend more than zero effort on the error
message for this case and nearby ones.
Per (half of) bug #18483 from Alexander Kozhemyakin.
Back-patch to all supported branches, not because this is
very significant (it's merely test scaffolding) but to make
our world a bit safer for fuzz testing.
Discussion: https://postgr.es/m/18483-30bfff42de238000@postgresql.org
|
|
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
|
|
Meson uses warning/debug/optimize flags such as "-Wall", "-g", and
"-O2" automatically based on "--warnlevel" and "--buildtype" options.
And we use "--warning_level=1" and "--buildtype=debugoptimized" by
default.
But we need these flags for Makefile.global (for extensions) and
pg_config, so we need to compute them manually based on the
higher-level options.
Without this change, extensions building using pgxs wouldn't get -Wall
or optimization options.
Author: Sutou Kouhei <kou@clear-code.com>
Discussion: https://www.postgresql.org/message-id/flat/20240122.141139.931086145628347157.kou%40clear-code.com
|
|
This is needed for example to pass an include path set in the CPPFLAGS
environment variable to the bitcode compile command.
Discussion: https://www.postgresql.org/message-id/flat/c1384a7b-ed12-4862-a0da-a05c7945171a%40eisentraut.org
|
|
|
|
Before the v13-era commit 913bbd88d, check_sql_fn_retval fails to
resolve polymorphic output types and then just throws up its hands and
assumes the check will be made at runtime. I think that's true for
ordinary functions returning RECORD, but it doesn't happen in CALL,
potentially resulting in crashes if the actual output of the SQL
procedure's SELECT doesn't match the type inferred from polymorphism.
With a little bit of rearrangement, we can use get_call_result_type
instead of get_func_result_type and thereby infer the correct types.
I'm still unwilling to back-patch all of 913bbd88d, so if the types
don't match you'll get an error rather than perhaps silently inserting
a cast as v13 and later can. That's consistent with prior behavior
though, so it seems fine.
Prior to 70ffb27b2, you'd typically get other errors due to other
shortcomings of CALL's management of polymorphism. Nonetheless,
this is an independent bug.
Although there is no bug in v13 and up, it seems prudent to add
the test case for this to the newer branches too. It's clearly
an under-tested area.
Per report from Andrew Bille.
Discussion: https://postgr.es/m/CAJnzarw9EeWHAQRm76dXd=7j+rgw6ERqC=nCay8jeFqTwKwhqQ@mail.gmail.com
|
|
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.
|
|
Introduced by 9f1337639.
Author: James Coleman <jtc331@gmail.com>
Discussion: https://postgr.es/m/CAAaqYe9ZQ_1+QiF_Nv7b37opicBu+35ZQK1CetQ54r5UdrF1eg@mail.gmail.com
|
|
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
|
|
Oversights in c649fa24a4 which added RETURNING support to MERGE.
Discussion: https://postgr.es/m/CAApHDvpqp6vtUzG-_josUEiBGyqnrnVxJ-VdF+hJLXjHdHzsyQ@mail.gmail.com
|
|
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
|
|
Commit 61461a300c1 accidentally checked memory allocation success
using the wrong variable.
Author: Ranier Vilela <ranier.vf@gmail.com>
Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CAEudQAqQFTH7xCB-+K6zEKjfqbhqCxcr_w4DuJTxVT6h3vzu2w@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 509199587df7.
Reported-by: Ranier Vilela
Author: Ashutosh Bapat
Discussion: https://postgr.es/m/CAEudQAqh_RZqoFcYKso5d9VhF-Vd64_ZodfQ_2zSusszkEmyRg@mail.gmail.com
|
|
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
|
|
A comment of the test was referring to an incorrect table.
Thinko in 1046a69b3087.
Extracted from a larger patch by the same author.
Author: Alexander Lakhin
Discussion: https://postgr.es/m/70b340bc-244a-589d-ef8b-d8aebb707a84@gmail.com
|
|
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.
|
|
Add missing punctuation, and un-gettext-mark an internal error.
|
|
The argument of dontWait at the top of ProcSleep() was documented
backwards, and there was a typo in lock.c.
Thinkos in 2346df6fc373.
Author: Will Mortensen
Reviewed-by: Jingxian Li, Michael Paquier
Discussion: https://postgr.es/m/CAMpnoC5f+eiS7tdy8PUpd_LacSTVT-pYpVooKfjHRQQmkHPZ2g@mail.gmail.com
|
|
This has been supported since c649fa24a.
Discussion: https://postgr.es/m/CAApHDvpqp6vtUzG-_josUEiBGyqnrnVxJ-VdF+hJLXjHdHzsyQ@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
|
|
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
|
|
The previous logic would fail to set groupList when
grouping_is_sortable() returned false, which could cause queries
to return wrong answers when some columns were not sortable.
David Rowley, reviewed by Heikki Linnakangas and Álvaro Herrera.
Reported by Hubert "depesz" Lubaczewski.
Discussion: http://postgr.es/m/Zktzf926vslR35Fv@depesz.com
Discussion: http://postgr.es/m/CAApHDvra=c8_zZT0J-TftByWN2Y+OJfnjNJFg4Dfdi2s+QzmqA@mail.gmail.com
|
|
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.
|
