Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 19 Aug, 2019 7 commits
  3. 18 Aug, 2019 6 commits
    • Tom Lane's avatar
      Avoid conflicts with library versions of inet_net_ntop() and friends. · 927f34ce
      Tom Lane authored 
      Prefix inet_net_ntop and sibling routines with "pg_" to ensure that
they aren't mistaken for C-library functions.  This fixes warnings
from cpluspluscheck on some platforms, and should help reduce reader
confusion everywhere, since our functions aren't exactly interchangeable
with the library versions (they may have different ideas about address
family codes).

This shouldn't be fixing any actual bugs, unless somebody's linker
is misbehaving, so no need to back-patch.

Discussion: https://postgr.es/m/20518.1559494394@sss.pgh.pa.us
      927f34ce
    • Tom Lane's avatar
      Fix incidental warnings from cpluspluscheck. · 232720be
      Tom Lane authored 
      Remove use of "register" keyword in hashfn.c.  It's obsolescent
according to recent C++ compilers, and no modern C compiler pays
much attention to it either.

Also fix one cosmetic warning about signed vs unsigned comparison.

Discussion: https://postgr.es/m/20518.1559494394@sss.pgh.pa.us
      232720be
    • Tom Lane's avatar
      Fix failure-to-compile-standalone in scripts_parallel.h. · 5f110933
      Tom Lane authored 
      Needs libpq-fe.h for references to PGConn.

Discussion: https://postgr.es/m/17463.1566153454@sss.pgh.pa.us
      5f110933
    • Tom Lane's avatar
      Fix failure-to-compile-standalone in ecpg's dt.h. · 5c66e991
      Tom Lane authored 
      This has to have <time.h>, or the references to "struct tm" don't
mean what they should.

We have some other recently-introduced issues of the same ilk,
but this one seems old.  No backpatch though, as it's only a
latent problem for most purposes.
      5c66e991
    • Tom Lane's avatar
      Disallow changing an inherited column's type if not all parents changed. · 4d4c66ad
      Tom Lane authored 
      If a table inherits from multiple unrelated parents, we must disallow
changing the type of a column inherited from multiple such parents, else
it would be out of step with the other parents.  However, it's possible
for the column to ultimately be inherited from just one common ancestor,
in which case a change starting from that ancestor should still be
allowed.  (I would not be excited about preserving that option, were
it not that we have regression test cases exercising it already ...)

It's slightly annoying that this patch looks different from the logic
with the same end goal in renameatt(), and more annoying that it
requires an extra syscache lookup to make the test.  However, the
recursion logic is quite different in the two functions, and a
back-patched bug fix is no place to be trying to unify them.

Per report from Manuel Rigger.  Back-patch to 9.5.  The bug exists in
9.4 too (and doubtless much further back); but the way the recursion
is done in 9.4 is a good bit different, so that substantial refactoring
would be needed to fix it in 9.4.  I'm disinclined to do that, or risk
introducing new bugs, for a bug that has escaped notice for this long.

Discussion: https://postgr.es/m/CA+u7OA4qogDv9rz1HAb-ADxttXYPqQdUdPY_yd4kCzywNxRQXA@mail.gmail.com
      4d4c66ad
    • Peter Eisentraut's avatar
      Remove obsolete reference to Irix · 7e78c872
      Peter Eisentraut authored
      7e78c872
  5. 17 Aug, 2019 2 commits
    • Tom Lane's avatar
      Make deadlock-parallel isolation test more robust. · 9be4ce4f
      Tom Lane authored 
      This test failed fairly reproducibly on some CLOBBER_CACHE_ALWAYS
buildfarm animals.  The cause seems to be that if a parallel worker
is slow enough to reach its lock wait, it may not be released by
the first deadlock check run, and then later deadlock checks might
decide to unblock the d2 session instead of the d1 session, leaving
us in an undetected deadlock state (since the isolationtester client
is waiting for d1 to complete first).

Fix by introducing an additional lock wait at the end of the d2a1
step, ensuring that the deadlock checker will recognize that d1
has to be unblocked before d2a1 completes.

