Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 22 Jun, 2018 2 commits
  3. 21 Jun, 2018 6 commits
  5. 20 Jun, 2018 11 commits
  7. 19 Jun, 2018 5 commits
  9. 18 Jun, 2018 9 commits
    • Tom Lane's avatar
      Fix jsonb_plperl to convert Perl UV values correctly. · 93b6e03a
      Tom Lane authored 
      Values greater than IV_MAX were incorrectly converted to SQL,
for instance ~0 would become -1 rather than 18446744073709551615
(on a 64-bit machine).

Dagfinn Ilmari Mannsåker, adjusted a bit by me

Discussion: https://postgr.es/m/d8jtvskjzzs.fsf@dalvik.ping.uio.no
      93b6e03a
    • Tom Lane's avatar
      Fix contrib/hstore_plperl to look through scalar refs. · e3b7f7cc
      Tom Lane authored 
      Bring this transform function into sync with the policy established
by commit 3a382983.

Also, fix it to make sure that what it drills down to is indeed a
hash, and not some other kind of Perl SV.  Previously, the test
cases added here provoked crashes.

Because of the crash hazard, back-patch to 9.5 where this module
was introduced.

Discussion: https://postgr.es/m/28336.1528393969@sss.pgh.pa.us
      e3b7f7cc
    • Tom Lane's avatar
      Allow plperl_sv_to_datum to look through scalar refs. · 3a382983
      Tom Lane authored 
      There seems little reason for the policy of throwing error if we
find a ref to something other than a hash or array.   Recursively
look through the ref, instead.  This makes the behavior in non-transform
cases comparable to what was already instantiated for jsonb_plperl.

Note that because we invoke any available transform function before
considering the ref case, it's up to each transform function whether
it wants to play along with this behavior or do something different.

Because the previous behavior was just to throw a useless error,
this seems unlikely to create any compatibility issues.  Still, given
the lack of field complaints so far, seems best not to back-patch.

Discussion: https://postgr.es/m/28336.1528393969@sss.pgh.pa.us
      3a382983
    • Tom Lane's avatar
      Avoid platform-dependent output from Data::Dumper. · e4300a35
      Tom Lane authored 
      Per buildfarm, the output from Data::Dumper for an IEEE infinity
is platform-dependent (e.g. "inf" vs "Inf").  Just skip that one
test case in the plperlu test; testing it on the plperl side is
coverage enough.  Fixes issue in commit 1731e374.
      e4300a35
    • Tom Lane's avatar
      Fix excessive enreferencing in jsonb-to-plperl transform. · 1731e374
      Tom Lane authored 
      We want, say, 2 to be transformed as 2, not \\2 which is what the
original coding produced.  Perl's standard seems to be to add an RV
wrapper only for hash and array SVs, so do it like that.

This was missed originally because the test cases only checked what came
out of a round trip back to SQL, and the strip-all-dereferences loop at
the top of SV_to_JsonbValue hides the extra refs from view.  As a better
test, print the Perl value with Data::Dumper, like the hstore_plperlu
tests do.  While we can't do that in the plperl test, only plperlu,
that should be good enough because this code is the same for both PLs.
But also add a simplistic test for extra REFs, which we can do in both.

That strip-all-dereferences behavior is now a bit dubious; it's unlike
what happens for other Perl-to-SQL conversions.  However, the best
thing to do seems to be to leave it alone and make the other conversions
act similarly.  That will be done separately.

Dagfinn Ilmari Mannsåker, adjusted a bit by me

Discussion: https://postgr.es/m/d8jlgbq66t9.fsf@dalvik.ping.uio.no
      1731e374
    • Tom Lane's avatar
      Remove obsolete prohibition on function name matching a column name. · 45e98ee7
      Tom Lane authored 
      ProcedureCreate formerly threw an error if the function to be created
has one argument of composite type and the function name matches some
column of the composite type.  This was a (very non-bulletproof) defense
against creating situations where f(x) and x.f are ambiguous.  But we
don't really need such a defense in the wake of commit b97a3465, which
allows us to deal with such situations fairly cleanly.  This behavior
also created a dump-and-reload hazard, since a function might be
rejected if a conflicting column name had been added to the input
composite type later.  Hence, let's just drop the check.

