1. 18 Jan, 2017 14 commits
    • Andres Freund's avatar
      Move targetlist SRF handling from expression evaluation to new executor node. · 69f4b9c8
      Andres Freund authored
      Evaluation of set returning functions (SRFs_ in the targetlist (like SELECT
      generate_series(1,5)) so far was done in the expression evaluation (i.e.
      ExecEvalExpr()) and projection (i.e. ExecProject/ExecTargetList) code.
      
      This meant that most executor nodes performing projection, and most
      expression evaluation functions, had to deal with the possibility that an
      evaluated expression could return a set of return values.
      
      That's bad because it leads to repeated code in a lot of places. It also,
      and that's my (Andres's) motivation, made it a lot harder to implement a
      more efficient way of doing expression evaluation.
      
      To fix this, introduce a new executor node (ProjectSet) that can evaluate
      targetlists containing one or more SRFs. To avoid the complexity of the old
      way of handling nested expressions returning sets (e.g. having to pass up
      ExprDoneCond, and dealing with arguments to functions returning sets etc.),
      those SRFs can only be at the top level of the node's targetlist.  The
      planner makes sure (via split_pathtarget_at_srfs()) that SRF evaluation is
      only necessary in ProjectSet nodes and that SRFs are only present at the
      top level of the node's targetlist. If there are nested SRFs the planner
      creates multiple stacked ProjectSet nodes.  The ProjectSet nodes always get
      input from an underlying node.
      
      We also discussed and prototyped evaluating targetlist SRFs using ROWS
      FROM(), but that turned out to be more complicated than we'd hoped.
      
      While moving SRF evaluation to ProjectSet would allow to retain the old
      "least common multiple" behavior when multiple SRFs are present in one
      targetlist (i.e.  continue returning rows until all SRFs are at the end of
      their input at the same time), we decided to instead only return rows till
      all SRFs are exhausted, returning NULL for already exhausted ones.  We
      deemed the previous behavior to be too confusing, unexpected and actually
      not particularly useful.
      
      As a side effect, the previously prohibited case of multiple set returning
      arguments to a function, is now allowed. Not because it's particularly
      desirable, but because it ends up working and there seems to be no argument
      for adding code to prohibit it.
      
      Currently the behavior for COALESCE and CASE containing SRFs has changed,
      returning multiple rows from the expression, even when the SRF containing
      "arm" of the expression is not evaluated. That's because the SRFs are
      evaluated in a separate ProjectSet node.  As that's quite confusing, we're
      likely to instead prohibit SRFs in those places.  But that's still being
      discussed, and the code would reside in places not touched here, so that's
      a task for later.
      
      There's a lot of, now superfluous, code dealing with set return expressions
      around. But as the changes to get rid of those are verbose largely boring,
      it seems better for readability to keep the cleanup as a separate commit.
      
      Author: Tom Lane and Andres Freund
      Discussion: https://postgr.es/m/20160822214023.aaxz5l4igypowyri@alap3.anarazel.de
      69f4b9c8
    • Robert Haas's avatar
      Improve comment in hashsearch.c. · e37360d5
      Robert Haas authored
      Typo fix from Mithun Cy; other improvements by me.
      e37360d5
    • Tom Lane's avatar
      Reset the proper GUC in create_index test. · 1586317c
      Tom Lane authored
      Thinko in commit a4523c5a.  It doesn't really affect anything at
      present, but it would be a problem if any tests added later in this
      file ought to get index-only-scan plans.  Back-patch, like the previous
      commit, just to avoid surprises in case we add such a test and then
      back-patch it.
      
      Nikita Glukhov
      
      Discussion: https://postgr.es/m/8b70135d-ad38-bdd8-ac92-71e2b3c273cf@postgrespro.ru
      1586317c
    • Alvaro Herrera's avatar
      Change some test macros to return true booleans · 594e61a1
      Alvaro Herrera authored
      These macros work fine when they are used directly in an "if" test or
      similar, but as soon as the return values are assigned to boolean
      variables (or passed as boolean arguments to some function), they become
      bugs, hopefully caught by compiler warnings.  To avoid future problems,
      fix the definitions so that they return actual booleans.
      
      To further minimize the risk that somebody uses them in back-patched
      fixes that only work correctly in branches starting from the current
      master and not in old ones, back-patch the change to supported branches
      as appropriate.
      
      See also commit af4472bc, and the long
      discussion (and larger patch) in the thread mentioned in its commit
      message.
      
      Discussion: https://postgr.es/m/18672.1483022414@sss.pgh.pa.us
      594e61a1
    • Magnus Hagander's avatar
      Implement array version of jsonb_delete and operator · d00ca333
      Magnus Hagander authored
      This makes it possible to delete multiple keys from a jsonb value by
      passing in an array of text values, which makes the operaiton much
      faster than individually deleting the keys (which would require copying
      the jsonb structure over and over again.
      
      Reviewed by Dmitry Dolgov and Michael Paquier
      d00ca333
    • Tom Lane's avatar
      Disable transforms that replaced AT TIME ZONE with RelabelType. · c22ecc65
      Tom Lane authored
      These resulted in wrong answers if the relabeled argument could be matched
      to an index column, as shown in bug #14504 from Evgeniy Kozlov.  We might
      be able to resurrect these optimizations by adjusting the planner's
      treatment of RelabelType, or by adjusting btree's rules for selecting
      comparison functions, but either solution will take careful analysis
      and does not sound like a fit candidate for backpatching.
      
      I left the catalog infrastructure in place and just reduced the transform
      functions to always-return-NULL.  This would be necessary anyway in the
      back branches, and it doesn't seem important to be more invasive in HEAD.
      
      Bug introduced by commit b8a18ad4.  Back-patch to 9.5 where that came in.
      
      Report: https://postgr.es/m/20170118144828.1432.52823@wrigleys.postgresql.org
      Discussion: https://postgr.es/m/18771.1484759439@sss.pgh.pa.us
      c22ecc65
    • Robert Haas's avatar
      Avoid use of DROP TABLE .. CASCADE in partitioning tests. · e509e7f9
      Robert Haas authored
      This isn't really guaranteed to always produce exactly the same
      output; the order can change from run to run.
      
      See related cleanup in 257d8157.
      e509e7f9
    • Robert Haas's avatar
      Add some more tests for tuple routing. · d26fa4fd
      Robert Haas authored
      Commit a2566508 fixed some issues with
      how PartitionDispatch related code handled multi-level partitioned
      tables, but didn't add any tests.
      
      Discussion: http://postgr.es/m/CA%2BTgmoZ86v1G%2Bzx9etMiSQaBBvYMKfU-iitqZArSh5z0n8Q4cA%40mail.gmail.com
      
      Amit Langote, per a complaint from me.
      d26fa4fd
    • Robert Haas's avatar
      Update information_schema queries and system views for new relkind. · 262e821d
      Robert Haas authored
      The original table partitioning patch overlooked this.
      
      Discussion: http://postgr.es/m/CAG1_KcDJiZB=L6yOUO_bVufj2q2851_xdkfhw0JdcD_2VtKssw@mail.gmail.com
      
      Keith Fiske and Amit Langote, adjusted by me.
      262e821d
    • Alvaro Herrera's avatar
      Make messages mentioning type names more uniform · 9a34123b
      Alvaro Herrera authored
      This avoids additional translatable strings for each distinct type, as
      well as making our quoting style around type names more consistent
      (namely, that we don't quote type names).  This continues what started
      as f402b995.
      
      Discussion: https://postgr.es/m/20160401170642.GA57509@alvherre.pgsql
      9a34123b
    • Robert Haas's avatar
      Factor out logic for computing number of parallel workers. · 716c7d4b
      Robert Haas authored
      Forthcoming patches to allow other types of parallel scans will
      need this logic, or something like it.
      
      Dilip Kumar
      716c7d4b
    • Tom Lane's avatar
      Avoid conflicts with collation aliases generated by stripping. · 0333a734
      Tom Lane authored
      This resulted in failures depending on the order of "locale -a" output.
      The original coding in initdb sorted the results, but that should be
      unnecessary as long as "locale -a" doesn't print duplicate names.  The
      original entries will then all be non-dups, and while we might generate
      duplicate aliases by stripping, they should be for different encodings and
      thus not conflict.  Even if the latter assumption fails somehow, it won't
      be fatal because we're using if_not_exists mode for the aliases.
      
      Discussion: https://postgr.es/m/26116.1484751196%40sss.pgh.pa.us
      0333a734
    • Tom Lane's avatar
      Improve RLS planning by marking individual quals with security levels. · 215b43cd
      Tom Lane authored
      In an RLS query, we must ensure that security filter quals are evaluated
      before ordinary query quals, in case the latter contain "leaky" functions
      that could expose the contents of sensitive rows.  The original
      implementation of RLS planning ensured this by pushing the scan of a
      secured table into a sub-query that it marked as a security-barrier view.
      Unfortunately this results in very inefficient plans in many cases, because
      the sub-query cannot be flattened and gets planned independently of the
      rest of the query.
      
      To fix, drop the use of sub-queries to enforce RLS qual order, and instead
      mark each qual (RestrictInfo) with a security_level field establishing its
      priority for evaluation.  Quals must be evaluated in security_level order,
      except that "leakproof" quals can be allowed to go ahead of quals of lower
      security_level, if it's helpful to do so.  This has to be enforced within
      the ordering of any one list of quals to be evaluated at a table scan node,
      and we also have to ensure that quals are not chosen for early evaluation
      (i.e., use as an index qual or TID scan qual) if they're not allowed to go
      ahead of other quals at the scan node.
      
      This is sufficient to fix the problem for RLS quals, since we only support
      RLS policies on simple tables and thus RLS quals will always exist at the
      table scan level only.  Eventually these qual ordering rules should be
      enforced for join quals as well, which would permit improving planning for
      explicit security-barrier views; but that's a task for another patch.
      
      Note that FDWs would need to be aware of these rules --- and not, for
      example, send an insecure qual for remote execution --- but since we do
      not yet allow RLS policies on foreign tables, the case doesn't arise.
      This will need to be addressed before we can allow such policies.
      
      Patch by me, reviewed by Stephen Frost and Dean Rasheed.
      
      Discussion: https://postgr.es/m/8185.1477432701@sss.pgh.pa.us
      215b43cd
    • Peter Eisentraut's avatar
      Add function to import operating system collations · aa17c06f
      Peter Eisentraut authored
      Move this logic out of initdb into a user-callable function.  This
      simplifies the code and makes it possible to update the standard
      collations later on if additional operating system collations appear.
      Reviewed-by: default avatarAndres Freund <andres@anarazel.de>
      Reviewed-by: default avatarEuler Taveira <euler@timbira.com.br>
      aa17c06f
  2. 17 Jan, 2017 13 commits
  3. 16 Jan, 2017 5 commits
  4. 15 Jan, 2017 1 commit
    • Tom Lane's avatar
      Fix matching of boolean index columns to sort ordering. · 0777f7a2
      Tom Lane authored
      Normally, if we have a WHERE clause like "indexcol = constant",
      the planner will figure out that that index column can be ignored
      when determining whether the index has a desired sort ordering.
      But this failed to work for boolean index columns, because a
      condition like "boolcol = true" is canonicalized to just "boolcol"
      which does not give rise to an EquivalenceClass.  Add a check to
      allow the same type of deduction to be made in this case too.
      
      Per a complaint from Dima Pavlov.  Arguably this is a bug, but given the
      limited impact and the small number of complaints so far, I won't risk
      destabilizing plans in stable branches by back-patching.
      
      Patch by me, reviewed by Michael Paquier
      
      Discussion: https://postgr.es/m/1788.1481605684@sss.pgh.pa.us
      0777f7a2
  5. 14 Jan, 2017 6 commits
    • Tom Lane's avatar
      Teach contrib/pg_stat_statements to handle multi-statement commands better. · 83f2061d
      Tom Lane authored
      Make use of the statement boundary info added by commit ab1f0c82
      to let pg_stat_statements behave more sanely when multiple SQL queries
      are jammed into one query string.  It now records just the relevant
      part of the source string, not the whole thing, for each individual
      query.
      
      Even when no multi-statement strings are involved, users may notice small
      changes in the output: leading and trailing whitespace and semicolons will
      be stripped from statements, which did not happen before.
      
      Also, significantly expand pg_stat_statements' regression test script.
      
      Fabien Coelho, reviewed by Craig Ringer and Kyotaro Horiguchi,
      some mods by me
      
      Discussion: https://postgr.es/m/alpine.DEB.2.20.1612200926310.29821@lancre
      83f2061d
    • Tom Lane's avatar
      Change representation of statement lists, and add statement location info. · ab1f0c82
      Tom Lane authored
      This patch makes several changes that improve the consistency of
      representation of lists of statements.  It's always been the case
      that the output of parse analysis is a list of Query nodes, whatever
      the types of the individual statements in the list.  This patch brings
      similar consistency to the outputs of raw parsing and planning steps:
      
      * The output of raw parsing is now always a list of RawStmt nodes;
      the statement-type-dependent nodes are one level down from that.
      
      * The output of pg_plan_queries() is now always a list of PlannedStmt
      nodes, even for utility statements.  In the case of a utility statement,
      "planning" just consists of wrapping a CMD_UTILITY PlannedStmt around
      the utility node.  This list representation is now used in Portal and
      CachedPlan plan lists, replacing the former convention of intermixing
      PlannedStmts with bare utility-statement nodes.
      
      Now, every list of statements has a consistent head-node type depending
      on how far along it is in processing.  This allows changing many places
      that formerly used generic "Node *" pointers to use a more specific
      pointer type, thus reducing the number of IsA() tests and casts needed,
      as well as improving code clarity.
      
      Also, the post-parse-analysis representation of DECLARE CURSOR is changed
      so that it looks more like EXPLAIN, PREPARE, etc.  That is, the contained
      SELECT remains a child of the DeclareCursorStmt rather than getting flipped
      around to be the other way.  It's now true for both Query and PlannedStmt
      that utilityStmt is non-null if and only if commandType is CMD_UTILITY.
      That allows simplifying a lot of places that were testing both fields.
      (I think some of those were just defensive programming, but in many places,
      it was actually necessary to avoid confusing DECLARE CURSOR with SELECT.)
      
      Because PlannedStmt carries a canSetTag field, we're also able to get rid
      of some ad-hoc rules about how to reconstruct canSetTag for a bare utility
      statement; specifically, the assumption that a utility is canSetTag if and
      only if it's the only one in its list.  While I see no near-term need for
      relaxing that restriction, it's nice to get rid of the ad-hocery.
      
      The API of ProcessUtility() is changed so that what it's passed is the
      wrapper PlannedStmt not just the bare utility statement.  This will affect
      all users of ProcessUtility_hook, but the changes are pretty trivial; see
      the affected contrib modules for examples of the minimum change needed.
      (Most compilers should give pointer-type-mismatch warnings for uncorrected
      code.)
      
      There's also a change in the API of ExplainOneQuery_hook, to pass through
      cursorOptions instead of expecting hook functions to know what to pick.
      This is needed because of the DECLARE CURSOR changes, but really should
      have been done in 9.6; it's unlikely that any extant hook functions
      know about using CURSOR_OPT_PARALLEL_OK.
      
      Finally, teach gram.y to save statement boundary locations in RawStmt
      nodes, and pass those through to Query and PlannedStmt nodes.  This allows
      more intelligent handling of cases where a source query string contains
      multiple statements.  This patch doesn't actually do anything with the
      information, but a follow-on patch will.  (Passing this information through
      cleanly is the true motivation for these changes; while I think this is all
      good cleanup, it's unlikely we'd have bothered without this end goal.)
      
      catversion bump because addition of location fields to struct Query
      affects stored rules.
      
      This patch is by me, but it owes a good deal to Fabien Coelho who did
      a lot of preliminary work on the problem, and also reviewed the patch.
      
      Discussion: https://postgr.es/m/alpine.DEB.2.20.1612200926310.29821@lancre
      ab1f0c82
    • Tom Lane's avatar
      Throw suitable error for COPY TO STDOUT/FROM STDIN in a SQL function. · 75abb955
      Tom Lane authored
      A client copy can't work inside a function because the FE/BE wire protocol
      doesn't support nesting of a COPY operation within query results.  (Maybe
      it could, but the protocol spec doesn't suggest that clients should support
      this, and libpq for one certainly doesn't.)
      
      In most PLs, this prohibition is enforced by spi.c, but SQL functions don't
      use SPI.  A comparison of _SPI_execute_plan() and init_execution_state()
      shows that rejecting client COPY is the only discrepancy in what they
      allow, so there's no other similar bugs.
      
      This is an astonishingly ancient oversight, so back-patch to all supported
      branches.
      
      Report: https://postgr.es/m/BY2PR05MB2309EABA3DEFA0143F50F0D593780@BY2PR05MB2309.namprd05.prod.outlook.com
      75abb955
    • Magnus Hagander's avatar
      Change default values for backup and replication parameters · f6d6d292
      Magnus Hagander authored
      This changes the default values of the following parameters:
      
      wal_level = replica
      max_wal_senders = 10
      max_replication_slots = 10
      
      in order to make it possible to make a backup and set up simple
      replication on the default settings, without requiring a system restart.
      
      Discussion: https://postgr.es/m/CABUevEy4PR_EAvZEzsbF5s+V0eEvw7shJ2t-AUwbHOjT+yRb3A@mail.gmail.com
      
      Reviewed by Peter Eisentraut. Benchmark help from Tomas Vondra.
      f6d6d292
    • Peter Eisentraut's avatar
      pg_ctl: Change default to wait for all actions · 05cd12ed
      Peter Eisentraut authored
      The different actions in pg_ctl had different defaults for -w and -W,
      mostly for historical reasons.  Most users will want the -w behavior, so
      make that the default.
      
      Remove the -w option in most example and test code, so avoid confusion
      and reduce verbosity.  pg_upgrade is not touched, so it can continue to
      work with older installations.
      Reviewed-by: default avatarBeena Emerson <memissemerson@gmail.com>
      Reviewed-by: default avatarRyan Murphy <ryanfmurphy@gmail.com>
      05cd12ed
    • Peter Eisentraut's avatar
      Updates to reflect that pg_ctl stop -m fast is the default · e574f15d
      Peter Eisentraut authored
      Various example and test code used -m fast explicitly, but since it's
      the default, this can be omitted now or should be replaced by a better
      example.
      
      pg_upgrade is not touched, so it can continue to operate with older
      installations.
      e574f15d
  6. 13 Jan, 2017 1 commit
    • Tom Lane's avatar
      Fix some more regression test row-order-instability issues. · 5ad966ab
      Tom Lane authored
      Commit 0563a3a8 just introduced another instance of the same unsafe
      testing methodology that appeared in 2ac3ef7a, which I corrected in
      257d8157.  Robert/Amit, please stop doing that.
      
      Also look through the rest of f0e44751's test cases, and correct some
      other queries with underdetermined ordering of results from the system
      catalogs.  These haven't failed in the buildfarm yet, but I don't
      have any confidence in that staying true.
      
      Per multiple buildfarm members.
      5ad966ab