Submit news and announcements by Sunday at 3:00pm PST8PDT to david@fetter.org.
-
Fix two issues in TOAST decompression. pglz_maximum_compressed_size()
potentially underestimated the amount of compressed data required to produce N
bytes of decompressed data; this is a fault in commit 11a078cf8. Separately
from that, pglz_decompress() failed to protect itself against corrupt
compressed data, particularly off == 0 in a match tag. Commit c60e520f6
turned such a situation into an infinite loop, where before it'd just have
resulted in garbage output. The combination of these two bugs seems like it
may explain bug #16694 from Tom Vijlbrief, though it's impossible to be quite
sure without direct inspection of the failing session. (One needs to assume
that the pglz_maximum_compressed_size() bug caused us to fail to fetch the
second byte of a match tag, and what happened to be there instead was a zero.
The reported infinite loop is hard to explain without off == 0, though.)
Aside from fixing the bugs, rewrite associated comments for more clarity.
Back-patch to v13 where both these commits landed. Discussion:
https://postgr.es/m/16694-f107871e499ec114@postgresql.org
https://git.postgresql.org/pg/commitdiff/dfc797730fc7a07c0e6bd636ad1a564aecab3161
-
Second thoughts on TOAST decompression. On detecting a corrupted match tag,
pglz_decompress() should just summarily return -1. Breaking out of the loop,
as I did in dfc797730, doesn't quite guarantee that will happen. Also, we can
use unlikely() on that check, just in case it helps. Backpatch to v13, like
the previous patch.
https://git.postgresql.org/pg/commitdiff/fd2997565c6f66837440dd57f5e52b56aa964d14
-
Rethink the generation rule for fmgroids.h macros. Traditionally, the names of
fmgroids.h macros for pg_proc OIDs have been constructed from the prosrc
field. But sometimes the same C function underlies multiple pg_proc entries,
forcing us to make an arbitrary choice of which OID to reference; the other
entries are then not namable via fmgroids.h. Moreover, we could not have
macros at all for pg_proc entries that aren't for C-coded functions. Instead,
use the proname field, and append the proargtypes field (replacing
inter-argument spaces with underscores) if proname is not unique.
Special-casing unique entries such as F_OIDEQ removes the need to change a lot
of code. Indeed, I can only find two places in the tree that need to be
adjusted; while this changes quite a few existing entries in fmgroids.h, few
of them are referenced from C code. With this patch, all entries in
pg_proc.dat have macros in fmgroids.h. Discussion:
https://postgr.es/m/472274.1604258384@sss.pgh.pa.us
https://git.postgresql.org/pg/commitdiff/8e1f37c07aafd4bb7aa6e1e1982010af11f8b5c7
-
Remove special checks for pg_rewrite.ev_qual and ev_action being NULL.
make_ruledef() and make_viewdef() were coded to cope with possible null-ness
of these columns, but they've been marked BKI_FORCE_NOT_NULL for some time.
So there's not really any need to do more than what we do for the other
columns of pg_rewrite, i.e. just Assert that we got non-null results. (There
is a school of thought that says Asserts aren't the thing to do to check for
corrupt data, but surely here is not the place to start if we want such a
policy.) Also, remove long-dead-if-indeed-it-ever-wasn't-dead handling of an
empty actions list in make_ruledef(). That's an error case and should be
treated as such. (DO INSTEAD NOTHING is represented by a CMD_NOTHING Query,
not an empty list; cf transformRuleStmt.) Kyotaro Horiguchi, some changes by
me Discussion:
https://postgr.es/m/CAEudQApoA=tMTic6xEPYP_hsNZ8XtToVThK_0x7D_aFQYowq3w@mail.gmail.com
https://git.postgresql.org/pg/commitdiff/e1339bfc7a2fd4629e1c3f8f919ddd05b4745e13
-
Fix unportable use of getnameinfo() in pg_hba_file_rules view. fill_hba_line()
thought it could get away with passing sizeof(struct sockaddr_storage) rather
than the actual addrlen previously returned by getaddrinfo(). While that
appears to work on many platforms, it does not work on FreeBSD 11: you get
back a failure, which leads to the view showing NULL for the address and
netmask columns in all rows. The POSIX spec for getnameinfo() is pretty
clearly on FreeBSD's side here: you should pass the actual address length. So
it seems plausible that there are other platforms where this coding also
fails, and we just hadn't noticed. Also, IMO the fact that getnameinfo()
failure leads to a NULL output is pretty bogus in itself. Our
pg_getnameinfo_all() wrapper is careful to emit "???" on failure, and we
should use that in such cases. NULL should only be emitted in rows that don't
have IP addresses. Per bug #16695 from Peter Vandivier. Back-patch to v10
where this code was added. Discussion:
https://postgr.es/m/16695-a665558e2f630be7@postgresql.org
https://git.postgresql.org/pg/commitdiff/0a4b34031279d938c2e59df8df7159d6c11e39b5
-
Allow users with BYPASSRLS to alter their own passwords. The intention in
commit 491c029db was to require superuserness to change the BYPASSRLS
property, but the actual effect of the coding in AlterRole() was to require
superuserness to change anything at all about a BYPASSRLS role. Other
properties of a BYPASSRLS role should be changeable under the same rules as
for a normal role, though. Fix that, and also take care of some documentation
omissions related to BYPASSRLS and REPLICATION role properties. Tom Lane and
Stephen Frost, per bug report from Wolfgang Walther. Back-patch to all
supported branches. Discussion:
https://postgr.es/m/a5548a9f-89ee-3167-129d-162b5985fcf8@technowledgy.de
https://git.postgresql.org/pg/commitdiff/d907bd0543aa63e59653d7345840bed0f8b3a83b
-
Improve error messages around REPLICATION and BYPASSRLS properties. Clarify
wording as per suggestion from Wolfgang Walther. No back-patch; this doesn't
seem worth thrashing translatable strings in the back branches. Tom Lane and
Stephen Frost Discussion:
https://postgr.es/m/a5548a9f-89ee-3167-129d-162b5985fcf8@technowledgy.de
https://git.postgresql.org/pg/commitdiff/17fb60387ce3fdc2bbb13d9b67bed0e4da77e173
-
Guard against core dump from uninitialized subplan. If the planner erroneously
puts a non-parallel-safe SubPlan into a parallelized portion of the query
tree, nodeSubplan.c will fail in the worker processes because it finds a null
in es_subplanstates, which it's unable to cope with. It seems worth a
test-and-elog to make that an error case rather than a core dump case. This
probably should have been included in commit 16ebab688, which was responsible
for allowing nulls to appear in es_subplanstates to begin with. So,
back-patch to v10 where that came in. Discussion:
https://postgr.es/m/924226.1604422326@sss.pgh.pa.us
https://git.postgresql.org/pg/commitdiff/92f87182f2c617fd420832972b6d0ae4527301c8
-
Remove useless entries for aggregate functions from fmgrtab.c. Gen_fmgrtab.pl
treated aggregate functions the same as other built-in functions, which is
wasteful because there is no real need to have entries for them in the
fmgr_builtins[] table. Suppressing those entries saves about 3KB in the
compiled table on my machine; which is not a lot but it's not nothing either,
considering that that table is pretty "hot". The only outside code change
needed is that ExecInitWindowAgg() can't be allowed to call fmgr_info_cxt() on
a plain aggregate function. But that saves a few cycles anyway. Having done
that, the aggregate_dummy() function is unreferenced and might as well be
dropped. Using "aggregate_dummy" as the prosrc value for an aggregate is now
just a documentation convention not something that matters. There was some
discussion of using NULL instead to save a few bytes in pg_proc, but we'd have
to remove prosrc's BKI_FORCE_NOT_NULL marking which doesn't seem a great idea.
Anyway, it's possible there's client-side code that expects to see
"aggregate_dummy" there, so I'm loath to change it without a strong reason.
Discussion: https://postgr.es/m/533989.1604263665@sss.pgh.pa.us
https://git.postgresql.org/pg/commitdiff/f21636e5d5b8394ed076e18ddc5f4ba710c69c99
-
Improve our ability to regurgitate SQL-syntax function calls. The SQL spec
calls out nonstandard syntax for certain function calls, for example
substring() with numeric position info is supposed to be spelled
"SUBSTRING(string FROM start FOR count)". We accept many of these things, but
up to now would not print them in the same format, instead simplifying down to
"substring"(string, start, count). That's long annoyed me because it creates
an interoperability problem: we're gratuitously injecting Postgres-specific
syntax into what might otherwise be a perfectly spec-compliant view
definition. However, the real reason for addressing it right now is to support
a planned change in the semantics of EXTRACT() a/k/a date_part(). When we
switch that to returning numeric, we'll have the parser translate EXTRACT() to
some new function name (might as well be "extract" if you ask me) and then
teach ruleutils.c to reverse-list that per SQL spec. In this way existing
calls to date_part() will continue to have the old semantics. To implement
this, invent a new CoercionForm value COERCE_SQL_SYNTAX, and make the parser
insert that rather than COERCE_EXPLICIT_CALL when the input has SQL-spec
decoration. (But if the input has the form of a plain function call, continue
to mark it COERCE_EXPLICIT_CALL, even if it's calling one of these functions.)
Then ruleutils.c recognizes COERCE_SQL_SYNTAX as a cue to emit SQL call
syntax. It can know which decoration to emit using hard-wired knowledge about
the functions that could be called this way. (While this solution isn't
extensible without manual additions, neither is the grammar, so this doesn't
seem unmaintainable.) Notice that this solution will reverse-list a function
call with SQL decoration only if it was entered that way; so dump-and-reload
will not by itself produce any changes in the appearance of views. This
requires adding a CoercionForm field to struct FuncCall. (I couldn't resist
the temptation to rearrange that struct's field order a tad while I was at
it.) FuncCall doesn't appear in stored rules, so that change isn't a reason
for a catversion bump, but I did one anyway because the new enum value for
CoercionForm fields could confuse old backend code. Possible future work: *
Perhaps CoercionForm should now be renamed to DisplayForm, or something like
that, to reflect its more general meaning. This'd require touching a couple
hundred places, so it's not clear it's worth the code churn. * The
SQLValueFunction node type, which was invented partly for the same goal of
improving SQL-compatibility of view output, could perhaps be replaced with
regular function calls marked with COERCE_SQL_SYNTAX. It's unclear if this
would be a net code savings, however. Discussion:
https://postgr.es/m/42b73d2d-da12-ba9f-570a-420e0cce19d9@phystech.edu
https://git.postgresql.org/pg/commitdiff/40c24bfef92530bd846e111c1742c2a54441c62c
-
Declare lead() and lag() using anycompatible not anyelement. This allows use
of a "default" expression that doesn't slavishly match the data column's type.
Formerly you got something like "function lag(numeric, integer, integer) does
not exist", which is not just unhelpful but actively misleading. The SQL spec
suggests that the default should be coerced to the data column's type, but
this implementation instead chooses the common supertype, which seems at least
as reasonable. (Note: I took the opportunity to run "make reformat-dat-files"
on pg_proc.dat, so this commit includes some cosmetic changes to
recently-added entries that aren't related to lead/lag.) Vik Fearing
Discussion:
https://postgr.es/m/77675130-89da-dab1-51dd-492c93dcf5d1@postgresfriends.org
https://git.postgresql.org/pg/commitdiff/5c292e6b90433c760a3e15027646c7b94afd0cdd
-
Declare assorted array functions using anycompatible not anyelement. Convert
array_append, array_prepend, array_cat, array_position, array_positions,
array_remove, array_replace, and width_bucket to use anycompatiblearray. This
is a simple extension of commit 5c292e6b9 to hit some other places where
there's a pretty obvious gain in usability from doing so. Ideally we'd also
modify other functions taking multiple old-style polymorphic arguments. But
most of the remainder are tied into one or more operator classes, making any
such change a much larger can of worms than I desire to open right now.
Discussion:
https://postgr.es/m/77675130-89da-dab1-51dd-492c93dcf5d1@postgresfriends.org
https://git.postgresql.org/pg/commitdiff/9e38c2bb5093ceb0c04d6315ccd8975bd17add66
-
Remove underflow error in float division with infinite divisor. float4_div and
float8_div correctly produced zero for zero divided by infinity, but threw an
underflow error for nonzero finite values divided by infinity. This seems
wrong; at the very least it's inconsistent with the behavior recently
implemented for numeric infinities. Remove the error and allow zero to be
returned. This patch also removes a useless isinf() test from the overflow
checks in these functions (non-Inf divided by Inf can't produce Inf).
Extracted from a larger patch; this seems significant outside the context of
geometric operators, so it deserves its own commit. Kyotaro Horiguchi
Discussion:
https://postgr.es/m/CAGf+fX70rWFOk5cd00uMfa__0yP+vtQg5ck7c2Onb-Yczp0URA@mail.gmail.com
https://git.postgresql.org/pg/commitdiff/fac83dbd6fe1ac3d4125bfa39f287f95bffe6cda
-
Don't throw an error for LOCK TABLE on a self-referential view. LOCK TABLE has
complained about "infinite recursion" when applied to a self-referential view,
ever since we made it recurse into views in v11. However, that breaks
pg_dump's new assumption that it's okay to lock every relation. There doesn't
seem to be any good reason to throw an error: if we just abandon the
recursion, we've still satisfied the requirement of locking every referenced
relation. Per bug #16703 from Andrew Bille (via Alexander Lakhin).
Discussion: https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org
https://git.postgresql.org/pg/commitdiff/5b7bfc39726ff9f6c52dd73e337c34e74e0d1f39
-
Revert "pg_dump: Lock all relations, not just plain tables". Revert 403a3d91c,
as well as the followup fix 7f4235032, in all branches. We need to think a
bit harder about what the behavior of LOCK TABLE on views should be, and
there's no time for that before next week's releases. We'll take another
crack at this later. Discussion:
https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org
https://git.postgresql.org/pg/commitdiff/d3adaabaf7d555ec8bb1d83c43f72e79f1bf0b7d
-
Revert "Accept relations of any kind in LOCK TABLE". Revert 59ab4ac32, as well
as the followup fix 33862cb9c, in all branches. We need to think a bit harder
about what the behavior of LOCK TABLE on views should be, and there's no time
for that before next week's releases. We'll take another crack at this later.
Discussion: https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org
https://git.postgresql.org/pg/commitdiff/eeda7f6338095701cfe1ba3da37070508efe019e
-
Fix ecpg's mishandling of B'...' and X'...' literals. These were broken in
multiple ways: * The xbstart and xhstart lexer actions neglected to set
"state_before_str_start" before transitioning to the xb/xh states, thus
possibly resulting in "internal error: unreachable state" later. * The test
for valid string contents at the end of xb state was flat out wrong, as it
accounted incorrectly for the "b" prefix that the xbstart action had injected.
Meanwhile, the xh state had no such check at all. * The generated literal
value failed to include any quote marks. * The grammar did the wrong thing
anyway, typically ignoring the literal value and emitting something else,
since BCONST and XCONST tokens were handled randomly differently from SCONST
tokens. The first of these problems is evidently an oversight in commit
7f380c59f, but the others seem to be very ancient. The lack of complaints
shows that ECPG users aren't using these syntaxes much (although I do vaguely
remember one previous complaint). As written, this patch is dependent on
7f380c59f, so it can't go back further than v13. Given the shortage of
complaints, I'm not excited about adapting the patch to prior branches.
Report and patch by Shenhao Wang (test case adjusted by me) Discussion:
https://postgr.es/m/d6402f1bacb74ecba22ef715dbba17fd@G08CNEXMBPEKD06.g08.fujitsu.local
https://git.postgresql.org/pg/commitdiff/1e3868ab3bef5cfa0f4d44a6937a880be7a3a482
-
Avoid re-using output variables in new ecpg test case. The buildfarm thinks
this leads to memory stomps, though annoyingly I can't duplicate that here.
The existing code in strings.pgc is doing something that doesn't seem to be
sanctioned at all really by the documentation, but I'm disinclined to try to
make that nicer right now. Let's just declare some more output variables in
hopes of working around it.
https://git.postgresql.org/pg/commitdiff/eed4356fad84b0fd6e3caa49c7006f401159ac9a