Discussion: https://postgr.es/m/CAOW5sYa3Wp7KozCuzjOdw6PiOYPi6D=VvRybtH2S=2C0SVmRmA@mail.gmail.com
      45e98ee7
    • Tom Lane's avatar
      Consider syntactic form when disambiguating function vs column reference. · b97a3465
      Tom Lane authored 
      Postgres has traditionally considered the syntactic forms f(x) and x.f
to be equivalent, allowing tricks such as writing a function and then
using it as though it were a computed-on-demand column.  However, our
behavior when both interpretations are feasible left something to be
desired: we always chose the column interpretation.  This could lead
to very surprising results, as in a recent bug report from Neil Conway.
It also created a dump-and-reload hazard, since what was a function
call in a dumped view could get interpreted as a column reference
at reload, if a matching column name had been added to the underlying
table since the view was created.

What seems better, in ambiguous situations, is to prefer the choice
matching the syntactic form of the reference.  This seems much less
astonishing in general, and it fixes the dump/reload hazard.

Although this could be called a bug fix, there have been few complaints
and there's some small risk of breaking applications that depend on the
old behavior, so no back-patch.  It does seem reasonable to slip it
into v11, though.

Discussion: https://postgr.es/m/CAOW5sYa3Wp7KozCuzjOdw6PiOYPi6D=VvRybtH2S=2C0SVmRmA@mail.gmail.com
      b97a3465
    • Thomas Munro's avatar
      Add PGTYPESchar_free() to avoid cross-module problems on Windows. · 4c8156d8
      Thomas Munro authored 
      On Windows, it is sometimes important for corresponding malloc() and
free() calls to be made from the same DLL, since some build options can
result in multiple allocators being active at the same time.  For that
reason we already provided PQfreemem().  This commit adds a similar
function for freeing string results allocated by the pgtypes library.

Author: Takayuki Tsunakawa
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F8AD5D6%40G01JPEXMBYT05
      4c8156d8
    • Michael Paquier's avatar
      Prevent hard failures of standbys caused by recycled WAL segments · 70b4f82a
      Michael Paquier authored 
      When a standby's WAL receiver stops reading WAL from a WAL stream, it
writes data to the current WAL segment without having priorily zero'ed
the page currently written to, which can cause the WAL reader to read
junk data from a past recycled segment and then it would try to get a
record from it.  While sanity checks in place provide most of the
protection needed, in some rare circumstances, with chances increasing
when a record header crosses a page boundary, then the startup process
could fail violently on an allocation failure, as follows:
FATAL:  invalid memory alloc request size XXX

This is confusing for the user and also unhelpful as this requires in
the worst case a manual restart of the instance, impacting potentially
the availability of the cluster, and this also makes WAL data look like
it is in a corrupted state.

The chances of seeing failures are higher if the connection between the
standby and its root node is unstable, causing WAL pages to be written
in the middle.  A couple of approaches have been discussed, like
zero-ing  new WAL pages within the WAL receiver itself but this has the
disadvantage of impacting performance of any existing instances as this
breaks the sequential writes done by the WAL receiver.  This commit
deals with the problem with a more simple approach, which has no
performance impact without reducing the detection of the problem: if a
record is found with a length higher than 1GB for backends, then do not
try any allocation and report a soft failure which will force the
standby to retry reading WAL.  It could be possible that the allocation
call passes and that an unnecessary amount of memory is allocated,
however follow-up checks on records would just fail, making this
allocation short-lived anyway.

This patch owes a great deal to Tsunakawa Takayuki for reporting the
failure first, and then discussing a couple of potential approaches to
the problem.

Backpatch down to 9.5, which is where palloc_extended has been
introduced.

Reported-by: Tsunakawa Takayuki
Reviewed-by: Tsunakawa Takayuki
Author: Michael Paquier
Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F8B57AD@G01JPEXMBYT05
      70b4f82a
  11. 17 Jun, 2018 1 commit
    • Tom Lane's avatar
      Suppress -Wshift-negative-value warnings. · 9b53d966
      Tom Lane authored 
      Clean up four places that result in compiler warnings when using recent
gcc with this warning class enabled (as seen on buildfarm members
calliphoridae, skink, and others).  In all these places, this is purely
cosmetic, because the shift distance could not be large enough to risk
a change of sign, so there's no chance of implementation-dependent
behavior.  Still, it's easy enough to avoid the warning by casting the
shifted value to unsigned, so let's do that.

