Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP Fix issue #7674 about UPDATE SET(..), with indirection #7675

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 221 additions & 0 deletions src/backend/distributed/deparser/citus_ruleutils.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "commands/sequence.h"
#include "foreign/foreign.h"
#include "lib/stringinfo.h"
#include "miscadmin.h"
#include "nodes/nodeFuncs.h"
#include "nodes/nodes.h"
#include "nodes/parsenodes.h"
Expand Down Expand Up @@ -1638,3 +1639,223 @@
}
}
}


/* Function to extract paramid from a FuncExpr node */
static AttrNumber
extract_paramid_from_funcexpr(FuncExpr *func)

Check warning on line 1646 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1646

Added line #L1646 was not covered by tests
{
AttrNumber targetAttnum = InvalidAttrNumber;
ListCell *lc;

Check warning on line 1649 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1648-L1649

Added lines #L1648 - L1649 were not covered by tests

/* Iterate through the arguments of the FuncExpr */
foreach(lc, func->args)

Check warning on line 1652 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1652

Added line #L1652 was not covered by tests
{
Node *arg = (Node *) lfirst(lc);

Check warning on line 1654 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1654

Added line #L1654 was not covered by tests

/* Check if the argument is a PARAM node */
if (IsA(arg, Param))
{
Param *param = (Param *) arg;
targetAttnum = param->paramid;

Check warning on line 1660 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1659-L1660

Added lines #L1659 - L1660 were not covered by tests

break; /* Exit loop once we find the PARAM node */

Check warning on line 1662 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1662

Added line #L1662 was not covered by tests
}
}

return targetAttnum;

Check warning on line 1666 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1666

Added line #L1666 was not covered by tests
}


/*
* processTargetsIndirection - reorder targets list (from indirection)
*
* We don't change anything but the order of the target list.
* The purpose here is to be able to deparse a query tree as if it was
* provided by the PostgreSQL parser, not the rewriter (which is the one
* received by the planner hook).
*
* It's required only for UPDATE SET (MULTIEXPR) queries at the moment, other
* candidates are not supported by Citus.
*/
static void
processTargetsIndirection(List **targetList)

Check warning on line 1682 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1682

Added line #L1682 was not covered by tests
{
int nAssignableCols;
int targetListPosition;
bool sawJunk = false;
List *newTargetList = NIL;
ListCell *lc;

Check warning on line 1688 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1684-L1688

Added lines #L1684 - L1688 were not covered by tests

/* Count non-junk columns and ensure they precede junk columns */
nAssignableCols = 0;
foreach(lc, *targetList)

Check warning on line 1692 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1691-L1692

Added lines #L1691 - L1692 were not covered by tests
{
TargetEntry *tle = lfirst_node(TargetEntry, lc);

Check warning on line 1694 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1694

Added line #L1694 was not covered by tests

if (tle->resjunk)
{
sawJunk = true;
}
else
{
if (sawJunk)
{
elog(ERROR, "Subplan target list is out of order");

Check warning on line 1704 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1704

Added line #L1704 was not covered by tests
}

nAssignableCols++;

Check warning on line 1707 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1707

Added line #L1707 was not covered by tests
}
}

/* If no assignable columns, return the original target list */
if (nAssignableCols == 0)
{
return;
}

/* Reorder the target list */
/* we start from 1 */
targetListPosition = 1;
while (nAssignableCols > 0)
{
nAssignableCols--;

Check warning on line 1722 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1722

Added line #L1722 was not covered by tests

foreach(lc, *targetList)

Check warning on line 1724 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1724

Added line #L1724 was not covered by tests
{
TargetEntry *tle = lfirst_node(TargetEntry, lc);

Check warning on line 1726 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1726

Added line #L1726 was not covered by tests

if (IsA(tle->expr, FuncExpr))
{
FuncExpr *funcexpr = (FuncExpr *) tle->expr;
AttrNumber attnum = extract_paramid_from_funcexpr(funcexpr);

Check warning on line 1731 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1730-L1731

Added lines #L1730 - L1731 were not covered by tests

if (attnum == targetListPosition)
{
ereport(DEBUG1, (errmsg("Adding FuncExpr resno: %d", tle->resno)));
newTargetList = lappend(newTargetList, tle);
targetListPosition++;
break;

Check warning on line 1738 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1735-L1738

Added lines #L1735 - L1738 were not covered by tests
}
}
else if (IsA(tle->expr, Param))

Check warning on line 1741 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1741

Added line #L1741 was not covered by tests
{
Param *param = (Param *) tle->expr;
AttrNumber attnum = param->paramid;

Check warning on line 1744 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1743-L1744

Added lines #L1743 - L1744 were not covered by tests

if (attnum == targetListPosition)
{
newTargetList = lappend(newTargetList, tle);
targetListPosition++;
break;

Check warning on line 1750 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1748-L1750

Added lines #L1748 - L1750 were not covered by tests
}
}
}
}

/* Append any remaining junk columns */
foreach(lc, *targetList)

Check warning on line 1757 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1757

Added line #L1757 was not covered by tests
{
TargetEntry *tle = lfirst_node(TargetEntry, lc);

Check warning on line 1759 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1759

Added line #L1759 was not covered by tests
if (tle->resjunk)
{
newTargetList = lappend(newTargetList, tle);

Check warning on line 1762 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1762

Added line #L1762 was not covered by tests
}
}
*targetList = newTargetList;

Check warning on line 1765 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1765

Added line #L1765 was not covered by tests
}


