Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 01 Sep, 2020 2 commits
    • Tom Lane's avatar
      Teach libpq to handle arbitrary-length lines in .pgpass files. · b55b4dad
      Tom Lane authored 
      Historically there's been a hard-wired assumption here that no line of
a .pgpass file could be as long as NAMEDATALEN*5 bytes.  That's a bit
shaky to start off with, because (a) there's no reason to suppose that
host names fit in NAMEDATALEN, and (b) this figure fails to allow for
backslash escape characters.  However, it fails completely if someone
wants to use a very long password, and we're now hearing reports of
people wanting to use "security tokens" that can run up to several
hundred bytes.  Another angle is that the file is specified to allow
comment lines, but there's no reason to assume that long comment lines
aren't possible.

Rather than guessing at what might be a more suitable limit, let's
replace the fixed-size buffer with an expansible PQExpBuffer.  That
adds one malloc/free cycle to the typical use-case, but that's surely
pretty cheap relative to the I/O this code has to do.

Also, add TAP test cases to exercise this code, because there was no
test coverage before.

This reverts most of commit 2eb3bc58, as there's no longer a need for
a warning message about overlength .pgpass lines.  (I kept the explicit
check for comment lines, though.)

In HEAD and v13, this also fixes an oversight in 74a308cf: there's not
much point in explicit_bzero'ing the line buffer if we only do so in two
of the three exit paths.

Back-patch to all supported branches, except that the test case only
goes back to v10 where src/test/authentication/ was added.

Discussion: https://postgr.es/m/4187382.1598909041@sss.pgh.pa.us
      b55b4dad
    • Amit Kapila's avatar
      Fix the SharedFileSetUnregister API. · 4ab77697
      Amit Kapila authored 
      Commit 808e13b2 introduced a few APIs to extend the existing Buffile
interface. In SharedFileSetDeleteOnProcExit, it tries to delete the list
element while traversing the list with 'foreach' construct which makes the
behavior of list traversal unpredictable.

Author: Amit Kapila
Reviewed-by: Dilip Kumar
Tested-by: Dilip Kumar and Neha Sharma
Discussion: https://postgr.es/m/CAA4eK1JhLatVcQ2OvwA_3s0ih6Hx9+kZbq107cXVsSWWukH7vA@mail.gmail.com
      4ab77697
  3. 31 Aug, 2020 13 commits
  5. 30 Aug, 2020 3 commits
    • Tom Lane's avatar
      Mark factorial operator, and postfix operators in general, as deprecated. · 6ca547cf
      Tom Lane authored 
      Per discussion, we're planning to remove parser support for postfix
operators in order to simplify the grammar.  So it behooves us to
put out a deprecation notice at least one release before that.

There is only one built-in postfix operator, ! for factorial.
Label it deprecated in the docs and in pg_description, and adjust
some examples that formerly relied on it.  (The sister prefix
operator !! is also deprecated.  We don't really have to remove
that one, but since we're suggesting that people use factorial()
instead, it seems better to remove both operators.)

Also state in the CREATE OPERATOR ref page that postfix operators
in general are going away.

Although this changes the initial contents of pg_description,
I did not force a catversion bump; it doesn't seem essential.

In v13, also back-patch 4c5cf543, so that there's someplace for
the <link>s to point to.

Mark Dilger and John Naylor, with some adjustments by me

Discussion: https://postgr.es/m/BE2DF53D-251A-4E26-972F-930E523580E9@enterprisedb.com
      6ca547cf
    • Tom Lane's avatar
      Redefine pg_class.reltuples to be -1 before the first VACUUM or ANALYZE. · 3d351d91
      Tom Lane authored 
      Historically, we've considered the state with relpages and reltuples
both zero as indicating that we do not know the table's tuple density.
This is problematic because it's impossible to distinguish "never yet
vacuumed" from "vacuumed and seen to be empty".  In particular, a user
cannot use VACUUM or ANALYZE to override the planner's normal heuristic
that an empty table should not be believed to be empty because it is
probably about to get populated.  That heuristic is a good safety
measure, so I don't care to abandon it, but there should be a way to
override it if the table is indeed intended to stay empty.

Hence, represent the initial state of ignorance by setting reltuples
to -1 (relpages is still set to zero), and apply the minimum-ten-pages
heuristic only when reltuples is still -1.  If the table is empty,
VACUUM or ANALYZE (but not CREATE INDEX) will override that to
reltuples = relpages = 0, and then we'll plan on that basis.

