From b88ef201d46e6519b5e0589358c952a4c0f5bf0f Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Thu, 28 Jan 2016 16:44:01 -0500 Subject: [PATCH] postgres_fdw: Refactor deparsing code for locking clauses. 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. --- contrib/postgres_fdw/deparse.c | 60 +++++++++++++++++++++++++++++ contrib/postgres_fdw/postgres_fdw.c | 51 +----------------------- contrib/postgres_fdw/postgres_fdw.h | 2 + 3 files changed, 64 insertions(+), 49 deletions(-) diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index e59af2c8e9..e577a03956 100644 --- a/contrib/postgres_fdw/deparse.c +++ b/contrib/postgres_fdw/deparse.c @@ -45,7 +45,9 @@ #include "catalog/pg_type.h" #include "commands/defrem.h" #include "nodes/nodeFuncs.h" +#include "nodes/plannodes.h" #include "optimizer/clauses.h" +#include "optimizer/prep.h" #include "optimizer/var.h" #include "parser/parsetree.h" #include "utils/builtins.h" @@ -807,6 +809,64 @@ deparseTargetList(StringInfo buf, appendStringInfoString(buf, "NULL"); } +/* + * Deparse the appropriate locking clause (FOR SELECT or FOR SHARE) for a + * given relation. + */ +void +deparseLockingClause(StringInfo buf, PlannerInfo *root, RelOptInfo *rel) +{ + /* + * Add FOR UPDATE/SHARE if appropriate. We apply locking during the + * initial row fetch, rather than later on as is done for local tables. + * The extra roundtrips involved in trying to duplicate the local + * semantics exactly don't seem worthwhile (see also comments for + * RowMarkType). + * + * Note: because we actually run the query as a cursor, this assumes that + * DECLARE CURSOR ... FOR UPDATE is supported, which it isn't before 8.3. + */ + if (rel->relid == root->parse->resultRelation && + (root->parse->commandType == CMD_UPDATE || + root->parse->commandType == CMD_DELETE)) + { + /* Relation is UPDATE/DELETE target, so use FOR UPDATE */ + appendStringInfoString(buf, " FOR UPDATE"); + } + else + { + PlanRowMark *rc = get_plan_rowmark(root->rowMarks, rel->relid); + + if (rc) + { + /* + * Relation is specified as a FOR UPDATE/SHARE target, so handle + * that. (But we could also see LCS_NONE, meaning this isn't a + * target relation after all.) + * + * For now, just ignore any [NO] KEY specification, since (a) it's + * not clear what that means for a remote table that we don't have + * complete information about, and (b) it wouldn't work anyway on + * older remote servers. Likewise, we don't worry about NOWAIT. + */ + switch (rc->strength) + { + case LCS_NONE: + /* No locking needed */ + break; + case LCS_FORKEYSHARE: + case LCS_FORSHARE: + appendStringInfoString(buf, " FOR SHARE"); + break; + case LCS_FORNOKEYUPDATE: + case LCS_FORUPDATE: + appendStringInfoString(buf, " FOR UPDATE"); + break; + } + } + } +} + /* * Deparse WHERE clauses in given list of RestrictInfos and append them to buf. * diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index a237e152c0..0aa7fbeac0 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -1013,55 +1013,8 @@ postgresGetForeignPlan(PlannerInfo *root, if (best_path->path.pathkeys) appendOrderByClause(&sql, root, baserel, best_path->path.pathkeys); - /* - * Add FOR UPDATE/SHARE if appropriate. We apply locking during the - * initial row fetch, rather than later on as is done for local tables. - * The extra roundtrips involved in trying to duplicate the local - * semantics exactly don't seem worthwhile (see also comments for - * RowMarkType). - * - * Note: because we actually run the query as a cursor, this assumes that - * DECLARE CURSOR ... FOR UPDATE is supported, which it isn't before 8.3. - */ - if (baserel->relid == root->parse->resultRelation && - (root->parse->commandType == CMD_UPDATE || - root->parse->commandType == CMD_DELETE)) - { - /* Relation is UPDATE/DELETE target, so use FOR UPDATE */ - appendStringInfoString(&sql, " FOR UPDATE"); - } - else - { - PlanRowMark *rc = get_plan_rowmark(root->rowMarks, baserel->relid); - - if (rc) - { - /* - * Relation is specified as a FOR UPDATE/SHARE target, so handle - * that. (But we could also see LCS_NONE, meaning this isn't a - * target relation after all.) - * - * For now, just ignore any [NO] KEY specification, since (a) it's - * not clear what that means for a remote table that we don't have - * complete information about, and (b) it wouldn't work anyway on - * older remote servers. Likewise, we don't worry about NOWAIT. - */ - switch (rc->strength) - { - case LCS_NONE: - /* No locking needed */ - break; - case LCS_FORKEYSHARE: - case LCS_FORSHARE: - appendStringInfoString(&sql, " FOR SHARE"); - break; - case LCS_FORNOKEYUPDATE: - case LCS_FORUPDATE: - appendStringInfoString(&sql, " FOR UPDATE"); - break; - } - } - } + /* Add any necessary FOR UPDATE/SHARE. */ + deparseLockingClause(&sql, root, baserel); /* * Build the fdw_private list that will be available to the executor. diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h index 59e9f60c04..0d8c271505 100644 --- a/contrib/postgres_fdw/postgres_fdw.h +++ b/contrib/postgres_fdw/postgres_fdw.h @@ -88,6 +88,8 @@ extern void deparseSelectSql(StringInfo buf, RelOptInfo *baserel, Bitmapset *attrs_used, List **retrieved_attrs); +extern void deparseLockingClause(StringInfo buf, + PlannerInfo *root, RelOptInfo *rel); extern void appendWhereClause(StringInfo buf, PlannerInfo *root, RelOptInfo *baserel,