Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 27 Oct, 2020 5 commits
  3. 26 Oct, 2020 4 commits
  5. 25 Oct, 2020 2 commits
    • Tom Lane's avatar
      Fix corner case for a BEFORE ROW UPDATE trigger returning OLD. · ba9f18ab
      Tom Lane authored 
      If the old row has any "missing" attributes that are supposed to
be retrieved from an associated tuple descriptor, the wrong things
happened because the trigger result is shoved directly into an
executor slot that lacks the missing-attribute data.  Notably,
CHECK-constraint verification would incorrectly see those columns
as NULL, and so would RETURNING-list evaluation.

Band-aid around this by forcibly expanding the tuple before passing
it to the trigger function.  (IMO it was a fundamental misdesign to
put the missing-attribute data into tuple constraints, which so
much of the system considers to be optional.  But we're probably
stuck with that now, and will have to continue to apply band-aids
as we find other places with similar issues.)

Back-patch to v12.  v11 would also have the issue, except that
commit 920311ab1 already applied a similar band-aid.  That forced
expansion in more cases than seem really necessary, though, so
this isn't a directly equivalent fix.

Amit Langote, with some cosmetic changes by me

Discussion: https://postgr.es/m/16644-5da7ef98a7ac4545@postgresql.org
      ba9f18ab
    • David Rowley's avatar
      Fix incorrect parameter name in a function header comment · e83c9f91
      David Rowley authored 
      Author: Zhijie Hou
Discussion: https://postgr.es/m/14cd74ea00204cc8a7ea5d738ac82cd1@G08CNEXMBPEKD05.g08.fujitsu.local
Backpatch-through: 12, where the mistake was introduced
      e83c9f91
  7. 24 Oct, 2020 3 commits
  9. 23 Oct, 2020 10 commits
  11. 22 Oct, 2020 7 commits
    • Tom Lane's avatar
      Add documentation and tests for quote marks in ECPG literal queries. · c16a1bbc
      Tom Lane authored 
      ECPG's PREPARE ... FROM and EXECUTE IMMEDIATE can optionally take
the target query as a simple literal, rather than the more usual
string-variable reference.  This was previously documented as
being a C string literal, but that's a lie in one critical respect:
you can't write a data double quote as \" in such literals.  That's
because the lexer is in SQL mode at this point, so it'll parse
double-quoted strings as SQL identifiers, within which backslash
is not special, so \" ends the literal.

I looked into making this work as documented, but getting the lexer
to switch behaviors at just the right point is somewhere between
very difficult and impossible.  It's not really worth the trouble,
because these cases are next to useless: if you have a fixed SQL
statement to execute or prepare, you might as well write it as
a direct EXEC SQL, saving the messiness of converting it into
a string literal and gaining the opportunity for compile-time
SQL syntax checking.

Instead, let's just document (and test) the workaround of writing
a double quote as an octal escape (\042) in such cases.

There's no code behavioral change here, so in principle this could
be back-patched, but it's such a niche case I doubt it's worth
the trouble.

Per report from 1250kv.

Discussion: https://postgr.es/m/673825.1603223178@sss.pgh.pa.us
      c16a1bbc
    • Tom Lane's avatar
      Avoid premature de-doubling of quote marks in ECPG strings. · 3dfb1942
      Tom Lane authored 
      If you write the literal 'abc''def' in an EXEC SQL command, that will
come out the other end as 'abc'def', triggering a syntax error in the
backend.  Likewise, "abc""def" is reduced to "abc"def" which is wrong
syntax for a quoted identifier.

The cause is that the lexer thinks it should emit just one quote
mark, whereas what it really should do is keep the string as-is.

Add some docs and test cases, too.

Although this seems clearly a bug, I fear users wouldn't appreciate
changing it in minor releases.  Some may well be working around it
by applying an extra doubling of affected quotes, as for example
sql/dyntest.pgc has been doing.

Per investigation of a report from 1250kv, although this isn't
exactly what he/she was on about.

Discussion: https://postgr.es/m/673825.1603223178@sss.pgh.pa.us
      3dfb1942
    • Robert Haas's avatar
      Try to avoid a compiler warning about using fxid uninitialized. · 8bb0c977
      Robert Haas authored 
      Mark Dilger, with a couple of stray semicolons removed by me.