This requires a bunch of fiddly little changes, but we can get rid of
some ugly kluges that were formerly needed to maintain the old definition.

One notable point is that FDWs' GetForeignRelSize methods will see
baserel->tuples = -1 when no ANALYZE has been done on the foreign table.
That seems like a net improvement, since those methods were formerly
also in the dark about what baserel->tuples = 0 really meant.  Still,
it is an API change.

I bumped catversion because code predating this change would get confused
by seeing reltuples = -1.

Discussion: https://postgr.es/m/F02298E0-6EF4-49A1-BCB6-C484794D9ACC@thebuild.com
      3d351d91
    • Michael Paquier's avatar
      Reset indisreplident for an invalid index in DROP INDEX CONCURRENTLY · 9511fb37
      Michael Paquier authored 
      A failure when dropping concurrently an index used in a replica identity
could leave in pg_index an index marked as !indisvalid and
indisreplident.  Reindexing this index would switch back indisvalid to
true, and if the replica identity of the parent relation was switched to
use a different index, it would be possible to finish with more than one
index marked as indisreplident.  If that were to happen, this could mess
up with the relation cache as an incorrect index could be used for the
replica identity.

Indexes marked as invalid are discarded as candidates for the replica
identity, as of RelationGetIndexList(), so similarly to what is done
with indisclustered, resetting indisreplident when the index is marked
as invalid keeps things consistent.  REINDEX CONCURRENTLY's swapping
already resets the flag for the old index, while the new index inherits
the value of the old index to-be-dropped, so only DROP INDEX was an
issue.

Even if this is a bug, the sequence able to reproduce a problem requires
a failure while running DROP INDEX CONCURRENTLY, something unlikely
going to happen in the field, so no backpatch is done.

Author: Michael Paquier
Reviewed-by: Dmitry Dolgov
Discussion: https://postgr.es/m/20200827025721.GN2017@paquier.xyz
      9511fb37
  7. 28 Aug, 2020 3 commits
  9. 27 Aug, 2020 2 commits
    • Tom Lane's avatar
      Fix code for re-finding scan position in a multicolumn GIN index. · 10564ee0
      Tom Lane authored 
      collectMatchBitmap() needs to re-find the index tuple it was previously
looking at, after transiently dropping lock on the index page it's on.
The tuple should still exist and be at its prior position or somewhere
to the right of that, since ginvacuum never removes tuples but
concurrent insertions could add one.  However, there was a thinko in
that logic, to the effect of expecting any inserted tuples to have the
same index "attnum" as what we'd been scanning.  Since there's no
physical separation of tuples with different attnums, it's not terribly
hard to devise scenarios where this fails, leading to transient "lost
saved point in index" errors.  (While I've duplicated this with manual
testing, it seems impossible to make a reproducible test case with our
available testing technology.)

Fix by just continuing the scan when the attnum doesn't match.

While here, improve the error message used if we do fail, so that it
matches the wording used in btree for a similar case.

collectMatchBitmap()'s posting-tree code path was previously not
exercised at all by our regression tests.  While I can't make
a regression test that exhibits the bug, I can at least improve
the code coverage here, so do that.  The test case I made for this
is an extension of one added by 4b754d6c, so it only works in
HEAD and v13; didn't seem worth trying hard to back-patch it.

Per bug #16595 from Jesse Kinkead.  This has been broken since
multicolumn capability was added to GIN (commit 27cb66fd),
so back-patch to all supported branches.

Discussion: https://postgr.es/m/16595-633118be8eef9ce2@postgresql.org
      10564ee0
    • Michael Paquier's avatar
      Fix comment in procarray.c · 77c7267c
      Michael Paquier authored 
      The description of GlobalVisDataRels was missing, GlobalVisCatalogRels
being mentioned instead.

Author: Jim Nasby
Discussion: https://postgr.es/m/8e06c883-2858-1fd4-07c5-560c28b08dcd@amazon.com
      77c7267c
  11. 26 Aug, 2020 7 commits
  13. 25 Aug, 2020 3 commits
  15. 24 Aug, 2020 2 commits
  17. 22 Aug, 2020 4 commits
  19. 21 Aug, 2020 1 commit