Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 04 Dec, 2019 4 commits
  3. 03 Dec, 2019 7 commits
    • Tomas Vondra's avatar
      Ensure maxlen is at leat 1 in dict_int · b5273943
      Tomas Vondra authored 
      The dict_int text search dictionary template accepts maxlen parameter,
which is then used to cap the length of input strings. The value was
not properly checked, and the code simply does

    txt[d->maxlen] = '\0';

to insert a terminator, leading to segfaults with negative values.

This commit simply rejects values less than 1. The issue was there since
dct_int was introduced in 9.3, so backpatch all the way back to 9.4
which is the oldest supported version.

Reported-by: cili
Discussion: https://postgr.es/m/16144-a36a5bef7657047d@postgresql.org
Backpatch-through: 9.4
      b5273943
    • Tom Lane's avatar
      Further sync postgres_fdw's "Relations" output with the rest of EXPLAIN. · bf39b3af
      Tom Lane authored 
      EXPLAIN generally only adds schema qualifications to table names when
VERBOSE is specified.  In postgres_fdw's "Relations" output, table
names were always so qualified, but that was an implementation
restriction: in the original coding, we didn't have access to the
verbose flag at the time the string was generated.  After the code
rearrangement of commit 4526951d, we do have that info available
at the right time, so make this output follow the normal rule.

Discussion: https://postgr.es/m/12424.1575168015@sss.pgh.pa.us
      bf39b3af
    • Michael Paquier's avatar
      Fix thinkos from commit 9989d37d · 68ab9829
      Michael Paquier authored 
      Error messages referring to incorrect WAL segment names could have been
generated for a fsync() failure or when creating a new segment at the
end of recovery.
      68ab9829
    • Peter Eisentraut's avatar
      Fix alter_system_table test · 88d45ac7
      Peter Eisentraut authored 
      Add workaround for disabling ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
warning for the test that tries to create a tablespace with a reserved
name.

Discussion: https://www.postgresql.org/message-id/flat/E1iacW7-0003h6-6U%40gemulon.postgresql.org
      88d45ac7
    • Michael Paquier's avatar
      Remove XLogFileNameP() from the tree · 9989d37d
      Michael Paquier authored 
      XLogFileNameP() is a wrapper routine able to build a palloc'd string for
a WAL segment name, which is used for error string generation.  There
were several code paths where it gets called in a critical section,
where memory allocation is not allowed.  This results in triggering
an assertion failure instead of generating the wanted error message.

Another, more annoying, problem is that if the allocation to generate
the WAL segment name fails on OOM, then the failure would be escalated
to a PANIC.

This removes the routine and all its callers are replaced with a logic
using a fixed-size buffer.  This way, all the existing mistakes are
fixed and future ones are prevented.

Author: Masahiko Sawada
Reviewed-by: Michael Paquier, Álvaro Herrera
Discussion: https://postgr.es/m/CA+fd4k5gC9H4uoWMLg9K_QfNrnkkdEw+-AFveob9YX7z8JnKTA@mail.gmail.com
      9989d37d
    • Michael Paquier's avatar
      Fix failures with TAP tests of pg_ctl on Windows · e5532f19
      Michael Paquier authored 
      On Windows, all the hosts spawned by the TAP tests bind to 127.0.0.1.
Hence, if there is a port conflict, starting a cluster would immediately
fail.  One of the test scripts of pg_ctl initializes a node without
PostgresNode.pm, using the default port 5432.  This could cause
unexpected startup failures in the tests if an independent server was up
and running on the same host (the reverse is also possible, though more
unlikely).  Fix this issue by assigning properly a free port to the node
configured, in the same range used as for the other nodes part of the
tests.

Author: Michael Paquier
Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/20191202031444.GC1696@paquier.xyz
Backpatch-through: 11
      e5532f19
    • Tom Lane's avatar
      Fix EXPLAIN's column alias output for mismatched child tables. · 55a1954d
      Tom Lane authored 
      If an inheritance/partitioning parent table is assigned some column
alias names in the query, EXPLAIN mapped those aliases onto the
child tables' columns by physical position, resulting in bogus output
if a child table's columns aren't one-for-one with the parent's.

