1. 18 Mar, 2020 7 commits
  2. 17 Mar, 2020 12 commits
    • Tom Lane's avatar
      Refactor our checks for valid function and aggregate signatures. · e6c178b5
      Tom Lane authored
      pg_proc.c and pg_aggregate.c had near-duplicate copies of the logic
      to decide whether a function or aggregate's signature is legal.
      This seems like a bad thing even without the problem that the
      upcoming "anycompatible" patch would roughly double the complexity
      of that logic.  Hence, refactor so that the rules are localized
      in new subroutines supplied by parse_coerce.c.  (One could quibble
      about just where to add that code, but putting it beside
      enforce_generic_type_consistency seems not totally unreasonable.)
      
      The fact that the rules are about to change would mandate some
      changes in the wording of the associated error messages in any case.
      I ended up spelling things out in a fairly literal fashion in the
      errdetail messages, eg "A result of type anyelement requires at
      least one input of type anyelement, anyarray, anynonarray, anyenum,
      or anyrange."  Perhaps this is overkill, but once there's more than
      one subgroup of polymorphic types, people might get confused by
      more-abstract messages.
      
      Discussion: https://postgr.es/m/24137.1584139352@sss.pgh.pa.us
      e6c178b5
    • Peter Geoghegan's avatar
      Doc: Correct deduplicate_items varlistentry id. · dbbb5538
      Peter Geoghegan authored
      Use a varlistentry id for the deduplicate_items storage parameter that
      is derived from the name of the parameter itself.
      
      This oversight happened because the storage parameter was renamed
      relatively late during the development of the patch that became commit
      0d861bbb.
      dbbb5538
    • Tom Lane's avatar
      Adjust handling of an ANYARRAY actual input for an ANYARRAY argument. · 77ec5aff
      Tom Lane authored
      Ordinarily it's impossible for an actual input of a function to have
      declared type ANYARRAY, since we'd resolve that to a concrete array
      type before doing argument type resolution for the function.  But an
      exception arises for functions applied to certain columns of pg_statistic
      or pg_stats, since we abuse the "anyarray" pseudotype by using it to
      declare those columns.  So parse_coerce.c has to deal with the case.
      
      Previously we allowed an ANYARRAY actual input to match an ANYARRAY
      polymorphic argument, but only if no other argument or result was
      declared ANYELEMENT.  When that logic was written, those were the only
      two polymorphic types, and I fear nobody thought carefully about how it
      ought to extend to the other ones that came along later.  But actually
      it was wrong even then, because if a function has two ANYARRAY
      arguments, it should be able to expect that they have identical element
      types, and we'd not be able to ensure that.
      
      The correct generalization is that we can match an ANYARRAY actual input
      to an ANYARRAY polymorphic argument only if no other argument or result
      is of any polymorphic type, so that no promises are being made about
      element type compatibility.  check_generic_type_consistency can't test
      that condition, but it seems better anyway to accept such matches there
      and then throw an error if needed in enforce_generic_type_consistency.
      That way we can produce a specific error message rather than an
      unintuitive "function does not exist" complaint.  (There's some risk
      perhaps of getting new ambiguous-function complaints, but I think that
      any set of functions for which that could happen would be ambiguous
      against ordinary array columns as well.)  While we're at it, we can
      improve the message that's produced in cases that the code did already
      object to, as shown in the regression test changes.
      
      Also, remove a similar test that got cargo-culted in for ANYRANGE;
      there are no catalog columns of type ANYRANGE, and I hope we never
      create any, so that's not needed.  (It was incomplete anyway.)
      
      While here, update some comments and rearrange the code a bit in
      preparation for upcoming additions of more polymorphic types.
      
      In practical situations I believe this is just exchanging one error
      message for another, hopefully better, one.  So it doesn't seem
      needful to back-patch, even though the mistake is ancient.
      
      Discussion: https://postgr.es/m/21569.1584314271@sss.pgh.pa.us
      77ec5aff
    • Alvaro Herrera's avatar
      Remove logical_read_local_xlog_page · 5d0c2d5e
      Alvaro Herrera authored
      It devolved into a content-less wrapper over read_local_xlog_page, with
      nothing to add, plus it's easily confused with walsender's
      logical_read_xlog_page.  There doesn't seem to be any reason for it to
      stay.
      
      src/include/replication/logicalfuncs.h becomes empty, so remove it too.
      The prototypes it initially had were absorbed by generated fmgrprotos.h.
      
      Discussion: https://postgr.es/m/20191115214102.GA15616@alvherre.pgsql
      5d0c2d5e
    • Alvaro Herrera's avatar
      Fix consistency issues with replication slot copy · bcd1c363
      Alvaro Herrera authored
      Commit 9f06d79e's replication slot copying failed to
      properly reserve the WAL that the slot is expecting to see
      during DecodingContextFindStartpoint (to set the confirmed_flush
      LSN), so concurrent activity could remove that WAL and cause the
      copy process to error out.  But it doesn't actually *need* that
      WAL anyway: instead of running decode to find confirmed_flush, it
      can be copied from the source slot. Fix this by rearranging things
      to avoid DecodingContextFindStartpoint() (leaving the target slot's
      confirmed_flush_lsn to invalid), and set that up afterwards by copying
      from the target slot's value.
      
      Also ensure the source slot's confirmed_flush_lsn is valid.
      
      Reported-by: Arseny Sher
      Author: Masahiko Sawada, Arseny Sher
      Discussion: https://postgr.es/m/871rr3ohbo.fsf@ars-thinkpad
      bcd1c363
    • Tom Lane's avatar
      Doc: clarify behavior of "anyrange" pseudo-type. · 31d846e0
      Tom Lane authored
      I noticed that we completely failed to document the restriction
      that an "anyrange" result type has to be inferred from an "anyrange"
      input.  The docs also were less clear than they could be about the
      relationship between "anyrange" and "anyarray".
      
      It's been like this all along, so back-patch.
      31d846e0
    • Tom Lane's avatar
      Remove bogus assertion about polymorphic SQL function result. · 9d9784c8
      Tom Lane authored
      It is possible to reach check_sql_fn_retval() with an unresolved
      polymorphic rettype, resulting in an assertion failure as demonstrated
      by one of the added test cases.  However, the code following that
      throws what seems an acceptable error message, so just remove the
      Assert and adjust commentary.
      
      While here, I thought it'd be a good idea to provide some parallel
      tests of SQL-function and PL/pgSQL-function polymorphism behavior.
      Some of these cases are perhaps duplicative of tests elsewhere,
      but we hadn't any organized coverage of the topic AFAICS.
      
      Although that assertion's been wrong all along, it won't have any
      effect in production builds, so I'm not bothering to back-patch.
      
      Discussion: https://postgr.es/m/21569.1584314271@sss.pgh.pa.us
      9d9784c8
    • Tom Lane's avatar
      Use pkg-config, if available, to locate libxml2 during configure. · 0bc8cebd
      Tom Lane authored
      If pkg-config is installed and knows about libxml2, use its information
      rather than asking xml2-config.  Otherwise proceed as before.  This
      patch allows "configure --with-libxml" to succeed on platforms that
      have pkg-config but not xml2-config, which is likely to soon become
      a typical situation.
      
      The old mechanism can be forced by setting XML2_CONFIG explicitly
      (hence, build processes that were already doing so will certainly
      not need adjustment).  Also, it's now possible to set XML2_CFLAGS
      and XML2_LIBS explicitly to override both programs.
      
      There is a small risk of this breaking existing build processes,
      if there are multiple libxml2 installations on the machine and
      pkg-config disagrees with xml2-config about which to use.  The
      only case where that seems really likely is if a builder has tried
      to select a non-default xml2-config by putting it early in his PATH
      rather than setting XML2_CONFIG.  Plan to warn against that in the
      minor release notes.
      
      Back-patch to v10; before that we had no pkg-config infrastructure,
      and it doesn't seem worth adding it for this.
      
      Hugh McMaster and Tom Lane; Peter Eisentraut also made an earlier
      attempt at this, from which I lifted most of the docs changes.
      
      Discussion: https://postgr.es/m/CAN9BcdvfUwc9Yx5015bLH2TOiQ-M+t_NADBSPhMF7dZ=pLa_iw@mail.gmail.com
      0bc8cebd
    • Fujii Masao's avatar
    • Fujii Masao's avatar
      Fix comment in xlog.c. · 1429d3f7
      Fujii Masao authored
      This commit fixes the comment about SharedHotStandbyActive variable.
      The comment was apparently copy-and-pasted.
      
      Author: Atsushi Torikoshi
      Discussion: https://postgr.es/m/CACZ0uYEjpqZB9wN2Rwc_RMvDybyYqdbkPuDr1NyxJg4f9yGfMw@mail.gmail.com
      1429d3f7
    • Tom Lane's avatar
      Remove useless pfree()s at the ends of various ValuePerCall SRFs. · 41b45576
      Tom Lane authored
      We don't need to manually clean up allocations in a SRF's
      multi_call_memory_ctx, because the SRF_RETURN_DONE infrastructure
      takes care of that (and also ensures that it will happen even if the
      function never gets a final call, which simple manual cleanup cannot
      do).
      
      Hence, the code removed by this patch is a waste of code and cycles.
      Worse, it gives the impression that cleaning up manually is a thing,
      which can lead to more serious errors such as those fixed in
      commits 085b6b66 and b4570d33.  So we should get rid of it.
      
      These are not quite actual bugs though, so I couldn't muster the
      enthusiasm to back-patch.  Fix in HEAD only.
      
      Justin Pryzby
      
      Discussion: https://postgr.es/m/20200308173103.GC1357@telsasoft.com
      41b45576
    • Tom Lane's avatar
      Avoid holding a directory FD open across assorted SRF calls. · b4570d33
      Tom Lane authored
      This extends the fixes made in commit 085b6b66 to other SRFs with the
      same bug, namely pg_logdir_ls(), pgrowlocks(), pg_timezone_names(),
      pg_ls_dir(), and pg_tablespace_databases().
      
      Also adjust various comments and documentation to warn against
      expecting to clean up resources during a ValuePerCall SRF's final
      call.
      
      Back-patch to all supported branches, since these functions were
      all born broken.
      
      Justin Pryzby, with cosmetic tweaks by me
      
      Discussion: https://postgr.es/m/20200308173103.GC1357@telsasoft.com
      b4570d33
  3. 16 Mar, 2020 8 commits
  4. 15 Mar, 2020 1 commit
  5. 14 Mar, 2020 6 commits
    • Tomas Vondra's avatar
      Improve test coverage for multi-column MCV lists · d8cfa82d
      Tomas Vondra authored
      The regression tests for extended statistics were not testing a couple
      of important cases for the MCV lists:
      
        * IS NOT NULL clauses - We did have queries with IS NULL clauses, but
          not the negative case.
      
        * clauses with variable on the right - All the clauses had the Var on
          the left, i.e. (Var op Const), so this adds (Const op Var) too.
      
        * columns with fixed-length types passed by reference - All columns
          were using either by-value or varlena types, so add a test with
          UUID columns too. This matters for (de)serialization.
      
        * NULL-only dimension - When one of the columns contains only NULL
          values, we treat it a a special case during (de)serialization.
      
        * arrays containing NULL - When the constant parameter contains NULL
          value, we need to handle it correctly during estimation, for all
          IN, ANY and ALL clauses.
      
      Discussion: https://www.postgresql.org/message-id/flat/20200113230008.g67iyk4cs3xbnjju@development
      Author: Tomas Vondra
      d8cfa82d
    • Tomas Vondra's avatar
      Improve test coverage for functional dependencies · f9696782
      Tomas Vondra authored
      The regression tests for functional dependencies were only using clauses
      of the form (Var op Const), i.e. with Var on the left side. This adds
      a couple of queries with Var on the right, to test other code paths.
      
      It also prints one of the functional dependencies, to test the data type
      output function. The functional dependencies are "perfect" with degree
      of 1.0 so this should be stable.
      
      Discussion: https://www.postgresql.org/message-id/flat/20200113230008.g67iyk4cs3xbnjju@development
      Author: Tomas Vondra
      f9696782
    • Tom Lane's avatar
      Rearrange pseudotypes.c to get rid of duplicative code. · 87c9c257
      Tom Lane authored
      Commit a5954de1 replaced a lot of manually-coded stub I/O routines
      with code generated by macros.  That was a good idea but it didn't
      go far enough, because there were still manually-coded stub input
      routines for types that had live output routines.  Refactor the
      macro so that we can generate just a stub input routine at need.
      
      Also create similar macros to generate stub binary I/O routines,
      since we have some of those now.  The only stub functions that remain
      hand-coded are shell_in() and shell_out(), which need to be separate
      because they use different error messages.
      
      While here, rearrange the commentary to discuss each type not each
      function.  This provides a better way to explain the *why* of which
      types need which support, rather than just duplicatively annotating
      the functions.
      
      Discussion: https://postgr.es/m/24137.1584139352@sss.pgh.pa.us
      87c9c257
    • Tom Lane's avatar
      Restructure polymorphic-type resolution in funcapi.c. · 4dbcb3f8
      Tom Lane authored
      resolve_polymorphic_tupdesc() and resolve_polymorphic_argtypes() failed to
      cover the case of having to resolve anyarray given only an anyrange input.
      The bug was masked if anyelement was also used (as either input or
      output), which probably helps account for our not having noticed.
      
      While looking at this I noticed that resolve_generic_type() would produce
      the wrong answer if asked to make that same resolution.  ISTM that
      resolve_generic_type() is confusingly defined and overly complex, so
      rather than fix it, let's just make funcapi.c do the specific lookups
      it requires for itself.
      
      With this change, resolve_generic_type() is not used anywhere, so remove
      it in HEAD.  In the back branches, leave it alone (complete with bug)
      just in case any external code is using it.
      
      While we're here, make some other refactoring adjustments in funcapi.c
      with an eye to upcoming future expansion of the set of polymorphic types:
      
      * Simplify quick-exit tests by adding an overall have_polymorphic_result
      flag.  This is about a wash now but will be a win when there are more
      flags.
      
      * Reduce duplication of code between resolve_polymorphic_tupdesc() and
      resolve_polymorphic_argtypes().
      
      * Don't bother to validate correct matching of anynonarray or anyenum;
      the parser should have done that, and even if it didn't, just doing
      "return false" here would lead to a very confusing, off-point error
      message.  (Really, "return false" in these two functions should only
      occur if the call_expr isn't supplied or we can't obtain data type
      info from it.)
      
      * For the same reason, throw an elog rather than "return false" if
      we fail to resolve a polymorphic type.
      
      The bug's been there since we added anyrange, so back-patch to
      all supported branches.
      
      Discussion: https://postgr.es/m/6093.1584202130@sss.pgh.pa.us
      4dbcb3f8
    • Tomas Vondra's avatar
      Use multi-variate MCV lists to estimate ScalarArrayOpExpr · e83daa7e
      Tomas Vondra authored
      Commit 8f321bd1 added support for estimating ScalarArrayOpExpr clauses
      (IN/ANY) clauses using functional dependencies. There's no good reason
      not to support estimation of these clauses using multi-variate MCV lists
      too, so this commits implements that. That makes the behavior consistent
      and MCV lists can estimate all variants (ANY/ALL, inequalities, ...).
      
      Author: Tomas Vondra
      Review: Dean Rasheed
      Discussion: https://www.postgresql.org/message-id/flat/13902317.Eha0YfKkKy%40pierred-pdoc
      e83daa7e
    • Tomas Vondra's avatar
      Use functional dependencies to estimate ScalarArrayOpExpr · 8f321bd1
      Tomas Vondra authored
      Until now functional dependencies supported only simple equality clauses
      and clauses that can be trivially translated to equalities. This commit
      allows estimation of some ScalarArrayOpExpr (IN/ANY) clauses.
      
      For IN clauses we can do this thanks to using operator with equality
      semantics, which means an IN clause
      
          WHERE c IN (1, 2, ..., N)
      
      can be translated to
      
          WHERE (c = 1 OR c = 2 OR ... OR c = N)
      
      IN clauses are now considered compatible with functional dependencies,
      and rely on the same assumption of consistency of queries with data
      (which is an assumption we already used for simple equality clauses).
      This applies also to ALL clauses with an equality operator, which can be
      considered equivalent to IN clause.
      
      ALL clauses are still considered incompatible, although there's some
      discussion about maybe relaxing this in the future.
      
      Author: Pierre Ducroquet
      Reviewed-by: Tomas Vondra, Dean Rasheed
      Discussion: https://www.postgresql.org/message-id/flat/13902317.Eha0YfKkKy%40pierred-pdoc
      8f321bd1
  6. 13 Mar, 2020 6 commits