Also reduce max_parallel_workers_per_gather to 3 in this test.  With the
default max_worker_processes value, we were only getting one parallel
worker for the d2a1 step, which is not the case I hoped to test.  We
should get 3 for d1a2 and 2 for d2a1, as the code stands; and maybe 3
for d2a1 if somebody figures out why the last parallel worker slot isn't
free already.

Discussion: https://postgr.es/m/22195.1566077308@sss.pgh.pa.us
      9be4ce4f
    • Peter Eisentraut's avatar
      Improve Assert output · d78d452b
      Peter Eisentraut authored 
      If an assertion expression contained a macro, the failed assertion
message would print the expanded macro, which is usually unhelpful and
confusing.  Restructure the Assert macros to not expand any macros
when constructing the failure message.

This also fixes that the existing output for Assert et al. shows
the *inverted* condition, which is also confusing and not how
assertions usually work.

Discussion: https://www.postgresql.org/message-id/flat/6c68efe3-117a-dcc1-73d4-18ba1ec532e2%402ndquadrant.com
      d78d452b
  7. 16 Aug, 2019 7 commits
  9. 15 Aug, 2019 3 commits
    • Tom Lane's avatar
      Fix plpgsql to re-look-up composite type names at need. · fe9b7b2f
      Tom Lane authored 
      Commit 4b93f579 rearranged things in plpgsql to make it cope better with
composite types changing underneath it intra-session.  However, I failed to
consider the case of a composite type being dropped and recreated entirely.
In my defense, the previous coding didn't consider that possibility at all
either --- but it would accidentally work so long as you didn't change the
type's field list, because the built-at-compile-time list of component
variables would then still match the type's new definition.  The new
coding, however, occasionally tries to re-look-up the type by OID, and
then fails to find the dropped type.

To fix this, we need to save the TypeName struct, and then redo the type
OID lookup from that.  Of course that's expensive, so we don't want to do
it every time we need the type OID.  This can be fixed in the same way that
4b93f579 dealt with changes to composite types' definitions: keep an eye
on the type's typcache entry to see if its tupledesc has been invalidated.
(Perhaps, at some point, this mechanism should be generalized so it can
work for non-composite types too; but for now, plpgsql only tries to
cope with intra-session redefinitions of composites.)

I'm slightly hesitant to back-patch this into v11, because it changes
the contents of struct PLpgSQL_type as well as the signature of
plpgsql_build_datatype(), so in principle it could break code that is
poking into the innards of plpgsql.  However, the only popular extension
of that ilk is pldebugger, and it doesn't seem to be affected.  Since
this is a regression for people who were relying on the old behavior,
it seems worth taking the small risk of causing compatibility issues.

Per bug #15913 from Daniel Fiori.  Back-patch to v11 where 4b93f579
came in.

Discussion: https://postgr.es/m/15913-a7e112e16dedcffc@postgresql.org
      fe9b7b2f
    • Tom Lane's avatar
      Use a hash table to de-duplicate NOTIFY events faster. · bb5ae8f6
      Tom Lane authored 
      Previously, async.c got rid of duplicate notifications by scanning
the list of pending events to compare each one to the proposed new
event.  This works okay for very small numbers of distinct events,
but degrades as O(N^2) for many events.  We can improve matters by
using a hash table to probe for duplicates.  So as not to add a
lot of overhead for the simple cases that the code did handle well
before, create the hash table only once a (sub)transaction has
queued more than 16 distinct notify events.

A downside is that we now have to do per-event work to propagate
a successful subtransaction's notify events up to its parent.
(But this isn't significant unless the subtransaction had many
events, in which case the O(N^2) behavior would have been in
play already, so we still come out ahead.)

We can make some lemonade out of this lemon, though: since we must
examine each event anyway, it's now possible to de-duplicate events
fully, rather than skipping that for events merged up from
subtransactions.  Hence, remove the old weasel wording in notify.sgml
about whether de-duplication happens or not, and adjust the test
case in async-notify.spec that exhibited the old behavior.

While at it, rearrange the definition of struct Notification to make
it more compact and require just one palloc per event, rather than
two or three.  This saves space when there are a lot of events,
in fact more than enough to buy back the space needed for the hash
table.

