1. 26 Oct, 2020 3 commits
  2. 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
  3. 24 Oct, 2020 3 commits
  4. 23 Oct, 2020 10 commits
  5. 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
    • 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
  6. 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
  7. 20 Oct, 2020 2 commits
  8. 19 Oct, 2020 7 commits
    • 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
    • Tom Lane's avatar
      Fix list-munging bug that broke SQL function result coercions. · c8ab9701
      Tom Lane authored
      Since commit 913bbd88, check_sql_fn_retval() can either insert type
      coercion steps in-line in the Query that produces the SQL function's
      results, or generate a new top-level Query to perform the coercions,
      if modifying the Query's output in-place wouldn't be safe.  However,
      it appears that the latter case has never actually worked, because
      the code tried to inject the new Query back into the query list it was
      passed ... which is not the list that will be used for later processing
      when we execute the SQL function "normally" (without inlining it).
      So we ended up with no coercion happening at run-time, leading to
      wrong results or crashes depending on the datatypes involved.
      
      While the regression tests look like they cover this area well enough,
      through a huge bit of bad luck all the test cases that exercise the
      separate-Query path were checking either inline-able cases (which
      accidentally didn't have the bug) or cases that are no-ops at runtime
      (e.g., varchar to text), so that the failure to perform the coercion
      wasn't obvious.  The fact that the cases that don't work weren't
      allowed at all before v13 probably contributed to not noticing the
      problem sooner, too.
      
      To fix, get rid of the separate "flat" list of Query nodes and instead
      pass the real two-level list that is going to be used later.  I chose
      to make the same change in check_sql_fn_statements(), although that has
      no actual bug, just so that we don't need that data structure at all.
      
      This is an API change, as evidenced by the adjustments needed to
      callers outside functions.c.  That's a bit scary to be doing in a
      released branch, but so far as I can tell from a quick search,
      there are no outside callers of these functions (and they are
      sufficiently specific to our semantics for SQL-language functions that
      it's not apparent why any extension would need to call them).  In any
      case, v13 already changed the API of check_sql_fn_retval() compared to
      prior branches.
      
      Per report from pinker.  Back-patch to v13 where this code came in.
      
      Discussion: https://postgr.es/m/1603050466566-0.post@n3.nabble.com
      c8ab9701
    • Heikki Linnakangas's avatar
      Misc documentation fixes. · c5f42daa
      Heikki Linnakangas authored
      - Misc grammar and punctuation fixes.
      
      - Stylistic cleanup: use spaces between function arguments and JSON fields
        in examples. For example "foo(a,b)" -> "foo(a, b)". Add semicolon after
        last END in a few PL/pgSQL examples that were missing them.
      
      - Make sentence that talked about "..." and ".." operators more clear,
        by avoiding to end the sentence with "..". That makes it look the same
        as "..."
      
      - Fix syntax description for HAVING: HAVING conditions cannot be repeated
      
      Patch by Justin Pryzby, per Yaroslav Schekin's report. Backpatch to all
      supported versions, to the extent that the patch applies easily.
      
      Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com
      c5f42daa
    • Heikki Linnakangas's avatar
      Fix TRUNCATE doc: ALTER SEQUENCE RESTART is now transactional. · 2a972e01
      Heikki Linnakangas authored
      ALTER SEQUENCE RESTART was made transactional in commit 3d79013b.
      Backpatch to v10, where that was introduced.
      
      Patch by Justin Pryzby, per Yaroslav Schekin's report.
      
      Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com
      2a972e01
    • Heikki Linnakangas's avatar
      Fix output of tsquery example in docs. · c0bc4c68
      Heikki Linnakangas authored
      The output for this query changed in commit 4e2477b7b8. Backport to 9.6
      like that commit.
      
      Patch by Justin Pryzby, per Yaroslav Schekin's report.
      
      Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com
      c0bc4c68
    • Heikki Linnakangas's avatar
      Fix doc for full text search distance operator. · 1a64c763
      Heikki Linnakangas authored
      Commit 028350f6 changed its behavior from "at most" to "exactly", but
      forgot to update the documentation. Backpatch to 9.6.
      
      Patch by Justin Pryzby, per Yaroslav Schekin's report.
      
      Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com
      1a64c763
    • Magnus Hagander's avatar
      Update link for pllua · b4d5b458
      Magnus Hagander authored
      Author: Daniel Gustafsson <daniel@yesql.se>
      Discussion: https://postgr.es/m/A05874AE-8771-4C61-A24E-0B6249B8F3C2@yesql.se
      b4d5b458