| Commit message (Collapse) | Author | Age |
|---|
| ... | |
| | |
|
| |
|
|
|
| |
Oversight in 16acf7f1aaea, where the test has been introduced. Issue
noticed while scanning this area of the tree.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
|
|
|
| |
This was an ommission in commit 66ea94e, and brings it into compliance
with both other methods and the standard.
Per complaint from David Wheeler.
Author: David Wheeler, Jeevan Chalke
Reviewed-by: Chapman Flack
Discussion: https://postgr.es/m/A64AE04F-4410-42B7-A141-7A7349260F4D@justatheory.com
|
| |
|
|
|
|
|
|
|
| |
After running pg_createsubscriber, these replication slots have no use
on subscriber, so drop them.
Author: Euler Taveira <euler.taveira@enterprisedb.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://www.postgresql.org/message-id/776c5cac-5ef5-4001-b1bc-5b698bc0c62a%40app.fastmail.com
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
It used to check if the replication slot exists and is active on
primary. This check might fail on slow hosts because the replication
slot might not be active at the time of this check.
The current code obtains the replication slot name from the
primary_slot_name on standby and assumes the replication slot exists
and is active on primary. If it doesn't exist, this tool will log an
error and continue.
Author: Euler Taveira <euler.taveira@enterprisedb.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://www.postgresql.org/message-id/776c5cac-5ef5-4001-b1bc-5b698bc0c62a%40app.fastmail.com
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
process
It used to check if the target server is connected to the primary
server (send required WAL) to rapidly react when the process won't
succeed. This code is not enough to guarantee that the recovery
process will complete. There is a window between the walreceiver
shutdown and the pg_is_in_recovery() returns false that can reach
NUM_CONN_ATTEMPTS attempts and fails.
Instead, rely only on the --recovery-timeout option to give up the
process after the specified number of seconds.
This should help with buildfarm failures on slow machines.
Author: Euler Taveira <euler.taveira@enterprisedb.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://www.postgresql.org/message-id/776c5cac-5ef5-4001-b1bc-5b698bc0c62a%40app.fastmail.com
|
| |
|
|
|
|
|
|
|
|
|
| |
The function has been introduced in 368202501539 to test at a low level
the new kinds of external toast datums, and would fail on OOM when
dealing with a plain storage attribute. The existing tests of
indirect_toast do not test this case, still the error generated was
confusing.
Author: Alexander Lakhin
Discussion: https://postgr.es/m/250a21e5-d677-6b2a-2692-cd4233785e37@gmail.com
|
| | |
|
| |
|
|
|
| |
As in 9d7ded0f4277f5c0063eca8e871a34e2355a8371, it causes warnings on
some MinGW compilers.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
The macros were confused about the argument data types. All the
arguments were called 'xid', and some of the macros included casts to
TransactionId, even though the arguments were actually either
MultiXactIds or MultiXactOffsets. It compiles to the same thing,
because TransactionId, MultiXactId and MultiXactOffset are all
typedefs of uint32, but it was highly misleading.
Author: Maxim Orlov <orlovmg@gmail.com>
Discussion: https://www.postgresql.org/message-id/CACG%3DezbLUG-OD1osAW3OchOMxZtdxHh2itYR9Zhh-a13wEBEQw%40mail.gmail.com
Discussion: https://www.postgresql.org/message-id/ff143b24-a093-40da-9833-d36b83726bdf%40iki.fi
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
| |
Oversight in 534287403. We missed this up to now because the
core regression tests create no such entries (at least up to
this test), so the only way to see the failure is to do
"make installcheck" in an installation where some other DB
has such entries. I happened to do that just now ...
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Commit f5e4dedfa exposed libpq's internal function PQsocketPoll
without a lot of thought about whether that was an API we really
wanted to chisel in stone. The main problem with it is the use of
time_t to specify the timeout. While we do want an absolute time
so that a loop around PQsocketPoll doesn't have problems with
timeout slippage, time_t has only 1-second resolution. That's
already problematic for libpq's own internal usage --- for example,
pqConnectDBComplete has long had a kluge to treat "connect_timeout=1"
as 2 seconds so that it doesn't accidentally round to nearly zero.
And it's even less likely to be satisfactory for external callers.
Hence, let's change this while we still can.
The best idea seems to be to use an int64 count of microseconds since
the epoch --- basically the same thing as the backend's TimestampTz,
but let's use the standard Unix epoch (1970-01-01) since that's more
likely for clients to be easy to calculate. Millisecond resolution
would be plenty for foreseeable uses, but maybe the day will come that
we're glad we used microseconds.
Also, since time(2) isn't especially helpful for computing timeouts
defined this way, introduce a new function PQgetCurrentTimeUSec
to get the current time in this form.
Remove the hack in pqConnectDBComplete, so that "connect_timeout=1"
now means what you'd expect.
We can also remove the "#include <time.h>" that f5e4dedfa added to
libpq-fe.h, since there's no longer a need for time_t in that header.
It seems better for v17 not to enlarge libpq-fe.h's include footprint
from what it's historically been, anyway.
I also failed to resist the temptation to do some wordsmithing
on PQsocketPoll's documentation.
Patch by me, per complaint from Dominique Devienne.
Discussion: https://postgr.es/m/913559.1718055575@sss.pgh.pa.us
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The previous coding here assumed that we didn't need to recheck any
of the querytree tests made in exec_simple_check_plan(). I think
we supposed that those properties were fully determined by the
syntax of the source text and hence couldn't change. That is true
for most of them, but at least hasTargetSRFs and hasAggs can change
by dint of forcibly dropping an originally-referenced function and
recreating it with new properties. That leads to "unexpected plan
node type" or similar failures.
These tests are pretty cheap compared to the cost of replanning, so
rather than sweat over exactly which properties need to be rechecked,
let's just recheck them all. Hence, factor out those tests into a new
function exec_is_simple_query(), and rearrange callers as needed.
A second problem in the same area was that if we failed during
replanning or during exec_save_simple_expr(), we'd potentially
leave behind now-dangling pointers to the old simple expression,
potentially resulting in crashes later. To fix, clear those pointers
before replanning.
The v12 code looks quite different in this area but still has the
bug about needing to recheck query simplicity. I chose to back-patch
all of the plpgsql_simple.sql test script, which formerly didn't exist
in this branch.
Per bug #18497 from Nikita Kalinin. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/18497-fe93b6da82ce31d4@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
|
| |
|
|
|
|
| |
These are checks that are already skipped on other Windows systems.
Backpatch to all live branches, as appropriate.
|
| |
|
|
|
|
| |
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.
|
