Skip to content
Snippets Groups Projects
  1. Oct 21, 2016
    • Robert Haas's avatar
      postgres_fdw: Push down aggregates to remote servers. · 7012b132
      Robert Haas authored
      Now that the upper planner uses paths, and now that we have proper hooks
      to inject paths into the upper planning process, it's possible for
      foreign data wrappers to arrange to push aggregates to the remote side
      instead of fetching all of the rows and aggregating them locally.  This
      figures to be a massive win for performance, so teach postgres_fdw to
      do it.
      
      Jeevan Chalke and Ashutosh Bapat.  Reviewed by Ashutosh Bapat with
      additional testing by Prabhat Sahu.  Various mostly cosmetic changes
      by me.
      7012b132
  2. Aug 27, 2016
    • Tom Lane's avatar
      Add macros to make AllocSetContextCreate() calls simpler and safer. · ea268cdc
      Tom Lane authored
      I found that half a dozen (nearly 5%) of our AllocSetContextCreate calls
      had typos in the context-sizing parameters.  While none of these led to
      especially significant problems, they did create minor inefficiencies,
      and it's now clear that expecting people to copy-and-paste those calls
      accurately is not a great idea.  Let's reduce the risk of future errors
      by introducing single macros that encapsulate the common use-cases.
      Three such macros are enough to cover all but two special-purpose contexts;
      those two calls can be left as-is, I think.
      
      While this patch doesn't in itself improve matters for third-party
      extensions, it doesn't break anything for them either, and they can
      gradually adopt the simplified notation over time.
      
      In passing, change TopMemoryContext to use the default allocation
      parameters.  Formerly it could only be extended 8K at a time.  That was
      probably reasonable when this code was written; but nowadays we create
      many more contexts than we did then, so that it's not unusual to have a
      couple hundred K in TopMemoryContext, even without considering various
      dubious code that sticks other things there.  There seems no good reason
      not to let it use growing blocks like most other contexts.
      
      Back-patch to 9.6, mostly because that's still close enough to HEAD that
      it's easy to do so, and keeping the branches in sync can be expected to
      avoid some future back-patching pain.  The bugs fixed by these changes
      don't seem to be significant enough to justify fixing them further back.
      
      Discussion: <21072.1472321324@sss.pgh.pa.us>
      ea268cdc
  3. Aug 26, 2016
    • Heikki Linnakangas's avatar
      Support OID system column in postgres_fdw. · ae025a15
      Heikki Linnakangas authored
      You can use ALTER FOREIGN TABLE SET WITH OIDS on a foreign table, but the
      oid column read out as zeros, because the postgres_fdw didn't know about
      it. Teach postgres_fdw how to fetch it.
      
      Etsuro Fujita, with an additional test case by me.
      
      Discussion: <56E90A76.5000503@lab.ntt.co.jp>
      ae025a15
  4. Aug 24, 2016
  5. Jul 15, 2016
    • Tom Lane's avatar
      Avoid invalidating all foreign-join cached plans when user mappings change. · 45639a05
      Tom Lane authored
      We must not push down a foreign join when the foreign tables involved
      should be accessed under different user mappings.  Previously we tried
      to enforce that rule literally during planning, but that meant that the
      resulting plans were dependent on the current contents of the
      pg_user_mapping catalog, and we had to blow away all cached plans
      containing any remote join when anything at all changed in pg_user_mapping.
      This could have been improved somewhat, but the fact that a syscache inval
      callback has very limited info about what changed made it hard to do better
      within that design.  Instead, let's change the planner to not consider user
      mappings per se, but to allow a foreign join if both RTEs have the same
      checkAsUser value.  If they do, then they necessarily will use the same
      user mapping at runtime, and we don't need to know specifically which one
      that is.  Post-plan-time changes in pg_user_mapping no longer require any
      plan invalidation.
      
      This rule does give up some optimization ability, to wit where two foreign
      table references come from views with different owners or one's from a view
      and one's directly in the query, but nonetheless the same user mapping
      would have applied.  We'll sacrifice the first case, but to not regress
      more than we have to in the second case, allow a foreign join involving
      both zero and nonzero checkAsUser values if the nonzero one is the same as
      the prevailing effective userID.  In that case, mark the plan as only
      runnable by that userID.
      
      The plancache code already had a notion of plans being userID-specific,
      in order to support RLS.  It was a little confused though, in particular
      lacking clarity of thought as to whether it was the rewritten query or just
      the finished plan that's dependent on the userID.  Rearrange that code so
      that it's clearer what depends on which, and so that the same logic applies
      to both RLS-injected role dependency and foreign-join-injected role
      dependency.
      
      Note that this patch doesn't remove the other issue mentioned in the
      original complaint, which is that while we'll reliably stop using a foreign
      join if it's disallowed in a new context, we might fail to start using a
      foreign join if it's now allowed, but we previously created a generic
      cached plan that didn't use one.  It was agreed that the chance of winning
      that way was not high enough to justify the much larger number of plan
      invalidations that would have to occur if we tried to cause it to happen.
      
      In passing, clean up randomly-varying spelling of EXPLAIN commands in
      postgres_fdw.sql, and fix a COSTS ON example that had been allowed to
      leak into the committed tests.
      
      This reverts most of commits fbe5a3fb and 5d4171d1, which were the
      previous attempt at ensuring we wouldn't push down foreign joins that
      span permissions contexts.
      
      Etsuro Fujita and Tom Lane
      
      Discussion: <d49c1e5b-f059-20f4-c132-e9752ee0113e@lab.ntt.co.jp>
      45639a05
  6. Jul 01, 2016
  7. Jun 17, 2016
    • Robert Haas's avatar
      postgres_fdw: Rephrase comment. · 177c56d6
      Robert Haas authored
      Per gripe from Thomas Munro, who only complained about a more
      localized problem, but I couldn't resist a bit more wordsmithing.
      177c56d6
  8. Jun 14, 2016
    • Robert Haas's avatar
      postgres_fdw: Check PlaceHolderVars before pushing down a join. · 131c7e70
      Robert Haas authored
      As discovered by Andreas Seltenreich via sqlsmith, it's possible for a
      remote join to need to generate a target list which contains a
      PlaceHolderVar which would need to be evaluated on the remote server.
      This happens when we try to push down a join tree which contains outer
      joins and the nullable side of the join contains a subquery which
      evauates some expression which can go to NULL above the level of the
      join.  Since the deparsing logic can't build a remote query that
      involves subqueries, it fails while trying to produce an SQL query
      that can be sent to the remote side.  Detect such cases and don't try
      to push down the join at all.
      
      It's actually fine to push down the join if the PlaceHolderVar needs
      to be evaluated at the current join level.  This patch makes a small
      change to build_tlist_to_deparse so that this case will work.
      
      Amit Langote, Ashutosh Bapat, and me.
      131c7e70
  9. Jun 10, 2016
  10. May 16, 2016
    • Robert Haas's avatar
      postgres_fdw: Fix the fix for crash when pushing down multiple joins. · 02a568a0
      Robert Haas authored
      Commit 3151f16e was intended to be
      a commit of a patch from Ashutosh Bapat, but instead I mistakenly
      committed an earlier version from Michael Paquier (because both
      patches were submitted with the same filename, and I confused them).
      Michael's patch fixes the crash but doesn't actually implement the
      correct test.
      
      Repair the incorrect logic, and also expand the comments considerably
      so that this is all more clear.
      
      Ashutosh Bapat and Robert Haas
      02a568a0
  11. Apr 21, 2016
    • Robert Haas's avatar
      Allow queries submitted by postgres_fdw to be canceled. · f039eaac
      Robert Haas authored
      This fixes a problem which is not new, but with the advent of direct
      foreign table modification in 0bf3ae88,
      it's somewhat more likely to be annoying than previously.  So,
      arrange for a local query cancelation to propagate to the remote side.
      
      Michael Paquier, reviewed by Etsuro Fujita.	 Original report by
      Thom Brown.
      f039eaac
    • Robert Haas's avatar
      postgres_fdw: Don't push down certain full joins. · 5b1f9ce1
      Robert Haas authored
      If there's a filter condition on either side of a full outer join,
      it is neither correct to attach it to the join's ON clause nor to
      throw it into the toplevel WHERE clause.  Just don't push down the
      join in that case.
      
      To maximize the number of cases where we can still push down full
      joins, push inner join conditions into the ON clause at the first
      opportunity rather than postponing them to the top-level WHERE
      clause.  This produces nicer SQL, anyway.
      
      This bug was introduced in e4106b25.
      
      Ashutosh Bapat, per report from Rajkumar Raghuwanshi.
      5b1f9ce1
  12. Apr 15, 2016
    • Robert Haas's avatar
      postgres_fdw: Clean up handling of system columns. · da7d44b6
      Robert Haas authored
      Previously, querying the xmin column of a single postgres_fdw foreign
      table fetched the tuple length, xmax the typmod, and cmin or cmax the
      composite type OID of the tuple.  However, when you queried several
      such tables and the join got shipped to the remote side, these columns
      ended up containing the remote values of the corresponding columns.
      Both behaviors are rather unprincipled, the former for obvious reasons
      and the latter because the remote values of these columns don't have
      any local significance; our transaction IDs are in a different space
      than those of the remote machine.  Clean this up by setting all of
      these fields to 0 in both cases.  Also fix the handling of tableoid
      to be sane.
      
      Robert Haas and Ashutosh Bapat, reviewed by Etsuro Fujita.
      da7d44b6
  13. Apr 06, 2016
  14. Mar 29, 2016
  15. Mar 23, 2016
    • Robert Haas's avatar
      postgres_fdw: Fix crash when pushing down multiple joins. · 3151f16e
      Robert Haas authored
      A join clause might mention multiple relations on either side, so it
      need not be the case that a given joinrel's constituent relations are
      all on one side of the join clause or all on the other.
      
      Report by Rajkumar Raghuwanshi.  Analysis and fix by Michael Paquier
      and Ashutosh Bapat.
      3151f16e
  16. Mar 21, 2016
    • Tom Lane's avatar
      Clean up some Coverity complaints about commit 0bf3ae88. · 92b7902d
      Tom Lane authored
      The two get_tle_by_resno() calls introduced by this commit lacked any
      check for a NULL return, unlike any other calls of that function anywhere
      in our tree.  Coverity quite properly complained about it.  Also fix a
      misindented line in process_query_params(), which Coverity also complained
      about on the grounds that the bad indentation suggested possible programmer
      misinterpretation.
      92b7902d
  17. Mar 18, 2016
    • Robert Haas's avatar
      Directly modify foreign tables. · 0bf3ae88
      Robert Haas authored
      postgres_fdw can now sent an UPDATE or DELETE statement directly to
      the foreign server in simple cases, rather than sending a SELECT FOR
      UPDATE statement and then updating or deleting rows one-by-one.
      
      Etsuro Fujita, reviewed by Rushabh Lathia, Shigeru Hanada, Kyotaro
      Horiguchi, Albe Laurenz, Thom Brown, and me.
      0bf3ae88
  18. Mar 15, 2016
  19. Mar 14, 2016
    • Tom Lane's avatar
      Allow callers of create_foreignscan_path to specify nondefault PathTarget. · 28048cba
      Tom Lane authored
      Although the default choice of rel->reltarget should typically be
      sufficient for scan or join paths, it's not at all sufficient for the
      purposes PathTargets were invented for; in particular not for
      upper-relation Paths.  So break API compatibility by adding a PathTarget
      argument to create_foreignscan_path().  To ease updating of existing
      code, accept a NULL value of the argument as selecting rel->reltarget.
      28048cba
    • Tom Lane's avatar
      Rethink representation of PathTargets. · 307c7885
      Tom Lane authored
      In commit 19a54114 I did not make PathTarget a subtype of Node,
      and embedded a RelOptInfo's reltarget directly into it rather than having
      a separately-allocated Node.  In hindsight that was misguided
      micro-optimization, enabled by the fact that at that point we didn't have
      any Paths with custom PathTargets.  Now that PathTarget processing has
      been fleshed out some more, it's easier to see that it's better to have
      PathTarget as an indepedent Node type, even if it does cost us one more
      palloc to create a RelOptInfo.  So change it while we still can.
      
      This commit just changes the representation, without doing anything more
      interesting than that.
      307c7885
    • Robert Haas's avatar
      Update more comments for 96198d94. · 6be84eeb
      Robert Haas authored
      Etsuro Fujita, reviewed (though not completely endorsed) by Ashutosh
      Bapat, and slightly expanded by me.
      6be84eeb
  20. Mar 09, 2016
    • Robert Haas's avatar
      postgres_fdw: Consider foreign joining and foreign sorting together. · aa09cd24
      Robert Haas authored
      Commit ccd8f979 gave us the ability to
      request that the remote side sort the data, and, later, commit
      e4106b25 gave us the ability to
      request that the remote side perform the join for us rather than doing
      it locally.  But we could not do both things at the same time: a
      remote SQL query that had an ORDER BY clause would never be a join.
      This commit adds that capability.
      
      Ashutosh Bapat, reviewed by me.
      aa09cd24
  21. Mar 08, 2016
  22. Feb 21, 2016
    • Robert Haas's avatar
      postgres_fdw: Avoid sharing list substructure. · dd077ef8
      Robert Haas authored
      list_concat(list_concat(a, b), c) destructively changes both a and b;
      to avoid such perils, copy lists of remote_conds before incorporating
      them into larger lists via list_concat().
      
      Ashutosh Bapat, per a report from Etsuro Fujita
      dd077ef8
  23. Feb 19, 2016
    • Tom Lane's avatar
      Add an explicit representation of the output targetlist to Paths. · 19a54114
      Tom Lane authored
      Up to now, there's been an assumption that all Paths for a given relation
      compute the same output column set (targetlist).  However, there are good
      reasons to remove that assumption.  For example, an indexscan on an
      expression index might be able to return the value of an expensive function
      "for free".  While we have the ability to generate such a plan today in
      simple cases, we don't have a way to model that it's cheaper than a plan
      that computes the function from scratch, nor a way to create such a plan
      in join cases (where the function computation would normally happen at
      the topmost join node).  Also, we need this so that we can have Paths
      representing post-scan/join steps, where the targetlist may well change
      from one step to the next.  Therefore, invent a "struct PathTarget"
      representing the columns we expect a plan step to emit.  It's convenient
      to include the output tuple width and tlist evaluation cost in this struct,
      and there will likely be additional fields in future.
      
      While Path nodes that actually do have custom outputs will need their own
      PathTargets, it will still be true that most Paths for a given relation
      will compute the same tlist.  To reduce the overhead added by this patch,
      keep a "default PathTarget" in RelOptInfo, and allow Paths that compute
      that column set to just point to their parent RelOptInfo's reltarget.
      (In the patch as committed, actually every Path is like that, since we
      do not yet have any cases of custom PathTargets.)
      
      I took this opportunity to provide some more-honest costing of
      PlaceHolderVar evaluation.  Up to now, the assumption that "scan/join
      reltargetlists have cost zero" was applied not only to Vars, where it's
      reasonable, but also PlaceHolderVars where it isn't.  Now, we add the eval
      cost of a PlaceHolderVar's expression to the first plan level where it can
      be computed, by including it in the PathTarget cost field and adding that
      to the cost estimates for Paths.  This isn't perfect yet but it's much
      better than before, and there is a way forward to improve it more.  This
      costing change affects the join order chosen for a couple of the regression
      tests, changing expected row ordering.
      19a54114
  24. Feb 10, 2016
  25. Feb 09, 2016
    • Robert Haas's avatar
      postgres_fdw: Push down joins to remote servers. · e4106b25
      Robert Haas authored
      If we've got a relatively straightforward join between two tables,
      this pushes that join down to the remote server instead of fetching
      the rows for each table and performing the join locally.  Some cases
      are not handled yet, such as SEMI and ANTI joins.  Also, we don't
      yet attempt to create presorted join paths or parameterized join
      paths even though these options do get tried for a base relation
      scan.  Nevertheless, this seems likely to be a very significant win
      in many practical cases.
      
      Shigeru Hanada and Ashutosh Bapat, reviewed by Robert Haas, with
      additional review at various points by Tom Lane, Etsuro Fujita,
      KaiGai Kohei, and Jeevan Chalke.
      e4106b25
  26. Feb 05, 2016
  27. Feb 03, 2016
    • Robert Haas's avatar
      Code review for commit dc203dc3. · 52b63649
      Robert Haas authored
      Remove duplicate assignment.  This part by Ashutosh Bapat.
      
      Remove now-obsolete comment.  This part by me, although the pending
      join pushdown patch does something similar, and for the same reason:
      there's no reason to keep two lists of the things in the fdw_private
      structure that have to be kept in sync with each other.
      52b63649
    • Robert Haas's avatar
      postgres_fdw: Allow fetch_size to be set per-table or per-server. · dc203dc3
      Robert Haas authored
      The default fetch size of 100 rows might not be right in every
      environment, so allow users to configure it.
      
      Corey Huinker, reviewed by Kyotaro Horiguchi, Andres Freund, and me.
      dc203dc3
  28. Jan 30, 2016
    • Robert Haas's avatar
      postgres_fdw: More preliminary refactoring for upcoming join pushdown. · cc592c48
      Robert Haas authored
      The code that generates a complete SQL query for a given foreign relation
      was repeated in two places, and they didn't quite agree: the EXPLAIN case
      left out the locking clause.  Centralize the code so we get the same
      behavior everywhere, and adjust calling conventions and which functions
      are static vs. extern accordingly .  Centralize the code so we get the same
      behavior everywhere, and adjust calling conventions and which functions
      are static vs. extern accordingly.
      
      Ashutosh Bapat, reviewed and slightly adjusted by me.
      cc592c48
  29. Jan 28, 2016
    • Robert Haas's avatar
      postgres_fdw: Refactor deparsing code for locking clauses. · b88ef201
      Robert Haas authored
      The upcoming patch to allow join pushdown in postgres_fdw needs to use
      this code multiple times, which requires moving it to deparse.c.  That
      seems like a good idea anyway, so do that now both on general principle
      and to simplify the future patch.
      
      Inspired by a patch by Shigeru Hanada and Ashutosh Bapat, but I did
      it a little differently than what that patch did.
      b88ef201
    • Robert Haas's avatar
      Avoid multiple foreign server connections when all use same user mapping. · 96198d94
      Robert Haas authored
      Previously, postgres_fdw's connection cache was keyed by user OID and
      server OID, but this can lead to multiple connections when it's not
      really necessary.  In particular, if all relevant users are mapped to
      the public user mapping, then their connection options are certainly
      the same, so one connection can be used for all of them.
      
      While we're cleaning things up here, drop the "server" argument to
      GetConnection(), which isn't really needed.  This saves a few cycles
      because callers no longer have to look this up; the function itself
      does, but only when establishing a new connection, not when reusing
      an existing one.
      
      Ashutosh Bapat, with a few small changes by me.
      96198d94
  30. Jan 02, 2016
  31. Dec 31, 2015
    • Tom Lane's avatar
      Add a comment noting that FDWs don't have to implement EXCEPT or LIMIT TO. · 5f36096b
      Tom Lane authored
      postgresImportForeignSchema pays attention to IMPORT's EXCEPT and LIMIT TO
      options, but only as an efficiency hack, not for correctness' sake.  The
      FDW documentation does explain that, but someone using postgres_fdw.c
      as a coding guide might not remember it, so let's add a comment here.
      Per question from Regina Obe.
      5f36096b
  32. Dec 22, 2015
    • Robert Haas's avatar
      postgres_fdw: Consider requesting sorted data so we can do a merge join. · ccd8f979
      Robert Haas authored
      When use_remote_estimate is enabled, consider adding ORDER BY to the
      query we sending to the remote server so that we can use that ordered
      data for a merge join.  Commit f18c944b
      arranges to push down the query pathkeys, which seems like the case
      mostly likely to be a win, but testing shows this can sometimes win,
      too.
      
      For a regular table, we know which indexes are present and therefore
      test whether the ordering provided by each such index is useful.  Here,
      we take the opposite approach: guess what orderings would be useful if
      they could be generated cheaply, and then ask the remote side what those
      will cost.
      
      Ashutosh Bapat, with very substantial cosmetic revisions by me.  Also
      reviewed by Rushabh Lathia.
      ccd8f979
  33. Dec 08, 2015
    • Robert Haas's avatar
      Allow foreign and custom joins to handle EvalPlanQual rechecks. · 385f337c
      Robert Haas authored
      Commit e7cb7ee1 provided basic
      infrastructure for allowing a foreign data wrapper or custom scan
      provider to replace a join of one or more tables with a scan.
      However, this infrastructure failed to take into account the need
      for possible EvalPlanQual rechecks, and ExecScanFetch would fail
      an assertion (or just overwrite memory) if such a check was attempted
      for a plan containing a pushed-down join.  To fix, adjust the EPQ
      machinery to skip some processing steps when scanrelid == 0, making
      those the responsibility of scan's recheck method, which also has
      the responsibility in this case of correctly populating the relevant
      slot.
      
      To allow foreign scans to gain control in the right place to make
      use of this new facility, add a new, optional RecheckForeignScan
      method.  Also, allow a foreign scan to have a child plan, which can
      be used to correctly populate the slot (or perhaps for something
      else, but this is the only use currently envisioned).
      
      KaiGai Kohei, reviewed by Robert Haas, Etsuro Fujita, and Kyotaro
      Horiguchi.
      385f337c
  34. Nov 18, 2015
  35. Nov 04, 2015
    • Tom Lane's avatar
      Allow postgres_fdw to ship extension funcs/operators for remote execution. · d8949416
      Tom Lane authored
      The user can whitelist specified extension(s) in the foreign server's
      options, whereupon we will treat immutable functions and operators of those
      extensions as candidates to be sent for remote execution.
      
      Whitelisting an extension in this way basically promises that the extension
      exists on the remote server and behaves compatibly with the local instance.
      We have no way to prove that formally, so we have to rely on the user to
      get it right.  But this seems like something that people can usually get
      right in practice.
      
      We might in future allow functions and operators to be whitelisted
      individually, but extension granularity is a very convenient special case,
      so it got done first.
      
      The patch as-committed lacks any regression tests, which is unfortunate,
      but introducing dependencies on other extensions for testing purposes
      would break "make installcheck" scenarios, which is worse.  I have some
      ideas about klugy ways around that, but it seems like material for a
      separate patch.  For the moment, leave the problem open.
      
      Paul Ramsey, hacked up a bit more by me
      d8949416
Loading