When sending data for logical decoding using the streaming replication
protocol via a WAL sender, the timestamp of the sent write message is
allocated at the beginning of the message when preparing for the write,
and actually computed when the write message is ready to be sent.
The timestamp was getting computed after sending the message. This
impacts anything using logical decoding, causing for example logical
replication to report mostly NULL for last_msg_send_time in
pg_stat_subscription.
This commit makes sure that the timestamp is computed before sending the
message. This is wrong since 5a991ef, so backpatch down to 9.4.
Author: Jeff Janes
Discussion: https://postgr.es/m/CAMkU=1z=WMn8jt7iEdC5sYNaPgAgOASb_OW5JYv-vMdYaJSL-w@mail.gmail.com
Backpatch-through: 9.4
WindowAgg will potentially store large numbers of input rows into
tuplestores to allow access to other rows in the frame. If the input
is coming via an explicit Sort node, then unneeded columns will
already have been discarded (since Sort requests a small tlist); but
there are idioms like COUNT(*) OVER () that result in the input not
being sorted at all, and cases where the input is being sorted by some
means other than a Sort; if we don't request a small tlist, then
WindowAgg's storage requirement is inflated by the unneeded columns.
Backpatch back to 9.6, where the current tlist handling was added.
(Prior to that, WindowAgg would always use a small tlist.)
Discussion: https://postgr.es/m/87a7ator8n.fsf@news-spur.riddles.org.uk
Previously ALTER MATERIALIZED VIEW / FOREIGN TABLE ... RENAME COLUMN ...
returned "ALTER TABLE" as a command tag. This commit fixes them so that
they return "ALTER MATERIALIZED VIEW" and "ALTER FOREIGN TABLE" as
command tags, respectively.
This issue exists in all supported versions, but we don't back-patch this
because it's not enough of a bug to justify taking any compatibility risks for.
Otherwise, the back-patch would cause minor version update to break,
for example, the existing event trigger functions using TG_TAG.
Author: Fujii Masao
Reviewed-by: Ibrar Ahmed
Discussion: https://postgr.es/m/CAHGQGwGUaC03FFdTFoHsCuDrrNvFvNVQ6xyd40==P25WvuBJjg@mail.gmail.com
This commit allows --init-steps option in pgbench to accept "G" character
meaning server-side data generation as an initialization step.
With "G", only limited queries are sent from pgbench client and
then data is actually generated in the server. This might make
the initialization phase faster if the bandwidth between pgbench client
and the server is low.
Author: Fabien Coelho
Reviewed-by: Anna Endo, Ibrar Ahmed, Fujii Masao
Discussion: https://postgr.es/m/alpine.DEB.2.21.1904061826420.3678@lancre
There's plenty places in frontend code that could benefit from a
string buffer implementation. Some because it yields simpler and
faster code, and some others because of the desire to share code
between backend and frontend.
While there is a string buffer implementation available to frontend
code, libpq's PQExpBuffer, it is clunkier than stringinfo, it
introduces a libpq dependency, doesn't allow for sharing between
frontend and backend code, and has a higher API/ABI stability
requirement due to being exposed via libpq.
Therefore it seems best to just making StringInfo being usable by
frontend code. There's not much to do for that, except for rewriting
two subsequent elog/ereport calls into others types of error
reporting, and deciding on a maximum string length.
For the maximum string size I decided to privately define MaxAllocSize
to the same value as used in the backend. It seems likely that we'll
want to reconsider this for both backend and frontend code in the not
too far away future.
For now I've left stringinfo.h in lib/, rather than common/, to reduce
the likelihood of unnecessary breakage. We could alternatively decide
to provide a redirecting stringinfo.h in lib/, or just not provide
compatibility.
Author: Andres Freund
Reviewed-By: Kyotaro Horiguchi, Daniel Gustafsson
Discussion: https://postgr.es/m/20190920051857.2fhnvhvx4qdddviz@alap3.anarazel.de
When maintaining or merging patches, one of the most common sources
for conflicts are the list of objects in makefiles. Especially when
the split across lines has been changed on both sides, which is
somewhat common due to attempting to stay below 80 columns, those
conflicts are unnecessarily laborious to resolve.
By splitting, and alphabetically sorting, OBJS style lines into one
object per line, conflicts should be less frequent, and easier to
resolve when they still occur.
Author: Andres Freund
Discussion: https://postgr.es/m/20191029200901.vww4idgcxv74cwes@alap3.anarazel.de
For a long time (since commit aed378e8d) we have had a policy to log
nothing about a connection if the client disconnects when challenged
for a password. This is because libpq-using clients will typically
do that, and then come back for a new connection attempt once they've
collected a password from their user, so that logging the abandoned
connection attempt will just result in log spam. However, this did
not work well for PAM authentication: the bottom-level function
pam_passwd_conv_proc() was on board with it, but we logged messages
at higher levels anyway, for lack of any reporting mechanism.
Add a flag and tweak the logic so that the case is silent, as it is
for other password-using auth mechanisms.
Per complaint from Yoann La Cancellera. It's been like this for awhile,
so back-patch to all supported branches.
Discussion: https://postgr.es/m/CACP=ajbrFFYUrLyJBLV8=q+eNCapa1xDEyvXhMoYrNphs-xqPw@mail.gmail.com
get_relkind_objtype, and hence get_object_type, failed when applied to a
toast table. This is not a good thing, because it prevents reporting of
perfectly legitimate permissions errors. (At present, these functions
are in fact *only* used to determine the ObjectType argument for
acl_error() calls.) It seems best to have them fall back to returning
OBJECT_TABLE in every case where they can't determine an object type
for a pg_class entry, so do that.
In passing, make some edits to alter.c to make it more obvious that
those calls of get_object_type() are used only for error reporting.
This might save a few cycles in the non-error code path, too.
Back-patch to v11 where this issue originated.
John Hsu, Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/C652D3DF-2B0C-4128-9420-FB5379F6B1E4@amazon.com
Commit d25ea0127 got rid of what I thought were entirely unnecessary
derived child expressions in EquivalenceClasses for EC members that
mention multiple baserels. But it turns out that some of the child
expressions that code created are necessary for partitionwise joins,
else we fail to find matching pathkeys for Sort nodes. (This happens
only for certain shapes of the resulting plan; it may be that
partitionwise aggregation is also necessary to show the failure,
though I'm not sure of that.)
Reverting that commit entirely would be quite painful performance-wise
for large partition sets. So instead, add code that explicitly
generates child expressions that match only partitionwise child join
rels we have actually generated.
Per report from Justin Pryzby. (Amit Langote noticed the problem
earlier, though it's not clear if he recognized then that it could
result in a planner error, not merely failure to exploit partitionwise
join, in the code as-committed.) Back-patch to v12 where commit
d25ea0127 came in.
Amit Langote, with lots of kibitzing from me
Discussion: https://postgr.es/m/CA+HiwqG2WVUGmLJqtR0tPFhniO=H=9qQ+Z3L_ZC+Y3-EVQHFGg@mail.gmail.com
Discussion: https://postgr.es/m/20191011143703.GN10470@telsasoft.com
Since 898e5e32, this command uses partially ShareUpdateExclusiveLock,
but the docs did not get the call.
Author: Justin Pryzby
Reviewed-by: Amit Langote, Álvaro Herrera, Michael Paquier
Discussion: https://postgr.es/m/20191028001207.GB23808@telsasoft.com
Backpatch-through: 12
This clarifies more how to use and how to take advantage of constraints
when attaching a new partition.
Author: Justin Pryzby
Reviewed-by: Amit Langote, Álvaro Herrera, Michael Paquier
Discussion: https://postgr.es/m/20191028001207.GB23808@telsasoft.com
Backpatch-through: 10
Historically, the code to build relation options has been shaped the
same way in multiple code paths by using a set of datums in input with
the options parsed with a static table which is then filled with the
option values. This introduces a new common routine in reloptions.c to
do most of the legwork for the in-core code paths.
Author: Amit Langote
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CA+HiwqGsoSn_uTPPYT19WrtR7oYpYtv4CdS0xuedTKiHHWuk_g@mail.gmail.com
The code only compared two triggers' names and namespaces (the latter
being the owning table's schema). This could result in falling back
to an OID-based sort of similarly-named triggers on different tables.
We prefer to avoid that, so add a comparison of the table names too.
(The sort order is thus table namespace, trigger name, table name,
which is a bit odd, but it doesn't seem worth contorting the code
to work around that.)
Likewise for policy objects, in 9.5 and up.
Complaint and fix by Benjie Gillam. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/CAMThMzEEt2mvBbPgCaZ1Ap1N-moGn=Edxmadddjq89WG4NpPtQ@mail.gmail.com
As the code stands, nEntries counts the number of ginEntryInsert()
calls, so that's what you end up with at the end of a GIN index build.
However, ginvacuumcleanup() recomputes nEntries as the number of
surviving leaf tuples, and that's generally consistent with the way that
gincostestimate() uses the value. So let's clearly define nEntries
as the number of leaf tuples, and therefore adjust ginEntryInsert() to
increment it only when we make a new one, not when we add TIDs into an
existing tuple or posting tree.
In practice this inconsistency probably has little impact, so I don't
feel a need to back-patch.
Insung Moon and Keisuke Kuroda
Discussion: https://postgr.es/m/CAEMmqBuH_O-oXL+3_ArQ6F5cJ7kXVow2SGQB3HRacku_T+xkmA@mail.gmail.com
Rearrange the logic in record_image_cmp() and datum_image_eq() to
error out on unexpected typlens (either not supported there or
completely invalid due to corruption). Barring corruption, this is
not possible today but it seems more future-proof and robust to fix
this.
Reported-by: Peter Geoghegan <pg@bowt.ie>
Commit 8af1624e3 introduced a warning about possibly returning
without a value, on compilers that don't realize that ereport(ERROR)
doesn't return. Tweak the code to avoid that.
Per buildfarm. Back-patch to 9.6, like the aforesaid commit.
DatumGetPointer() takes a Datum argument, not a pointer.
This is cosmetic given the current definitions of the
underlying macros, but it's still formally a type violation.
Bug was introduced in commit 389bb2818, but there seems
no need to back-patch.
Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/d8jlfsxq3a0.fsf@dalvik.ping.uio.no
Using incorrect, or just mismatched, dictionary and affix files
could result in a crash, due to failure to cross-check offsets
obtained from the file. Add necessary validation, as well as
some Asserts for future-proofing.
Per bug #16050 from Alexander Lakhin. Back-patch to 9.6 where the
problem was introduced.
Arthur Zakirov, per initial investigation by Tomas Vondra
Discussion: https://postgr.es/m/16050-024ae722464ab604@postgresql.org
Discussion: https://postgr.es/m/20191013012610.2p2fp3zzpoav7jzf@development
When using CREATE TABLE for a new partition, the partitioned indexes of
the parent are created automatically in a fashion similar to LIKE
INDEXES. The new partition and its parent use a mapping for attribute
numbers for this operation, and while the mapping was correctly built,
its length was defined as the number of attributes of the newly-created
child, and not the parent. If the parent includes dropped columns, this
could cause failures.
This is wrong since 8b08f7d which has introduced the concept of
partitioned indexes, so backpatch down to 11.
Reported-by: Wyatt Alt
Author: Michael Paquier
Reviewed-by: Amit Langote
Discussion: https://postgr.es/m/CAGem3qCcRmhbs4jYMkenYNfP2kEusDXvTfw-q+eOhM0zTceG-g@mail.gmail.com
Backpatch-through: 11
A couple of routines assume that the LWLock SyncRepLock needs to be
taken, so add a couple of assertions to be sure of that. Also, when
waiting for a given LSN at transaction commit, the code implied that the
syncrep queue cleanup happens while holding interrupts, but the code
never checked after that.
Author: Michael Paquier
Reviewed-by: Fujii Masao, Kyotaro Horiguchi, Dongming Liu
Discussion: https://postgr.es/m/a0806273-8bbb-43b3-bbe1-c45a58f6ae21.lingce.ldm@alibaba-inc.com
When a backend exits, it gets deleted from the syncrep queue if present.
The queue was checked without SyncRepLock taken in exclusive mode, so it
would have been possible for a backend to remove itself after a WAL
sender already did the job. Fix this issue based on a suggestion from
Fujii Masao, by first checking the queue without the lock. Then, if the
backend is present in the queue, take the lock and perform an additional
lookup check before doing the element deletion.
Author: Dongming Liu
Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/a0806273-8bbb-43b3-bbe1-c45a58f6ae21.lingce.ldm@alibaba-inc.com
Backpatch-through: 9.4
In these macros, the rd_options pointer is cast to ViewOption *. Add
some assertions that the passed-in relation is actually a view before
doing that.
Author: Nikolay Shaplov <dhyan@nataraj.su>
Discussion: https://www.postgresql.org/message-id/flat/3634983.eHpMQ1mJnI@x200m
This gives an alternative way of catching exceptions, for the common
case where the cleanup code is the same in the error and non-error
cases. So instead of
PG_TRY();
{
... code that might throw ereport(ERROR) ...
}
PG_CATCH();
{
cleanup();
PG_RE_THROW();
}
PG_END_TRY();
cleanup();
one can write
PG_TRY();
{
... code that might throw ereport(ERROR) ...
}
PG_FINALLY();
{
cleanup();
}
PG_END_TRY();
Discussion: https://www.postgresql.org/message-id/flat/95a822c3-728b-af0e-d7e5-71890507ae0c%402ndquadrant.com
IDENT_USERNAME_MAX is the maximum length of the information returned
by an ident server, per RFC 1413. Using it as the buffer size in peer
authentication is inappropriate. It was done here because of the
historical relationship between peer and ident authentication. To
reduce confusion between the two authenticaton methods and disentangle
their code, use a dynamically allocated buffer instead.
Discussion: https://www.postgresql.org/message-id/flat/c798fba5-8b71-4f27-c78e-37714037ea31%402ndquadrant.com
For historical reasons, the functions for peer authentication were
grouped under ident authentication. But they are really completely
separate, so give them their own section headings.
The additional newline seems to have accidentally been introduced in
2c03216d83, in 9.5. The newline is only issued when an FPW is
present for the block reference.
While there could be an argument that removing the newlines in the
back branches could cause a problem for somebody parsing the
pg_waldump output, the likelihood of that seems small enough. It seems
at least equally likely that the randomness of when newlines are
issued causes problems.
Author: Andres Freund
Discussion: https://postgr.es/m/20191029233341.4gnyau7e5v2lh5sc@alap3.anarazel.de
Backpatch: 9.5, like 2c03216d83.
Under MinGW, when compiling the ecpg test files, you get compiler
warnings about the use of %lld in printf().
These files don't use our printf replacement or the c.h porting layer,
so determine the appropriate format conversion the hard way.
Reviewed-by: Michael Meskes <meskes@postgresql.org>
Discussion: https://www.postgresql.org/message-id/flat/760c9dd1-2d80-c223-3f90-609b615f7918%402ndquadrant.com
When cancelling REINDEX CONCURRENTLY after swapping the old and new
indexes (for example interruption at step 5), the old index remains
around and is marked as invalid. The old index should also be manually
droppable to clean up the parent relation from any invalid indexes still
remaining. For a partition index reindexed, pg_class.relispartition was
not getting updated, causing the index to not be droppable as DROP INDEX
would look for dependencies in a partition tree, which do not exist
anymore after the swap phase is done.
The fix here is simple: when swapping the old and new indexes, make sure
that pg_class.relispartition is correctly switched, similarly to what is
done for the index name.
Reported-by: Justin Pryzby
Author: Michael Paquier
Discussion: https://postgr.es/m/20191015164047.GA22729@telsasoft.com
Backpatch-through: 12
Teach get_expr_result_type() to manufacture a tuple descriptor directly
from a RowExpr node. If the RowExpr has type RECORD, this is the only
way to get a tupdesc for its result, since even if the rowtype has been
blessed, we don't have its typmod available at this point. (If the
RowExpr has some named composite type, we continue to let the existing
code handle it, since the RowExpr might well not have the correct column
names embedded in it.)
This fixes assorted corner cases illustrated by the added regression
tests.
Discussion: https://postgr.es/m/10872.1572202006@sss.pgh.pa.us
Historically, psql consulted COMSPEC to spawn a shell in its \! command,
but we just invoked "cmd" when spawning shells in pg_ctl and pg_regress.
It seems better to rely on the environment variable, if it's set,
in all cases.
It's debatable whether this is a bug fix or just a behavioral change,
so no back-patch.
Juan José Santamaría Flecha
Discussion: https://postgr.es/m/16080-5d7f03222469f717@postgresql.org
Commit 9556aa01c rearranged the innards of text_position() in a way
that would make it not work for empty search strings. Which is fine,
because all callers of that code special-case an empty pattern in
some way. However, the primary use-case (text_position itself) got
special-cased incorrectly: historically it's returned 1 not 0 for
an empty search string. Restore the historical behavior.
Per complaint from Austin Drenski (via Shay Rojansky).
Back-patch to v12 where it got broken.
Discussion: https://postgr.es/m/CADT4RqAz7oN4vkPir86Kg1_mQBmBxCp-L_=9vRpgSNPJf0KRkw@mail.gmail.com
When swapping the dependencies of the old and new indexes, the code has
been correctly switching all links in pg_depend from the old to the new
index for both referencing and referenced entries. However it forgot
the fact that the new index may itself have existing entries in
pg_depend, like references to the parent table attributes. This
resulted in duplicated entries in pg_depend after running REINDEX
CONCURRENTLY.
Fix this problem by removing any existing entries in pg_depend on the
new index before switching the dependencies of the old index to the new
one. More regression tests are added to check the consistency of
entries in pg_depend for indexes, including partition indexes.
Author: Michael Paquier
Discussion: https://postgr.es/m/20191025064318.GF8671@paquier.xyz
Backpatch-through: 12
9155580 has changed the value of the first fake LSN for unlogged
relations from 1 to FirstNormalUnloggedLSN (aka 1000), GiST requiring a
non-zero LSN on some pages to allow an interlocking logic to work, but
its value was still initialized to 1 at the beginning of recovery or
after running pg_resetwal. This fixes the initialization for both code
paths.
Author: Takayuki Tsunakawa
Reviewed-by: Dilip Kumar, Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/OSBPR01MB2503CE851940C17DE44AE3D9FE6F0@OSBPR01MB2503.jpnprd01.prod.outlook.com
Backpatch-through: 12
When we added the GUC units feature, we didn't make any great effort
to adjust the documentation of individual GUCs; they tended to still
say things like "this is the number of milliseconds that ...", even
though users might prefer to write some other units, and SHOW might
even show the value in other units. Commit 6c9fb69f2 made an effort
to improve this situation, but I thought it made things less readable
by injecting units information in mid-sentence. It also wasn't very
consistent, and did not touch all the GUCs that have units.
To improve matters, standardize on the phrasing "If this value is
specified without units, it is taken as <units>". Also, try to
standardize where this is mentioned, right before the specification
of the default. (In a couple of places, doing that would've required
more rewriting than seemed justified, so I wasn't 100% consistent
about that.) I also tried to use the phrases "amount of time",
"amount of memory", etc rather than describing the contents of GUCs
in other ways, as those were the majority usage in places that weren't
overcommitting to a particular unit. (I left "length of time" alone
in a couple of places, though.)
I failed to resist the temptation to copy-edit some awkward text, too.
Backpatch to v12, like 6c9fb69f2, mainly because v12 hasn't diverged
much from HEAD yet.
Discussion: https://postgr.es/m/15882.1571942223@sss.pgh.pa.us
Remove SQL_LANGUAGES, which was eliminated in SQL:2008, and
SQL_PACKAGES and SQL_SIZING_PROFILES, which were eliminated in
SQL:2011. Since they were dropped by the SQL standard, the
information in them was no longer updated and therefore no longer
useful.
This also removes the feature-package association information in
sql_feature_packages.txt, but for the time begin we are keeping the
information which features are in the Core package (that is, mandatory
SQL features). Maybe at some point someone wants to invent a way to
store that that does not involve using the "package" mechanism
anymore.
Discussion https://www.postgresql.org/message-id/flat/91334220-7900-071b-9327-0c6ecd012017%402ndquadrant.com
It appears that libxml2 doesn't bother to set the "children" field of
an XML_NAMESPACE_DECL node to null; that field just contains garbage.
In v10 and v11, this can result in a crash in XMLTABLE(). The rewrite
done in commit 251cf2e27 fixed this, somewhat accidentally, in v12.
We're not going to back-patch 251cf2e27, however. The case apparently
doesn't have wide use, so rather than risk introducing other problems,
just add a safety check to throw an error.
Even though no bug manifests in v12/HEAD, add the relevant test case
there too, to prevent future regressions.
Chapman Flack (per private report)
pgtypeslib_extern.h contained fallback definitions of "bool", "FALSE",
and "TRUE". The latter two are just plain unused, and have been for
awhile. The former came into play only if there wasn't a macro
definition of "bool", which is true only if we aren't using <stdbool.h>.
However, it then defined bool as "char"; since commit d26a810eb that
conflicts with c.h's desire to use "unsigned char". We'd missed seeing
any bad effects of that due to accidental header inclusion order choices,
but dddf4cdc3 exposed that it was problematic.
To fix, let's just get rid of these definitions. They should not be
needed because everyplace in Postgres should be relying on c.h to
provide a definition for type bool. (Note that despite its name,
pgtypeslib_extern.h isn't exposed to any outside code; we don't
install it.)
This doesn't fully resolve the issue, because ecpglib.h is doing
similar things, but that seems to require more thought to fix.
Back-patch to v12 where d26a810eb came in, to forestall any unpleasant
surprises from future back-patched bug fixes.
Discussion: https://postgr.es/m/CAA4eK1LmaKO7Du9M9Lo=kxGU8sB6aL8fa3sF6z6d5yYYVe3BuQ@mail.gmail.com
Peter Eisentraut