Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 20 Jun, 2018 9 commits
  3. 19 Jun, 2018 5 commits
  5. 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
  7. 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
  9. 16 Jun, 2018 7 commits
  11. 15 Jun, 2018 3 commits
  13. 14 Jun, 2018 3 commits
  15. 13 Jun, 2018 3 commits
    • Tom Lane's avatar
      Code review for match_clause_to_partition_key(). · 91781335
      Tom Lane authored 
      Fix inconsistent decisions about NOMATCH vs UNSUPPORTED result codes.
If we're going to cater for partkeys that have the same expression and
different collations, surely we should also support partkeys with the
same expression and different opclasses.

Clean up shaky handling of commuted opclauses, eg checking the wrong
operator to see what its negator is.  This wouldn't cause any actual
bugs given a sane opclass definition, but it doesn't seem helpful to
expend more code to be less correct.

Improve handling of null elements in ScalarArrayOp arrays: in the
"op ALL" case, we can conclude they result in an unsatisfiable clause.

Minor cosmetic changes and comment improvements.
      91781335
    • Tom Lane's avatar
      Fix some ill-chosen names for globally-visible partition support functions. · 19832753
      Tom Lane authored 
      "compute_hash_value" is particularly gratuitously generic, but IMO
all of these ought to have names clearly related to partitioning.
      19832753
    • Tom Lane's avatar
      Fix up run-time partition pruning's use of relcache's partition data. · e23bae82
      Tom Lane authored 
      The previous coding saved pointers into the partitioned table's relcache
entry, but then closed the relcache entry, causing those pointers to
nominally become dangling.  Actual trouble would be seen in the field
only if a relcache flush occurred mid-query, but that's hardly out of
the question.

While we could fix this by copying all the data in question at query
start, it seems better to just hold the relcache entry open for the
whole query.

While at it, improve the handling of support-function lookups: do that
once per query not once per pruning test.  There's still something to be
desired here, in that we fail to exploit the possibility of caching data
across queries in the fn_extra fields of the relcache's FmgrInfo structs,
which could happen if we just used those structs in-place rather than
copying them.  However, combining that with the possibility of per-query
lookups of cross-type comparison functions seems to require changes in the
APIs of a lot of the pruning support functions, so it's too invasive to
consider as part of this patch.  A win would ensue only for complex
partition key data types (e.g. arrays), so it may not be worth the
trouble.

David Rowley and Tom Lane

Discussion: https://postgr.es/m/17850.1528755844@sss.pgh.pa.us
      e23bae82