From 6f6cb1a0d622ed1a26e199f84c3541f4a901176e Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Sat, 3 Sep 2016 12:53:13 +0200 Subject: [PATCH] Allow noop updates of the partition column --- .../planner/multi_router_planner.c | 61 +++++++++++++++++-- .../regress/expected/multi_modifications.out | 6 ++ src/test/regress/sql/multi_modifications.sql | 6 ++ 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/backend/distributed/planner/multi_router_planner.c b/src/backend/distributed/planner/multi_router_planner.c index 1c54e58bb..151bd040b 100644 --- a/src/backend/distributed/planner/multi_router_planner.c +++ b/src/backend/distributed/planner/multi_router_planner.c @@ -42,6 +42,7 @@ #include "nodes/pg_list.h" #include "nodes/primnodes.h" #include "optimizer/clauses.h" +#include "optimizer/predtest.h" #include "optimizer/restrictinfo.h" #include "parser/parsetree.h" #include "storage/lock.h" @@ -67,6 +68,8 @@ static bool MasterIrreducibleExpression(Node *expression, bool *varArgument, bool *badCoalesce); static bool MasterIrreducibleExpressionWalker(Node *expression, WalkerState *state); static char MostPermissiveVolatileFlag(char left, char right); +static bool TargetEntryChangesValue(TargetEntry *targetEntry, Var *column, + FromExpr *joinTree); static Task * RouterModifyTask(Query *originalQuery, Query *query); static ShardInterval * TargetShardIntervalForModify(Query *query); static List * QueryRestrictList(Query *query); @@ -286,7 +289,7 @@ ErrorIfModifyQueryNotSupported(Query *queryTree) { bool hasVarArgument = false; /* A STABLE function is passed a Var argument */ bool hasBadCoalesce = false; /* CASE/COALESCE passed a mutable function */ - FromExpr *joinTree = NULL; + FromExpr *joinTree = queryTree->jointree; ListCell *targetEntryCell = NULL; foreach(targetEntryCell, queryTree->targetList) @@ -308,12 +311,14 @@ ErrorIfModifyQueryNotSupported(Query *queryTree) } if (commandType == CMD_UPDATE && - targetEntry->resno == partitionColumn->varattno) + TargetEntryChangesValue(targetEntry, partitionColumn, + queryTree->jointree)) { specifiesPartitionValue = true; } - if (targetEntry->resno == partitionColumn->varattno && + if (commandType == CMD_INSERT && + targetEntry->resno == partitionColumn->varattno && !IsA(targetEntry->expr, Const)) { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -329,7 +334,6 @@ ErrorIfModifyQueryNotSupported(Query *queryTree) } } - joinTree = queryTree->jointree; if (joinTree != NULL) { if (contain_volatile_functions(joinTree->quals)) @@ -680,6 +684,55 @@ MostPermissiveVolatileFlag(char left, char right) } +/* + * TargetEntryChangesValue determines whether the given target entry may + * change the value in a given column, given a join tree. The result is + * true unless the expression refers directly to the column, or the + * expression is a value that is implied by the qualifiers of the join + * tree, or the target entry sets a different column. + */ +static bool +TargetEntryChangesValue(TargetEntry *targetEntry, Var *column, FromExpr *joinTree) +{ + bool isColumnValueChanged = true; + Expr *setExpr = targetEntry->expr; + + if (targetEntry->resno != column->varattno) + { + /* target entry of the form SET some_other_col = */ + isColumnValueChanged = false; + } + else if (IsA(setExpr, Var)) + { + Var *newValue = (Var *) setExpr; + if (newValue->varattno == column->varattno) + { + /* target entry of the form SET col = table.col */ + isColumnValueChanged = false; + } + } + else if (IsA(setExpr, Const)) + { + Const *newValue = (Const *) setExpr; + List *restrictClauseList = WhereClauseList(joinTree); + OpExpr *equalityExpr = MakeOpExpression(column, BTEqualStrategyNumber); + Const *rightConst = (Const *) get_rightop((Expr *) equalityExpr); + + rightConst->constvalue = newValue->constvalue; + rightConst->constisnull = newValue->constisnull; + rightConst->constbyval = newValue->constbyval; + + if (predicate_implied_by(list_make1(equalityExpr), restrictClauseList)) + { + /* target entry of the form SET col = WHERE col = AND ... */ + isColumnValueChanged = false; + } + } + + return isColumnValueChanged; +} + + /* * RouterModifyTask builds a Task to represent a modification performed by * the provided query against the provided shard interval. This task contains diff --git a/src/test/regress/expected/multi_modifications.out b/src/test/regress/expected/multi_modifications.out index 49409a080..1f2f6c6ba 100644 --- a/src/test/regress/expected/multi_modifications.out +++ b/src/test/regress/expected/multi_modifications.out @@ -396,6 +396,12 @@ ERROR: distributed modifications must target exactly one shard -- attempting to change the partition key is unsupported UPDATE limit_orders SET id = 0 WHERE id = 246; ERROR: modifying the partition value of rows is not allowed +UPDATE limit_orders SET id = 0 WHERE id = 0 OR id = 246; +ERROR: modifying the partition value of rows is not allowed +-- setting the partition column value to itself is allowed +UPDATE limit_orders SET id = 246 WHERE id = 246; +UPDATE limit_orders SET id = 246 WHERE id = 246 AND symbol = 'GM'; +UPDATE limit_orders SET id = limit_orders.id WHERE id = 246; -- UPDATEs with a FROM clause are unsupported UPDATE limit_orders SET limit_price = 0.00 FROM bidders WHERE limit_orders.id = 246 AND diff --git a/src/test/regress/sql/multi_modifications.sql b/src/test/regress/sql/multi_modifications.sql index 03decd887..88eff899f 100644 --- a/src/test/regress/sql/multi_modifications.sql +++ b/src/test/regress/sql/multi_modifications.sql @@ -284,6 +284,12 @@ UPDATE limit_orders SET limit_price = 0.00; -- attempting to change the partition key is unsupported UPDATE limit_orders SET id = 0 WHERE id = 246; +UPDATE limit_orders SET id = 0 WHERE id = 0 OR id = 246; + +-- setting the partition column value to itself is allowed +UPDATE limit_orders SET id = 246 WHERE id = 246; +UPDATE limit_orders SET id = 246 WHERE id = 246 AND symbol = 'GM'; +UPDATE limit_orders SET id = limit_orders.id WHERE id = 246; -- UPDATEs with a FROM clause are unsupported UPDATE limit_orders SET limit_price = 0.00 FROM bidders