To fix, make expand_single_inheritance_child() generate a correctly
re-mapped column alias list, rather than just copying the parent
RTE's alias node.  (We have to fill the alias field, not just
adjust the eref field, because ruleutils.c will ignore eref in
favor of looking at the real column names.)

This means that child tables will now always have alias fields in
plan rtables, where before they might not have.  That results in
a rather substantial set of regression test output changes:
EXPLAIN will now always show child tables with aliases that match
the parent table (usually with "_N" appended for uniqueness).
But that seems like a net positive for understandability, since
the parent alias corresponds to something that actually appeared
in the original query, while the child table names didn't.
(Note that this does not change anything for cases where an explicit
table alias was written in the query for the parent table; it
just makes cases without such aliases behave similarly to that.)
Hence, while we could avoid these subsidiary changes if we made
inherit.c more complicated, we choose not to.

Discussion: https://postgr.es/m/12424.1575168015@sss.pgh.pa.us
      55a1954d
  5. 02 Dec, 2019 4 commits
    • Tom Lane's avatar
      Add a reverse-translation column number array to struct AppendRelInfo. · ce76c0ba
      Tom Lane authored 
      This provides for cheaper mapping of child columns back to parent
columns.  The one existing use-case in examine_simple_variable()
would hardly justify this by itself; but an upcoming bug fix will
make use of this array in a mainstream code path, and it seems
likely that we'll find other uses for it as we continue to build
out the partitioning infrastructure.

Discussion: https://postgr.es/m/12424.1575168015@sss.pgh.pa.us
      ce76c0ba
    • Tom Lane's avatar
      Make postgres_fdw's "Relations" output agree with the rest of EXPLAIN. · 4526951d
      Tom Lane authored 
      The relation aliases shown in the "Relations" line for a foreign scan
didn't always agree with those used in the rest of EXPLAIN's output.
The regression test result changes appearing here provide examples.

It's really impossible for postgres_fdw to duplicate EXPLAIN's alias
assignment logic during postgresGetForeignRelSize(), because of the
de-duplication that EXPLAIN does on a global basis --- and anyway,
trying to duplicate that would be unmaintainable.  Instead, just put
numeric rangetable indexes into the string, and convert those to
table names/aliases in postgresExplainForeignScan, which does have
access to the results of ruleutils.c's alias assignment logic.
Aside from being more reliable, this shifts some work from planning
to EXPLAIN, which is a good tradeoff for performance.  (I also
changed from using StringInfo to using psprintf, which makes the
code slightly simpler and reduces its memory consumption.)

A kluge required by this solution is that we have to reverse-engineer
the rtoffset applied by setrefs.c.  If that logic ever fails
(presumably because the member tables of a join got offset by
different amounts), we'll need some more cooperation with setrefs.c
to keep things straight.  But for now, there's no need for that.

Arguably this is a back-patchable bug fix, but since this is a mostly
cosmetic issue and there have been no field complaints, I'll refrain
for now.

Discussion: https://postgr.es/m/12424.1575168015@sss.pgh.pa.us
      4526951d
    • Michael Paquier's avatar
      Add query cancellation capabilities in pgbench init phase · 1d468b9a
      Michael Paquier authored 
      This can be useful to stop data generation happening on the server for
long-running queries caused by large scale factors.  This cannot happen
by default as data is generated on the client, but it is possible to
control the initialization steps of pgbench to do that.

Reported-by: Fujii Masao
Author: Fabien Coelho
Discussion: https://postgr.es/m/alpine.DEB.2.21.1910311939430.27369@lancre
Discussion: https://postgr.es/m/CAHGQGwHWEyTXxZh46qgFY8a2bDF_EYeUdp3+_Hy=qLZSzwVPKg@mail.gmail.com
      1d468b9a
    • Michael Paquier's avatar
      Refactor query cancellation code into src/fe_utils/ · a4fd3aa7
      Michael Paquier authored 
      Originally, this code was duplicated in src/bin/psql/ and