Patch HEAD only, this isn't worth a back-patch.
      9b53d966
  13. 16 Jun, 2018 6 commits
    • Peter Geoghegan's avatar
      Remove INCLUDE attributes section from docs. · 514d4a13
      Peter Geoghegan authored 
      Discussing covering indexes in a chapter that is mostly about the
behavior of B-Tree operator classes is unnecessary.  The CREATE INDEX
documentation's handling of covering indexes seems sufficient.

Discussion: https://postgr.es/m/CAH2-WzmpU=L_6VjhhOAMfoyHLr-pZd1kDc+jpa3c3a8EOmtcXA@mail.gmail.com
      514d4a13
    • Tom Lane's avatar
      Use -Wno-format-truncation and -Wno-stringop-truncation, if available. · e7165852
      Tom Lane authored 
      gcc 8 has started emitting some warnings that are largely useless for
our purposes, particularly since they complain about code following
the project-standard coding convention that path names are assumed
to be shorter than MAXPGPATH.  Even if we make the effort to remove
that assumption in some future release, the changes wouldn't get
back-patched.  Hence, just suppress these warnings, on compilers that
have these switches.

Backpatch to all supported branches.

Discussion: https://postgr.es/m/1524563856.26306.9.camel@gunduz.org
      e7165852
    • Tom Lane's avatar
      Avoid unnecessary use of strncpy in a couple of places in ecpg. · 6b74f5ea
      Tom Lane authored 
      Use of strncpy with a length limit based on the source, rather than
the destination, is non-idiomatic and draws warnings from gcc 8.
Replace with memcpy, which does exactly the same thing in these cases,
but with less chance for confusion.

Backpatch to all supported branches.

Discussion: https://postgr.es/m/21789.1529170195@sss.pgh.pa.us
      6b74f5ea
    • Tom Lane's avatar
      Use snprintf not sprintf in pg_waldump's timestamptz_to_str. · 5d923eb2
      Tom Lane authored 
      This could only cause an issue if strftime returned a ridiculously
long timezone name, which seems unlikely; and it wouldn't qualify
as a security problem even then, since pg_waldump (nee pg_xlogdump)
is a debug tool not part of the server.  But gcc 8 has started issuing
warnings about it, so let's use snprintf and be safe.

Backpatch to 9.3 where this code was added.

Discussion: https://postgr.es/m/21789.1529170195@sss.pgh.pa.us
      5d923eb2
    • Tom Lane's avatar
      Fix some minor error-checking oversights in ParseFuncOrColumn(). · 0dcf68e5
      Tom Lane authored 
      Recent additions to ParseFuncOrColumn to make it reject non-procedure
functions in CALL were neither adequate nor documented.  Reorganize
the code to ensure uniform results for all the cases that should be
rejected.  Also, use ERRCODE_WRONG_OBJECT_TYPE for this case as well
as the converse case of a procedure in a non-CALL context.  The
original coding used ERRCODE_UNDEFINED_FUNCTION which seems wrong,
and is certainly inconsistent with the adjacent wrong-kind-of-routine
errors.

This reorganization also causes the checks for aggregate decoration with
a non-aggregate function to be made in the FUNCDETAIL_COERCION case;
that they were not is a long-standing oversight.

Discussion: https://postgr.es/m/14497.1529089235@sss.pgh.pa.us
      0dcf68e5
    • Simon Riggs's avatar
      Remove AELs from subxids correctly on standby · 15378c1a
      Simon Riggs authored 
      Issues relate only to subtransactions that hold AccessExclusiveLocks
when replayed on standby.

Prior to PG10, aborting subtransactions that held an
AccessExclusiveLock failed to release the lock until top level commit or
abort. 49bff530 fixed that.

However, 49bff530 also introduced a similar bug where subtransaction
commit would fail to release an AccessExclusiveLock, leaving the lock to
be removed sometimes early and sometimes late. This commit fixes
that bug also. Backpatch to PG10 needed.

Tested by observation. Note need for multi-node isolationtester to improve
test coverage for this and other HS cases.

Reported-by: Simon Riggs
Author: Simon Riggs
      15378c1a