Discussion: http://postgr.es/m/2A7DA1A8-C4AA-43DF-A985-3CA52F4DC775@enterprisedb.com
      8bb0c977
    • Tom Lane's avatar
      Clean up some unpleasant behaviors in psql's \connect command. · 94929f1c
      Tom Lane authored 
      The check for whether to complain about not having an old connection
to get parameters from was seriously out of date: it had not been
rethought when we invented connstrings, nor when we invented the
-reuse-previous option.  Replace it with a check that throws an
error if reuse-previous is active and we lack an old connection to
reuse.  While that doesn't move the goalposts very far in terms of
easing reconnection after a server crash, at least it's consistent.

If the user specifies a connstring plus additional parameters
(which is invalid per the documentation), the extra parameters were
silently ignored.  That seems like it could be really confusing,
so let's throw a syntax error instead.

Teach the connstring code path to re-use the old connection's password
in the same cases as the old-style-syntax code path would, ie if we
are reusing parameters and the values of username, host/hostaddr, and
port are not being changed.  Document this behavior, too, since it was
unmentioned before.  Also simplify the implementation a bit, giving
rise to two new and useful properties: if there's a "password=xxx" in
the connstring, we'll use it not ignore it, and by default (i.e.,
except with --no-password) we will prompt for a password if the
re-used password or connstring password doesn't work.  The previous
code just failed if the re-used password didn't work.

Given the paucity of field complaints about these issues, I don't
think that they rise to the level of back-patchable bug fixes,
and in any case they might represent undesirable behavior changes
in minor releases.  So no back-patch.

Discussion: https://postgr.es/m/235210.1603321144@sss.pgh.pa.us
      94929f1c
    • Robert Haas's avatar
      Extend amcheck to check heap pages. · 866e24d4
      Robert Haas authored 
      Mark Dilger, reviewed by Peter Geoghegan, Andres Freund, Álvaro Herrera,
Michael Paquier, Amul Sul, and by me. Some last-minute cosmetic
revisions by me.

Discussion: http://postgr.es/m/12ED3DA8-25F0-4B68-937D-D907CFBF08E7@enterprisedb.com
      866e24d4
    • Peter Eisentraut's avatar
      Use croak instead of die in Perl code when appropriate · f8721bd7
      Peter Eisentraut authored
      f8721bd7
    • David Rowley's avatar
      Optimize a few list_delete_ptr calls · e7c2b95d
      David Rowley authored 
      There is a handful of places where we called list_delete_ptr() to remove
some element from a List.  In many of these places we know, or with very
little additional effort know the index of the ListCell that we need to
remove.

Here we change all of those places to instead either use one of;
list_delete_nth_cell(), foreach_delete_current() or list_delete_last().
Each of these saves from having to iterate over the list to search for the
element to remove by its pointer value.

There are some small performance gains to be had by doing this, but in the
general case, none of these lists are likely to be very large, so the
lookup was probably never that expensive anyway.  However, some of the
calls are in fairly hot code paths, e.g process_equivalence().  So any
small gains there are useful.

Author: Zhijie Hou and David Rowley
Discussion: https://postgr.es/m/b3517353ec7c4f87aa560678fbb1034b@G08CNEXMBPEKD05.g08.fujitsu.local
      e7c2b95d
  13. 21 Oct, 2020 6 commits
    • Tom Lane's avatar
      Fix connection string handling in psql's \connect command. · 85c54287
      Tom Lane authored 
      psql's \connect claims to be able to re-use previous connection
parameters, but in fact it only re-uses the database name, user name,
host name (and possibly hostaddr, depending on version), and port.
This is problematic for assorted use cases.  Notably, pg_dump[all]
emits "\connect databasename" commands which we would like to have
re-use all other parameters.  If such a script is loaded in a psql run
that initially had "-d connstring" with some non-default parameters,
those other parameters would be lost, potentially causing connection
failure.  (Thus, this is the same kind of bug addressed in commits
a45bc8a4 and 8e5793ab, although the details are much different.)

To fix, redesign do_connect() so that it pulls out all properties
of the old PGconn using PQconninfo(), and then replaces individual
properties in that array.  In the case where we don't wish to re-use
anything, get libpq's default settings using PQconndefaults() and
replace entries in that, so that we don't need different code paths
for the two cases.

