Skip to content
Snippets Groups Projects
  1. May 15, 2015
    • Simon Riggs's avatar
      TABLESAMPLE, SQL Standard and extensible · f6d208d6
      Simon Riggs authored
      Add a TABLESAMPLE clause to SELECT statements that allows
      user to specify random BERNOULLI sampling or block level
      SYSTEM sampling. Implementation allows for extensible
      sampling functions to be written, using a standard API.
      Basic version follows SQLStandard exactly. Usable
      concrete use cases for the sampling API follow in later
      commits.
      
      Petr Jelinek
      
      Reviewed by Michael Paquier and Simon Riggs
      f6d208d6
    • Simon Riggs's avatar
      Separate block sampling functions · 83e176ec
      Simon Riggs authored
      Refactoring ahead of tablesample patch
      
      Requested and reviewed by Michael Paquier
      
      Petr Jelinek
      83e176ec
  2. May 13, 2015
    • Tom Lane's avatar
      Fix postgres_fdw to return the right ctid value in EvalPlanQual cases. · 0bb8528b
      Tom Lane authored
      If a postgres_fdw foreign table is a non-locked source relation in an
      UPDATE, DELETE, or SELECT FOR UPDATE/SHARE, and the query selects its
      ctid column, the wrong value would be returned if an EvalPlanQual
      recheck occurred.  This happened because the foreign table's result row
      was copied via the ROW_MARK_COPY code path, and EvalPlanQualFetchRowMarks
      just unconditionally set the reconstructed tuple's t_self to "invalid".
      
      To fix that, we can have EvalPlanQualFetchRowMarks copy the composite
      datum's t_ctid field, and be sure to initialize that along with t_self
      when postgres_fdw constructs a tuple to return.
      
      If we just did that much then EvalPlanQualFetchRowMarks would start
      returning "(0,0)" as ctid for all other ROW_MARK_COPY cases, which perhaps
      does not matter much, but then again maybe it might.  The cause of that is
      that heap_form_tuple, which is the ultimate source of all composite datums,
      simply leaves t_ctid as zeroes in newly constructed tuples.  That seems
      like a bad idea on general principles: a field that's really not been
      initialized shouldn't appear to have a valid value.  So let's eat the
      trivial additional overhead of doing "ItemPointerSetInvalid(&(td->t_ctid))"
      in heap_form_tuple.
      
      This closes out our handling of Etsuro Fujita's report that tableoid and
      ctid weren't correctly set in postgres_fdw EvalPlanQual cases.  Along the
      way we did a great deal of work to improve FDWs' ability to control row
      locking behavior; which was not wasted effort by any means, but it didn't
      end up being a fix for this problem because that feature would be too
      expensive for postgres_fdw to use all the time.
      
      Although the fix for the tableoid misbehavior was back-patched, I'm
      hesitant to do so here; it seems far less likely that people would care
      about remote ctid than tableoid, and even such a minor behavioral change
      as this in heap_form_tuple is perhaps best not back-patched.  So commit
      to HEAD only, at least for the moment.
      
      Etsuro Fujita, with some adjustments by me
      0bb8528b
  3. May 12, 2015
  4. May 10, 2015
    • Tom Lane's avatar
      Code review for foreign/custom join pushdown patch. · 1a8a4e5c
      Tom Lane authored
      Commit e7cb7ee1 included some design
      decisions that seem pretty questionable to me, and there was quite a lot
      of stuff not to like about the documentation and comments.  Clean up
      as follows:
      
      * Consider foreign joins only between foreign tables on the same server,
      rather than between any two foreign tables with the same underlying FDW
      handler function.  In most if not all cases, the FDW would simply have had
      to apply the same-server restriction itself (far more expensively, both for
      lack of caching and because it would be repeated for each combination of
      input sub-joins), or else risk nasty bugs.  Anyone who's really intent on
      doing something outside this restriction can always use the
      set_join_pathlist_hook.
      
      * Rename fdw_ps_tlist/custom_ps_tlist to fdw_scan_tlist/custom_scan_tlist
      to better reflect what they're for, and allow these custom scan tlists
      to be used even for base relations.
      
      * Change make_foreignscan() API to include passing the fdw_scan_tlist
      value, since the FDW is required to set that.  Backwards compatibility
      doesn't seem like an adequate reason to expect FDWs to set it in some
      ad-hoc extra step, and anyway existing FDWs can just pass NIL.
      
      * Change the API of path-generating subroutines of add_paths_to_joinrel,
      and in particular that of GetForeignJoinPaths and set_join_pathlist_hook,
      so that various less-used parameters are passed in a struct rather than
      as separate parameter-list entries.  The objective here is to reduce the
      probability that future additions to those parameter lists will result in
      source-level API breaks for users of these hooks.  It's possible that this
      is even a small win for the core code, since most CPU architectures can't
      pass more than half a dozen parameters efficiently anyway.  I kept root,
      joinrel, outerrel, innerrel, and jointype as separate parameters to reduce
      code churn in joinpath.c --- in particular, putting jointype into the
      struct would have been problematic because of the subroutines' habit of
      changing their local copies of that variable.
      
      * Avoid ad-hocery in ExecAssignScanProjectionInfo.  It was probably all
      right for it to know about IndexOnlyScan, but if the list is to grow
      we should refactor the knowledge out to the callers.
      
      * Restore nodeForeignscan.c's previous use of the relcache to avoid
      extra GetFdwRoutine lookups for base-relation scans.
      
      * Lots of cleanup of documentation and missed comments.  Re-order some
      code additions into more logical places.
      1a8a4e5c
  5. May 08, 2015
    • Andres Freund's avatar
      Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE. · 168d5805
      Andres Freund authored
      The newly added ON CONFLICT clause allows to specify an alternative to
      raising a unique or exclusion constraint violation error when inserting.
      ON CONFLICT refers to constraints that can either be specified using a
      inference clause (by specifying the columns of a unique constraint) or
      by naming a unique or exclusion constraint.  DO NOTHING avoids the
      constraint violation, without touching the pre-existing row.  DO UPDATE
      SET ... [WHERE ...] updates the pre-existing tuple, and has access to
      both the tuple proposed for insertion and the existing tuple; the
      optional WHERE clause can be used to prevent an update from being
      executed.  The UPDATE SET and WHERE clauses have access to the tuple
      proposed for insertion using the "magic" EXCLUDED alias, and to the
      pre-existing tuple using the table name or its alias.
      
      This feature is often referred to as upsert.
      
      This is implemented using a new infrastructure called "speculative
      insertion". It is an optimistic variant of regular insertion that first
      does a pre-check for existing tuples and then attempts an insert.  If a
      violating tuple was inserted concurrently, the speculatively inserted
      tuple is deleted and a new attempt is made.  If the pre-check finds a
      matching tuple the alternative DO NOTHING or DO UPDATE action is taken.
      If the insertion succeeds without detecting a conflict, the tuple is
      deemed inserted.
      
      To handle the possible ambiguity between the excluded alias and a table
      named excluded, and for convenience with long relation names, INSERT
      INTO now can alias its target table.
      
      Bumps catversion as stored rules change.
      
      Author: Peter Geoghegan, with significant contributions from Heikki
          Linnakangas and Andres Freund. Testing infrastructure by Jeff Janes.
      Reviewed-By: Heikki Linnakangas, Andres Freund, Robert Haas, Simon Riggs,
          Dean Rasheed, Stephen Frost and many others.
      168d5805
    • Andres Freund's avatar
      Represent columns requiring insert and update privileges indentently. · 2c8f4836
      Andres Freund authored
      Previously, relation range table entries used a single Bitmapset field
      representing which columns required either UPDATE or INSERT privileges,
      despite the fact that INSERT and UPDATE privileges are separately
      cataloged, and may be independently held.  As statements so far required
      either insert or update privileges but never both, that was
      sufficient. The required permission could be inferred from the top level
      statement run.
      
      The upcoming INSERT ... ON CONFLICT UPDATE feature needs to
      independently check for both privileges in one statement though, so that
      is not sufficient anymore.
      
      Bumps catversion as stored rules change.
      
      Author: Peter Geoghegan
      Reviewed-By: Andres Freund
      2c8f4836
  6. Apr 30, 2015
    • Robert Haas's avatar
      Create an infrastructure for parallel computation in PostgreSQL. · 924bcf4f
      Robert Haas authored
      This does four basic things.  First, it provides convenience routines
      to coordinate the startup and shutdown of parallel workers.  Second,
      it synchronizes various pieces of state (e.g. GUCs, combo CID
      mappings, transaction snapshot) from the parallel group leader to the
      worker processes.  Third, it prohibits various operations that would
      result in unsafe changes to that state while parallelism is active.
      Finally, it propagates events that would result in an ErrorResponse,
      NoticeResponse, or NotifyResponse message being sent to the client
      from the parallel workers back to the master, from which they can then
      be sent on to the client.
      
      Robert Haas, Amit Kapila, Noah Misch, Rushabh Lathia, Jeevan Chalke.
      Suggestions and review from Andres Freund, Heikki Linnakangas, Noah
      Misch, Simon Riggs, Euler Taveira, and Jim Nasby.
      924bcf4f
  7. Apr 22, 2015
    • Stephen Frost's avatar
      Pull in tableoid for inheiritance with rowMarks · 4ccc5bd2
      Stephen Frost authored
      As noted by Etsuro Fujita [1] and Dean Rasheed[2],
      cb1ca4d8 changed ExecBuildAuxRowMark()
      to always look for the tableoid in the target list, but didn't also
      change preprocess_targetlist() to always include the tableoid.  This
      resulted in errors with soon-to-be-added RLS with inheritance tests,
      and errors when using inheritance with foreign tables.
      
      Authors: Etsuro Fujita and Dean Rasheed (independently)
      
      Minor word-smithing on the comments by me.
      
      [1] 552CF0B6.8010006@lab.ntt.co.jp
      [2] CAEZATCVmFUfUOwwhnBTcgi6AquyjQ0-1fyKd0T3xBWJvn+xsFA@mail.gmail.com
      4ccc5bd2
  8. Mar 31, 2015
  9. Mar 30, 2015
    • Tom Lane's avatar
      Be more careful about printing constants in ruleutils.c. · 542320c2
      Tom Lane authored
      The previous coding in get_const_expr() tried to avoid quoting integer,
      float, and numeric literals if at all possible.  While that looks nice,
      it means that dumped expressions might re-parse to something that's
      semantically equivalent but not the exact same parsetree; for example
      a FLOAT8 constant would re-parse as a NUMERIC constant with a cast to
      FLOAT8.  Though the result would be the same after constant-folding,
      this is problematic in certain contexts.  In particular, Jeff Davis
      pointed out that this could cause unexpected failures in ALTER INHERIT
      operations because of child tables having not-exactly-equivalent CHECK
      expressions.  Therefore, favor correctness over legibility and dump
      such constants in quotes except in the limited cases where they'll
      be interpreted as the same type even without any casting.
      
      This results in assorted small changes in the regression test outputs,
      and will affect display of user-defined views and rules similarly.
      The odds of that causing problems in the field seem non-negligible;
      given the lack of previous complaints, it seems best not to change
      this in the back branches.
      542320c2
  10. Mar 22, 2015
    • Tom Lane's avatar
      Allow foreign tables to participate in inheritance. · cb1ca4d8
      Tom Lane authored
      Foreign tables can now be inheritance children, or parents.  Much of the
      system was already ready for this, but we had to fix a few things of
      course, mostly in the area of planner and executor handling of row locks.
      
      As side effects of this, allow foreign tables to have NOT VALID CHECK
      constraints (and hence to accept ALTER ... VALIDATE CONSTRAINT), and to
      accept ALTER SET STORAGE and ALTER SET WITH/WITHOUT OIDS.  Continuing to
      disallow these things would've required bizarre and inconsistent special
      cases in inheritance behavior.  Since foreign tables don't enforce CHECK
      constraints anyway, a NOT VALID one is a complete no-op, but that doesn't
      mean we shouldn't allow it.  And it's possible that some FDWs might have
      use for SET STORAGE or SET WITH OIDS, though doubtless they will be no-ops
      for most.
      
      An additional change in support of this is that when a ModifyTable node
      has multiple target tables, they will all now be explicitly identified
      in EXPLAIN output, for example:
      
       Update on pt1  (cost=0.00..321.05 rows=3541 width=46)
         Update on pt1
         Foreign Update on ft1
         Foreign Update on ft2
         Update on child3
         ->  Seq Scan on pt1  (cost=0.00..0.00 rows=1 width=46)
         ->  Foreign Scan on ft1  (cost=100.00..148.03 rows=1170 width=46)
         ->  Foreign Scan on ft2  (cost=100.00..148.03 rows=1170 width=46)
         ->  Seq Scan on child3  (cost=0.00..25.00 rows=1200 width=46)
      
      This was done mainly to provide an unambiguous place to attach "Remote SQL"
      fields, but it is useful for inherited updates even when no foreign tables
      are involved.
      
      Shigeru Hanada and Etsuro Fujita, reviewed by Ashutosh Bapat and Kyotaro
      Horiguchi, some additional hacking by me
      cb1ca4d8
  11. Mar 15, 2015
    • Tom Lane's avatar
      Improve representation of PlanRowMark. · 7b8b8a43
      Tom Lane authored
      This patch fixes two inadequacies of the PlanRowMark representation.
      
      First, that the original LockingClauseStrength isn't stored (and cannot be
      inferred for foreign tables, which always get ROW_MARK_COPY).  Since some
      PlanRowMarks are created out of whole cloth and don't actually have an
      ancestral RowMarkClause, this requires adding a dummy LCS_NONE value to
      enum LockingClauseStrength, which is fairly annoying but the alternatives
      seem worse.  This fix allows getting rid of the use of get_parse_rowmark()
      in FDWs (as per the discussion around commits 462bd957 and
      8ec8760f), and it simplifies some things elsewhere.
      
      Second, that the representation assumed that all child tables in an
      inheritance hierarchy would use the same RowMarkType.  That's true today
      but will soon not be true.  We add an "allMarkTypes" field that identifies
      the union of mark types used in all a parent table's children, and use
      that where appropriate (currently, only in preprocess_targetlist()).
      
      In passing fix a couple of minor infelicities left over from the SKIP
      LOCKED patch, notably that _outPlanRowMark still thought waitPolicy
      is a bool.
      
      Catversion bump is required because the numeric values of enum
      LockingClauseStrength can appear in on-disk rules.
      
      Extracted from a much larger patch to support foreign table inheritance;
      it seemed worth breaking this out, since it's a separable concern.
      
      Shigeru Hanada and Etsuro Fujita, somewhat modified by me
      7b8b8a43
  12. Feb 21, 2015
  13. Jan 06, 2015
  14. Dec 18, 2014
    • Tom Lane's avatar
      Improve hash_create's API for selecting simple-binary-key hash functions. · 4a14f13a
      Tom Lane authored
      Previously, if you wanted anything besides C-string hash keys, you had to
      specify a custom hashing function to hash_create().  Nearly all such
      callers were specifying tag_hash or oid_hash; which is tedious, and rather
      error-prone, since a caller could easily miss the opportunity to optimize
      by using hash_uint32 when appropriate.  Replace this with a design whereby
      callers using simple binary-data keys just specify HASH_BLOBS and don't
      need to mess with specific support functions.  hash_create() itself will
      take care of optimizing when the key size is four bytes.
      
      This nets out saving a few hundred bytes of code space, and offers
      a measurable performance improvement in tidbitmap.c (which was not
      exploiting the opportunity to use hash_uint32 for its 4-byte keys).
      There might be some wins elsewhere too, I didn't analyze closely.
      
      In future we could look into offering a similar optimized hashing function
      for 8-byte keys.  Under this design that could be done in a centralized
      and machine-independent fashion, whereas getting it right for keys of
      platform-dependent sizes would've been notationally painful before.
      
      For the moment, the old way still works fine, so as not to break source
      code compatibility for loadable modules.  Eventually we might want to
      remove tag_hash and friends from the exported API altogether, since there's
      no real need for them to be explicitly referenced from outside dynahash.c.
      
      Teodor Sigaev and Tom Lane
      4a14f13a
  15. Dec 17, 2014
    • Tom Lane's avatar
      Allow CHECK constraints to be placed on foreign tables. · fc2ac1fb
      Tom Lane authored
      As with NOT NULL constraints, we consider that such constraints are merely
      reports of constraints that are being enforced by the remote server (or
      other underlying storage mechanism).  Their only real use is to allow
      planner optimizations, for example in constraint-exclusion checks.  Thus,
      the code changes here amount to little more than removal of the error that
      was formerly thrown for applying CHECK to a foreign table.
      
      (In passing, do a bit of cleanup of the ALTER FOREIGN TABLE reference page,
      which had accumulated some weird decisions about ordering etc.)
      
      Shigeru Hanada and Etsuro Fujita, reviewed by Kyotaro Horiguchi and
      Ashutosh Bapat.
      fc2ac1fb
  16. Dec 12, 2014
    • Tom Lane's avatar
      Revert misguided change to postgres_fdw FOR UPDATE/SHARE code. · 8ec8760f
      Tom Lane authored
      In commit 462bd957, I changed postgres_fdw
      to rely on get_plan_rowmark() instead of get_parse_rowmark().  I still
      think that's a good idea in the long run, but as Etsuro Fujita pointed out,
      it doesn't work today because planner.c forces PlanRowMarks to have
      markType = ROW_MARK_COPY for all foreign tables.  There's no urgent reason
      to change this in the back branches, so let's just revert that part of
      yesterday's commit rather than trying to design a better solution under
      time pressure.
      
      Also, add a regression test case showing what postgres_fdw does with FOR
      UPDATE/SHARE.  I'd blithely assumed there was one already, else I'd have
      realized yesterday that this code didn't work.
      8ec8760f
    • Tom Lane's avatar
      Fix planning of SELECT FOR UPDATE on child table with partial index. · 462bd957
      Tom Lane authored
      Ordinarily we can omit checking of a WHERE condition that matches a partial
      index's condition, when we are using an indexscan on that partial index.
      However, in SELECT FOR UPDATE we must include the "redundant" filter
      condition in the plan so that it gets checked properly in an EvalPlanQual
      recheck.  The planner got this mostly right, but improperly omitted the
      filter condition if the index in question was on an inheritance child
      table.  In READ COMMITTED mode, this could result in incorrectly returning
      just-updated rows that no longer satisfy the filter condition.
      
      The cause of the error is using get_parse_rowmark() when get_plan_rowmark()
      is what should be used during planning.  In 9.3 and up, also fix the same
      mistake in contrib/postgres_fdw.  It's currently harmless there (for lack
      of inheritance support) but wrong is wrong, and the incorrect code might
      get copied to someplace where it's more significant.
      
      Report and fix by Kyotaro Horiguchi.  Back-patch to all supported branches.
      462bd957
  17. Dec 04, 2014
    • Peter Eisentraut's avatar
      Fix SHLIB_PREREQS use in contrib, allowing PGXS builds · 1e95bbc8
      Peter Eisentraut authored
      dblink and postgres_fdw use SHLIB_PREREQS = submake-libpq to build libpq
      first.  This doesn't work in a PGXS build, because there is no libpq to
      build.  So just omit setting SHLIB_PREREQS in this case.
      
      Note that PGXS users can still use SHLIB_PREREQS (although it is not
      documented).  The problem here is only that contrib modules can be built
      in-tree or using PGXS, and the prerequisite is only applicable in the
      former case.
      
      Commit 6697aa2b previously attempted to
      address this by creating a somewhat fake submake-libpq target in
      Makefile.global.  That was not the right fix, and it was also done in a
      nonportable way, so revert that.
      1e95bbc8
  18. Nov 28, 2014
    • Tom Lane's avatar
      Add bms_next_member(), and use it where appropriate. · f4e031c6
      Tom Lane authored
      This patch adds a way of iterating through the members of a bitmapset
      nondestructively, unlike the old way with bms_first_member().  While
      bms_next_member() is very slightly slower than bms_first_member()
      (at least for typical-size bitmapsets), eliminating the need to palloc
      and pfree a temporary copy of the target bitmapset is a significant win.
      So this method should be preferred in all cases where a temporary copy
      would be necessary.
      
      Tom Lane, with suggestions from Dean Rasheed and David Rowley
      f4e031c6
  19. Nov 24, 2014
  20. Nov 22, 2014
    • Tom Lane's avatar
      Fix mishandling of system columns in FDW queries. · 9c581011
      Tom Lane authored
      postgres_fdw would send query conditions involving system columns to the
      remote server, even though it makes no effort to ensure that system
      columns other than CTID match what the remote side thinks.  tableoid,
      in particular, probably won't match and might have some use in queries.
      Hence, prevent sending conditions that include non-CTID system columns.
      
      Also, create_foreignscan_plan neglected to check local restriction
      conditions while determining whether to set fsSystemCol for a foreign
      scan plan node.  This again would bollix the results for queries that
      test a foreign table's tableoid.
      
      Back-patch the first fix to 9.3 where postgres_fdw was introduced.
      Back-patch the second to 9.2.  The code is probably broken in 9.1 as
      well, but the patch doesn't apply cleanly there; given the weak state
      of support for FDWs in 9.1, it doesn't seem worth fixing.
      
      Etsuro Fujita, reviewed by Ashutosh Bapat, and somewhat modified by me
      9c581011
  21. Nov 15, 2014
  22. Aug 26, 2014
    • Andres Freund's avatar
      Specify the port in dblink and postgres_fdw tests. · 57ca1d4f
      Andres Freund authored
      That allows to run those tests against a postmaster listening on a
      nonstandard port without requiring to export PGPORT in postmaster's
      environment.
      
      This still doesn't support connecting to a nondefault host without
      configuring it in postmaster's environment. That's harder and less
      frequently used though. So this is a useful step.
      57ca1d4f
    • Andres Freund's avatar
      Don't hardcode contrib_regression dbname in postgres_fdw and dblink tests. · ddc2504d
      Andres Freund authored
      That allows parallel installcheck to succeed inside contrib/. The
      output is not particularly pretty unless make's -O option to
      synchronize the output is used.
      
      There's other tests, outside contrib, that use a hardcoded,
      non-unique, database name. Those prohibit paralell installcheck to be
      used across more directories; but that's something for a separate
      patch.
      ddc2504d
  23. Jul 14, 2014
  24. Jul 10, 2014
    • Tom Lane's avatar
      Implement IMPORT FOREIGN SCHEMA. · 59efda3e
      Tom Lane authored
      This command provides an automated way to create foreign table definitions
      that match remote tables, thereby reducing tedium and chances for error.
      In this patch, we provide the necessary core-server infrastructure and
      implement the feature fully in the postgres_fdw foreign-data wrapper.
      Other wrappers will throw a "feature not supported" error until/unless
      they are updated.
      
      Ronan Dunklau and Michael Paquier, additional work by me
      59efda3e
    • Bruce Momjian's avatar
      Adjust blank lines around PG_MODULE_MAGIC defines, for consistency · 6a605cd6
      Bruce Momjian authored
      Report by Robert Haas
      6a605cd6
  25. Jun 16, 2014
    • Tom Lane's avatar
      Avoid recursion when processing simple lists of AND'ed or OR'ed clauses. · 2146f134
      Tom Lane authored
      Since most of the system thinks AND and OR are N-argument expressions
      anyway, let's have the grammar generate a representation of that form when
      dealing with input like "x AND y AND z AND ...", rather than generating
      a deeply-nested binary tree that just has to be flattened later by the
      planner.  This avoids stack overflow in parse analysis when dealing with
      queries having more than a few thousand such clauses; and in any case it
      removes some rather unsightly inconsistencies, since some parts of parse
      analysis were generating N-argument ANDs/ORs already.
      
      It's still possible to get a stack overflow with weirdly parenthesized
      input, such as "x AND (y AND (z AND ( ... )))", but such cases are not
      mainstream usage.  The maximum depth of parenthesization is already
      limited by Bison's stack in such cases, anyway, so that the limit is
      probably fairly platform-independent.
      
      Patch originally by Gurjeet Singh, heavily revised by me
      2146f134
  26. May 06, 2014
    • Bruce Momjian's avatar
      pgindent run for 9.4 · 0a783200
      Bruce Momjian authored
      This includes removing tabs after periods in C comments, which was
      applied to back branches, so this change should not effect backpatching.
      0a783200
  27. Apr 18, 2014
    • Peter Eisentraut's avatar
      Create function prototype as part of PG_FUNCTION_INFO_V1 macro · e7128e8d
      Peter Eisentraut authored
      Because of gcc -Wmissing-prototypes, all functions in dynamically
      loadable modules must have a separate prototype declaration.  This is
      meant to detect global functions that are not declared in header files,
      but in cases where the function is called via dfmgr, this is redundant.
      Besides filling up space with boilerplate, this is a frequent source of
      compiler warnings in extension modules.
      
      We can fix that by creating the function prototype as part of the
      PG_FUNCTION_INFO_V1 macro, which such modules have to use anyway.  That
      makes the code of modules cleaner, because there is one less place where
      the entry points have to be listed, and creates an additional check that
      functions have the right prototype.
      
      Remove now redundant prototypes from contrib and other modules.
      e7128e8d
  28. Apr 16, 2014
    • Tom Lane's avatar
      Fix contrib/postgres_fdw's remote-estimate representation of array Params. · 5b68d816
      Tom Lane authored
      We were emitting "(SELECT null::typename)", which is usually interpreted
      as a scalar subselect, but not so much in the context "x = ANY(...)".
      This led to remote-side parsing failures when remote_estimate is enabled.
      A quick and ugly fix is to stick in an extra cast step,
      "((SELECT null::typename)::typename)".  The cast will be thrown away as
      redundant by parse analysis, but not before it's done its job of making
      sure the grammar sees the ANY argument as an a_expr rather than a
      select_with_parens.  Per an example from Hannu Krosing.
      5b68d816
  29. Apr 04, 2014
    • Tom Lane's avatar
      Fix non-equivalence of VARIADIC and non-VARIADIC function call formats. · c7b35395
      Tom Lane authored
      For variadic functions (other than VARIADIC ANY), the syntaxes foo(x,y,...)
      and foo(VARIADIC ARRAY[x,y,...]) should be considered equivalent, since the
      former is converted to the latter at parse time.  They have indeed been
      equivalent, in all releases before 9.3.  However, commit 75b39e79 made an
      ill-considered decision to record which syntax had been used in FuncExpr
      nodes, and then to make equal() test that in checking node equality ---
      which caused the syntaxes to not be seen as equivalent by the planner.
      This is the underlying cause of bug #9817 from Dmitry Ryabov.
      
      It might seem that a quick fix would be to make equal() disregard
      FuncExpr.funcvariadic, but the same commit made that untenable, because
      the field actually *is* semantically significant for some VARIADIC ANY
      functions.  This patch instead adopts the approach of redefining
      funcvariadic (and aggvariadic, in HEAD) as meaning that the last argument
      is a variadic array, whether it got that way by parser intervention or was
      supplied explicitly by the user.  Therefore the value will always be true
      for non-ANY variadic functions, restoring the principle of equivalence.
      (However, the planner will continue to consider use of VARIADIC as a
      meaningful difference for VARIADIC ANY functions, even though some such
      functions might disregard it.)
      
      In HEAD, this change lets us simplify the decompilation logic in
      ruleutils.c, since the funcvariadic/aggvariadic flag tells directly whether
      to print VARIADIC.  However, in 9.3 we have to continue to cope with
      existing stored rules/views that might contain the previous definition.
      Fortunately, this just means no change in ruleutils.c, since its existing
      behavior effectively ignores funcvariadic for all cases other than VARIADIC
      ANY functions.
      
      In HEAD, bump catversion to reflect the fact that FuncExpr.funcvariadic
      changed meanings; this is sort of pro forma, since I don't believe any
      built-in views are affected.
      
      Unfortunately, this patch doesn't magically fix everything for affected
      9.3 users.  After installing 9.3.5, they might need to recreate their
      rules/views/indexes containing variadic function calls in order to get
      everything consistent with the new definition.  As in the cited bug,
      the symptom of a problem would be failure to use a nominally matching
      index that has a variadic function call in its definition.  We'll need
      to mention this in the 9.3.5 release notes.
      c7b35395
  30. Mar 23, 2014
    • Noah Misch's avatar
      Don't test xmin/xmax columns of a postgres_fdw foreign table. · b2b2491b
      Noah Misch authored
      Their values are unspecified and system-dependent.
      
      Per buildfarm member kouprey.
      b2b2491b
    • Noah Misch's avatar
      Offer triggers on foreign tables. · 7cbe57c3
      Noah Misch authored
      This covers all the SQL-standard trigger types supported for regular
      tables; it does not cover constraint triggers.  The approach for
      acquiring the old row mirrors that for view INSTEAD OF triggers.  For
      AFTER ROW triggers, we spool the foreign tuples to a tuplestore.
      
      This changes the FDW API contract; when deciding which columns to
      populate in the slot returned from data modification callbacks, writable
      FDWs will need to check for AFTER ROW triggers in addition to checking
      for a RETURNING clause.
      
      In support of the feature addition, refactor the TriggerFlags bits and
      the assembly of old tuples in ModifyTable.
      
      Ronan Dunklau, reviewed by KaiGai Kohei; some additional hacking by me.
      7cbe57c3
  31. Mar 07, 2014
    • Tom Lane's avatar
      Fix contrib/postgres_fdw to handle multiple join conditions properly. · 83204e10
      Tom Lane authored
      The previous coding supposed that it could consider just a single join
      condition in any one parameterized path for the foreign table.  But in
      reality, the parameterized-path machinery forces all join clauses that are
      "movable to" the foreign table to be evaluated at that node; including
      clauses that we might not consider safe to send across.  Such cases would
      result in an Assert failure in an assert-enabled build, and otherwise in
      sending an unsafe clause to the foreign server, which might result in
      errors or silently-wrong answers.  A lesser problem was that the
      cost/rowcount estimates generated for the parameterized path failed to
      account for any additional join quals that get assigned to the scan.
      
      To fix, rewrite postgresGetForeignPaths so that it correctly collects all
      the movable quals for any one outer relation when generating parameterized
      paths; we'll now generate just one path per outer relation not one per join
      qual.  Also fix bogus assumptions in postgresGetForeignPlan and
      estimate_path_cost_size that only safe-to-send join quals will be
      presented.
      
      Based on complaint from Etsuro Fujita that the path costs were being
      miscalculated, though this is significantly different from his proposed
      patch.
      83204e10
  32. Feb 04, 2014
    • Tom Lane's avatar
      Improve connection-failure error handling in contrib/postgres_fdw. · 00d4f2af
      Tom Lane authored
      postgres_fdw tended to say "unknown error" if it tried to execute a command
      on an already-dead connection, because some paths in libpq just return a
      null PGresult for such cases.  Out-of-memory might result in that, too.
      To fix, pass the PGconn to pgfdw_report_error, and look at its
      PQerrorMessage() string if we can't get anything out of the PGresult.
      
      Also, fix the transaction-exit logic to reliably drop a dead connection.
      It was attempting to do that already, but it assumed that only connection
      cache entries with xact_depth > 0 needed to be examined.  The folly in that
      is that if we fail while issuing START TRANSACTION, we'll not have bumped
      xact_depth.  (At least for the case I was testing, this fix masks the
      other problem; but it still seems like a good idea to have the PGconn
      fallback logic.)
      
      Per investigation of bug #9087 from Craig Lucas.  Backpatch to 9.3 where
      this code was introduced.
      00d4f2af
  33. Jan 07, 2014
Loading