/*
* helper function to evaluate if we are in an SET (...)
* Caller is responsible to check the command type (UPDATE)
*/
static inline bool
is_update_set_multiexpr(Query *query)
{
ListCell *lc;

/* we're only interested by UPDATE */
if (query->commandType != CMD_UPDATE)
{
return false;
}

/*
* Then foreach target entry, check if one of the node or it's descendant
* is a PARAM_MULTIEXPR (i.e. a SET (a, b) = (...))
*/
foreach(lc, query->targetList)
{
TargetEntry *tle = (TargetEntry *) lfirst(lc);
Node *expr;

if (tle->resjunk)
{
continue;

Check warning on line 1795 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1795

Added line #L1795 was not covered by tests
}

expr = strip_implicit_coercions((Node *) tle->expr);

if (expr && IsA(expr, Param) &&
((Param *) expr)->paramkind == PARAM_MULTIEXPR)
{
return true;

Check warning on line 1803 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1803

Added line #L1803 was not covered by tests
}
}

/* No multi-column set expression found */
return false;
}


/*
* helper function to evaluate if we are in SELECT with CTE.
*/
static inline bool
is_select_cte(Query *query)
{
/* we are only looking for a SELECT query */
if (query->commandType != CMD_SELECT)
{
return false;
}

/* and it must contain CTE */
if (query->cteList == NIL)
{
return false;
}
return true;
}


/*
* We may need to reorder parts of the planner tree we are receiving here.
* We expect to produce an SQL query text but our tree has been optimized by
* PostgreSL rewriter already...
*/
void
RebuildParserTreeFromPlannerTree(Query *query)
{
/* Guard against excessively long or deeply-nested queries */
CHECK_FOR_INTERRUPTS();

/* prevent unloyal defeat */
check_stack_depth();

if (is_update_set_multiexpr(query))
{
processTargetsIndirection(&query->targetList);

Check warning on line 1849 in src/backend/distributed/deparser/citus_ruleutils.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/deparser/citus_ruleutils.c#L1849

Added line #L1849 was not covered by tests
}
/* also match UPDATE in CTE, this one is recursive */
else if (is_select_cte(query))
{
ListCell *lc;
foreach(lc, query->cteList)
{
CommonTableExpr *cte = (CommonTableExpr *) lfirst(lc);
RebuildParserTreeFromPlannerTree((Query *) cte->ctequery);
}
}
}
1 change: 0 additions & 1 deletion src/backend/distributed/planner/distributed_planner.c
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ distributed_planner(Query *parse,

planContext.originalQuery = copyObject(parse);


if (!fastPathRouterQuery)
{
/*
Expand Down
7 changes: 7 additions & 0 deletions src/backend/distributed/planner/multi_router_planner.c
Original file line number Diff line number Diff line change
Expand Up @@ -1869,6 +1869,13 @@ RouterJob(Query *originalQuery, PlannerRestrictionContext *plannerRestrictionCon
}
else
{
/*
* We may need to reorder parts of the planner tree we are receiving here.
* We expect to produce an SQL query text but our tree has been optimized by
* PostgreSL rewriter already...
* FIXME is there conditions to reduce the number of calls ?
*/
RebuildParserTreeFromPlannerTree(originalQuery);
(*planningError) = PlanRouterQuery(originalQuery, plannerRestrictionContext,
&placementList, &shardId, &relationShardList,
&prunedShardIntervalListList,
Expand Down
2 changes: 2 additions & 0 deletions src/include/distributed/citus_ruleutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,7 @@ extern char * generate_operator_name(Oid operid, Oid arg1, Oid arg2);
extern List * getOwnedSequences_internal(Oid relid, AttrNumber attnum, char deptype);
extern void AppendOptionListToString(StringInfo stringData, List *options);

extern void RebuildParserTreeFromPlannerTree(Query *query);


#endif /* CITUS_RULEUTILS_H */
Loading
Loading