This does result in an additional behavioral change for cases where
the original connection parameters allowed multiple hosts, say
"psql -h host1,host2", and the \connect request allows re-use of the
host setting.  Because the previous coding relied on PQhost(), it
would only permit reconnection to the same host originally selected.
Although one can think of scenarios where that's a good thing, there
are others where it is not.  Moreover, that behavior doesn't seem to
meet the principle of least surprise, nor was it documented; nor is
it even clear it was intended, since that coding long pre-dates the
addition of multi-host support to libpq.  Hence, this patch is content
to drop it and re-use the host list as given.

Per Peter Eisentraut's comments on bug #16604.  Back-patch to all
supported branches.

Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
      85c54287
    • Alvaro Herrera's avatar
      Use fast checkpoint in PostgresNode::backup() · 831611b1
      Alvaro Herrera authored 
      Should cause tests to be a bit faster
      831611b1
    • Tom Lane's avatar
      Remove the option to build thread_test.c outside configure. · 8a212118
      Tom Lane authored 
      Theoretically one could go into src/test/thread and build/run this
program there.  In practice, that hasn't worked since 96bf88d5,
and probably much longer on some platforms (likely including just
the sort of hoary leftovers where this test might be of interest).
While it wouldn't be too hard to repair the breakage, the fact that
nobody has noticed for two years shows that there is zero usefulness
in maintaining this build pathway.  Let's get rid of it and decree
that thread_test.c is *only* meant to be built/used in configure.

Given that decision, it makes sense to put thread_test.c under config/
and get rid of src/test/thread altogether, so that's what I did.

In passing, update src/test/README, which had been ignored by some
not-so-recent additions of subdirectories.

Discussion: https://postgr.es/m/227659.1603041612@sss.pgh.pa.us
      8a212118
    • Peter Eisentraut's avatar
      Remove obsolete ifdefs · 555eb1a4
      Peter Eisentraut authored 
      Commit 8dace66e added #ifdefs for a
number of errno symbols because they were not present on Windows.
Later, commit 125ad539 added
replacement #defines for some of those symbols.  So some of the
changes from the first commit are made dead code by the second commit
and can now be removed.

Discussion: https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0033@2ndquadrant.com
      555eb1a4
    • Peter Eisentraut's avatar
      Fix -Wcast-function-type warnings on Windows/MinGW · 8a58347a
      Peter Eisentraut authored 
      After de8feb1f, some warnings remained
that were only visible when using GCC on Windows.  Fix those as well.

Note that the ecpg test source files don't use the full pg_config.h,
so we can't use pg_funcptr_t there but have to do it the long way.
      8a58347a
    • Michael Paquier's avatar
      Review format of code generated by PerfectHash.pm · 19ae53c9
      Michael Paquier authored 
      80f8eb79 has added to the normalization quick check headers some code
generated by PerfectHash.pm that is incompatible with the settings of
gitattributes for this repository, as whitespaces followed a set of tabs
for the first element of a line in the table.  Instead of adding a new
exception to gitattributes, rework the format generated so as a right
padding with spaces is used instead of a left padding.  This keeps the
table generated in a readable shape with its set of columns, making
unnecessary an update of gitattributes.

Reported-by: Peter Eisentraut
Author: John Naylor
Discussion: https://postgr.es/m/d601b3b5-a3c7-5457-2f84-3d6513d690fc@2ndquadrant.com
      19ae53c9
  15. 20 Oct, 2020 2 commits
  17. 19 Oct, 2020 1 commit
    • Tom Lane's avatar
      Fix connection string handling in src/bin/scripts/ programs. · 8e5793ab
      Tom Lane authored 
      When told to process all databases, clusterdb, reindexdb, and vacuumdb
would reconnect by replacing their --maintenance-db parameter with the
name of the target database.  If that parameter is a connstring (which
has been allowed for a long time, though we failed to document that
before this patch), we'd lose any other options it might specify, for
example SSL or GSS parameters, possibly resulting in failure to connect.
Thus, this is the same bug as commit a45bc8a4 fixed in pg_dump and
pg_restore.  We can fix it in the same way, by using libpq's rules for
handling multiple "dbname" parameters to add the target database name
separately.  I chose to apply the same refactoring approach as in that
patch, with a struct to handle the command line parameters that need to
be passed through to connectDatabase.  (Maybe someday we can unify the
very similar functions here and in pg_dump/pg_restore.)

Per Peter Eisentraut's comments on bug #16604.  Back-patch to all
supported branches.

Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
      8e5793ab