src/bin/scripts/, but it can be useful for other frontend applications,
like pgbench.  This refactoring offers the possibility to setup a custom
callback which would get called in the signal handler for SIGINT or when
the interruption console events happen on Windows.

Author: Fabien Coelho, with contributions from Michael Paquier
Reviewed-by: Álvaro Herrera, Ibrar Ahmed
Discussion: https://postgr.es/m/alpine.DEB.2.21.1910311939430.27369@lancre
      a4fd3aa7
  7. 01 Dec, 2019 2 commits
    • Andrew Dunstan's avatar
      Add dummy versions of new SSL functions for non-SSL builds · c01ac6dc
      Andrew Dunstan authored 
      This rectifies an oversight in commit 4dc63552, which caused certain
builds to fail, especially on Windows.
      c01ac6dc
    • Tom Lane's avatar
      Fix misbehavior with expression indexes on ON COMMIT DELETE ROWS tables. · c35b714c
      Tom Lane authored 
      We implement ON COMMIT DELETE ROWS by truncating tables marked that
way, which requires also truncating/rebuilding their indexes.  But
RelationTruncateIndexes asks the relcache for up-to-date copies of any
index expressions, which may cause execution of eval_const_expressions
on them, which can result in actual execution of subexpressions.
This is a bad thing to have happening during ON COMMIT.  Manuel Rigger
reported that use of a SQL function resulted in crashes due to
expectations that ActiveSnapshot would be set, which it isn't.
The most obvious fix perhaps would be to push a snapshot during
PreCommit_on_commit_actions, but I think that would just open the door
to more problems: CommitTransaction explicitly expects that no
user-defined code can be running at this point.

Fortunately, since we know that no tuples exist to be indexed, there
seems no need to use the real index expressions or predicates during
RelationTruncateIndexes.  We can set up dummy index expressions
instead (we do need something that will expose the right data type,
as there are places that build index tupdescs based on this), and
just ignore predicates and exclusion constraints.

In a green field it'd likely be better to reimplement ON COMMIT DELETE
ROWS using the same "init fork" infrastructure used for unlogged
relations.  That seems impractical without catalog changes though,
and even without that it'd be too big a change to back-patch.
So for now do it like this.

Per private report from Manuel Rigger.  This has been broken forever,
so back-patch to all supported branches.
      c35b714c
  9. 30 Nov, 2019 3 commits
    • Andrew Dunstan's avatar
      libq support for sslpassword connection param, DER format keys · 4dc63552
      Andrew Dunstan authored 
      This patch providies for support for password protected SSL client
keys in libpq, and for DER format keys, both encrypted and unencrypted.
There is a new connection parameter sslpassword, which is supplied to
the OpenSSL libraries via a callback function. The callback function can
also be set by an application by calling PQgetSSLKeyPassHook(). There is
also a function to retreive the connection setting, PQsslpassword().

Craig Ringer and Andrew Dunstan

Reviewed by: Greg Nancarrow

Discussion: https://postgr.es/m/f7ee88ed-95c4-95c1-d4bf-7b415363ab62@2ndQuadrant.com
      4dc63552
    • Tomas Vondra's avatar
      Fix off-by-one error in PGTYPEStimestamp_fmt_asc · 3ff660bb
      Tomas Vondra authored 
      When using %b or %B patterns to format a date, the code was simply using
tm_mon as an index into array of month names. But that is wrong, because
tm_mon is 1-based, while array indexes are 0-based. The result is we
either use name of the next month, or a segfault (for December).

Fix by subtracting 1 from tm_mon for both patterns, and add a regression
test triggering the issue. Backpatch to all supported versions (the bug
is there far longer, since at least 2003).

Reported-by: Paul Spencer
Backpatch-through: 9.4
Discussion: https://postgr.es/m/16143-0d861eb8688d3fef%40postgresql.org
      3ff660bb
    • Amit Kapila's avatar
      Revert commits 290acac9 and 8a7e9e9d. · 98a9b37b
      Amit Kapila authored 
      This commit revert the commits to add a test case that tests the 'force'
option when there is an active backend connected to the database being
dropped.