Patch by me, based on discussions around a different patch
submitted by Filip Rembiałkowski.

Discussion: https://postgr.es/m/17822.1564186806@sss.pgh.pa.us
      bb5ae8f6
    • Tom Lane's avatar
      Doc: improve documentation about postgresql.auto.conf. · 45aaaa42
      Tom Lane authored 
      Clarify what external tools can do to this file, and add a bit
of detail about what ALTER SYSTEM itself does.

Discussion: https://postgr.es/m/aed6cc9f-98f3-2693-ac81-52bb0052307e@2ndquadrant.com
      45aaaa42
  11. 14 Aug, 2019 5 commits
  13. 13 Aug, 2019 6 commits
  15. 12 Aug, 2019 4 commits
    • Peter Geoghegan's avatar
      amcheck: Skip unlogged relations during recovery. · 6754fe65
      Peter Geoghegan authored 
      contrib/amcheck failed to consider the possibility that unlogged
relations will not have any main relation fork files when running in hot
standby mode.  This led to low-level "can't happen" errors that complain
about the absence of a relfilenode file.

To fix, simply skip verification of unlogged index relations during
recovery.  In passing, add a direct check for the presence of a main
fork just before verification proper begins, so that we cleanly verify
the presence of the main relation fork file.

Author: Andrey Borodin, Peter Geoghegan
Reported-By: Andrey Borodin
Diagnosed-By: Andrey Borodin
Discussion: https://postgr.es/m/DA9B33AC-53CB-4643-96D4-7A0BBC037FA1@yandex-team.ru
Backpatch: 10-, where amcheck was introduced.
      6754fe65
    • Tom Lane's avatar
      Fix planner's test for case-foldable characters in ILIKE with ICU. · 03c811a4
      Tom Lane authored 
      As coded, the ICU-collation path in pattern_char_isalpha() failed
to consider regular ASCII letters to be case-varying.  This led to
like_fixed_prefix treating too much of an ILIKE pattern as being a
fixed prefix, so that indexscans derived from an ILIKE clause might
miss entries that they should find.

Per bug #15892 from James Inform.  This is an oversight in the original
ICU patch (commit eccfef81), so back-patch to v10 where that came in.

Discussion: https://postgr.es/m/15892-e5d2bea3e8a04a1b@postgresql.org
      03c811a4
    • Tom Lane's avatar
      Remove EState.es_range_table_array. · 3c926587
      Tom Lane authored 
      Now that list_nth is O(1), there's no good reason to maintain a
separate array of RTE pointers rather than indexing into
estate->es_range_table.  Deleting the array doesn't save all that
much either; but just on cleanliness grounds, it's better not to
have duplicate representations of the identical information.

Discussion: https://postgr.es/m/14960.1565384592@sss.pgh.pa.us
      3c926587
    • Tom Lane's avatar
      Rationalize use of list_concat + list_copy combinations. · 5ee190f8
      Tom Lane authored 
      In the wake of commit 1cff1b95, the result of list_concat no longer
shares the ListCells of the second input.  Therefore, we can replace
"list_concat(x, list_copy(y))" with just "list_concat(x, y)".

To improve call sites that were list_copy'ing the first argument,
or both arguments, invent "list_concat_copy()" which produces a new
list sharing no ListCells with either input.  (This is a bit faster
than "list_concat(list_copy(x), y)" because it makes the result list
the right size to start with.)

In call sites that were not list_copy'ing the second argument, the new
semantics mean that we are usually leaking the second List's storage,
since typically there is no remaining pointer to it.  We considered
inventing another list_copy variant that would list_free the second
input, but concluded that for most call sites it isn't worth worrying
about, given the relative compactness of the new List representation.
(Note that in cases where such leakage would happen, the old code
already leaked the second List's header; so we're only discussing
the size of the leak not whether there is one.  I did adjust two or
three places that had been troubling to free that header so that
they manually free the whole second List.)

Patch by me; thanks to David Rowley for review.

Discussion: https://postgr.es/m/11587.1550975080@sss.pgh.pa.us
      5ee190f8