This feature internally sends SIGTERM to all the backends connected to the
database being dropped and then the same is reported to the client.  We
found that on Windows, the client end of the socket is not able to read
the data once we close the socket in the server which leads to loss of
error message which is not what we expect.  We also observed  similar
behavior in other cases like pg_terminate_backend(),
pg_ctl kill TERM <pid>.  There are probably a few others like that.  The
fix for this requires further study.

Discussion: https://postgr.es/m/E1iaD8h-0004us-K9@gemulon.postgresql.org
      98a9b37b
  11. 29 Nov, 2019 5 commits
  13. 28 Nov, 2019 8 commits
  15. 27 Nov, 2019 4 commits
  17. 26 Nov, 2019 3 commits
    • Andrew Dunstan's avatar
      Fix closing stdin in TestLib.pm · 792dba73
      Andrew Dunstan authored 
      Commit 9af34f3c naively assumed that all non-windows platforms would
have pseudoterminals and that perl would have IO::Pty. Such is not the
case. Instead of assumung anything, test for the presence of IO::Pty and
only use code that might depend on it if it's present. The test result is
exposed in $TestLib::have_io_pty so that it can be conveniently used in
SKIP tests.

Discussion: https://postgr.es/m/20191126044110.GB5435@paquier.xyz
      792dba73
    • Tom Lane's avatar
      Allow access to child table statistics if user can read parent table. · 553d2ec2
      Tom Lane authored 
      The fix for CVE-2017-7484 disallowed use of pg_statistic data for
planning purposes if the user would not be able to select the associated
column and a non-leakproof function is to be applied to the statistics
values.  That turns out to disable use of pg_statistic data in some
common cases involving inheritance/partitioning, where the user does
have permission to select from the parent table that was actually named
in the query, but not from a child table whose stats are needed.  Since,
in non-corner cases, the user *can* select the child table's data via
the parent, this restriction is not actually useful from a security
standpoint.  Improve the logic so that we also check the permissions of
the originally-named table, and allow access if select permission exists
for that.

When checking access to stats for a simple child column, we can map
the child column number back to the parent, and perform this test
exactly (including not allowing access if the child column isn't
exposed by the parent).  For expression indexes, the current logic
just insists on whole-table select access, and this patch allows
access if the user can select the whole parent table.  In principle,
if the child table has extra columns, this might allow access to
stats on columns the user can't read.  In practice, it's unlikely
that the planner is going to do any stats calculations involving
expressions that are not visible to the query, so we'll ignore that
fine point for now.  Perhaps someday we'll improve that logic to
detect exactly which columns are used by an expression index ...
but today is not that day.

Back-patch to v11.  The issue was created in 9.2 and up by the
CVE-2017-7484 fix, but this patch depends on the append_rel_array[]
planner data structure which only exists in v11 and up.  In
practice the issue is most urgent with partitioned tables, so
fixing v11 and later should satisfy much of the practical need.

Dilip Kumar and Amit Langote, with some kibitzing by me

Discussion: https://postgr.es/m/3876.1531261875@sss.pgh.pa.us
      553d2ec2
    • Michael Paquier's avatar
      Add safeguards for pg_fsync() called with incorrectly-opened fds · 12198239
      Michael Paquier authored 
      On some platforms, fsync() returns EBADFD when opening a file descriptor
with O_RDONLY (read-only), leading ultimately now to a PANIC to prevent
data corruption.

This commit adds a new sanity check in pg_fsync() based on fcntl() to
make sure that we don't repeat again mistakes with incorrectly-set file
descriptors so as problems are detected at an early stage.  Without
that, such errors could only be detected after running Postgres on a
specific supported platform for the culprit code path, which could take
some time before being found.  b8e19b93 was a fix for such a problem,
which got undetected for more than 5 years, and a586cc4b fixed another
similar issue.

Note that the new check added works as well when fsync=off is
configured, so as all regression tests would detect problems as long as
assertions are enabled.  fcntl() being not available on Windows, the
new checks do not happen there.

Author: Michael Paquier
Reviewed-by: Mark Dilger
Discussion: https://postgr.es/m/20191009062640.GB21379@paquier.xyz
      12198239