Skip to content

Commit

Permalink
Fix #7684 : order "planner" tree
Browse files Browse the repository at this point in the history
Similar to other bugs affected by citus usage of ruletuils: the provided
tree as input is distinct from the tree expected in ruleutils in
PostgreSQL.
As a consequence, it is required in some places to re-order the tree
structure to make it compliant or "back" to the parser tree (before it's
rewritten and reordered by PostgreSQL to optimize execution). Or to
lookup the target list and ensure it's the one "we expect".
Fox this bug, the `get_rule_sortgroupclause()` is used to check if
target list entry `resname` is defined, and use it directly if it
exists.

No benchmark where run, it's not expected to impact a lot.
  • Loading branch information
c2main committed Sep 25, 2024
1 parent 5bad6c6 commit 8011824
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 9 deletions.
18 changes: 15 additions & 3 deletions src/backend/distributed/deparser/ruleutils_14.c
Original file line number Diff line number Diff line change
Expand Up @@ -2462,9 +2462,21 @@ get_basic_select_query(Query *query, deparse_context *context,
{
SortGroupClause *srt = (SortGroupClause *) lfirst(l);

appendStringInfoString(buf, sep);
get_rule_sortgroupclause(srt->tleSortGroupRef, query->targetList,
false, context);
/* Patch: Ensure the correct target list is used without alias swapping */
TargetEntry *tle = get_sortgroupclause_tle(srt, query->targetList);

/* Ensure the attribute names are preserved */
if (tle && tle->resname != NULL)
{
appendStringInfoString(buf, sep);
appendStringInfoString(buf, quote_identifier(tle->resname));
}
else
{
/* Fallback to the default behavior */
get_rule_sortgroupclause(srt->tleSortGroupRef, query->targetList, false, context);
}

sep = ", ";
}
appendStringInfoChar(buf, ')');
Expand Down
18 changes: 15 additions & 3 deletions src/backend/distributed/deparser/ruleutils_15.c
Original file line number Diff line number Diff line change
Expand Up @@ -2551,9 +2551,21 @@ get_basic_select_query(Query *query, deparse_context *context,
{
SortGroupClause *srt = (SortGroupClause *) lfirst(l);

appendStringInfoString(buf, sep);
get_rule_sortgroupclause(srt->tleSortGroupRef, query->targetList,
false, context);
/* Patch: Ensure the correct target list is used without alias swapping */
TargetEntry *tle = get_sortgroupclause_tle(srt, query->targetList);

/* Ensure the attribute names are preserved */
if (tle && tle->resname != NULL)
{
appendStringInfoString(buf, sep);
appendStringInfoString(buf, quote_identifier(tle->resname));
}
else
{
/* Fallback to the default behavior */
get_rule_sortgroupclause(srt->tleSortGroupRef, query->targetList, false, context);
}

sep = ", ";
}
appendStringInfoChar(buf, ')');
Expand Down
18 changes: 15 additions & 3 deletions src/backend/distributed/deparser/ruleutils_16.c
Original file line number Diff line number Diff line change
Expand Up @@ -2565,9 +2565,21 @@ get_basic_select_query(Query *query, deparse_context *context,
{
SortGroupClause *srt = (SortGroupClause *) lfirst(l);

appendStringInfoString(buf, sep);
get_rule_sortgroupclause(srt->tleSortGroupRef, query->targetList,
false, context);
/* Patch: Ensure the correct target list is used without alias swapping */
TargetEntry *tle = get_sortgroupclause_tle(srt, query->targetList);

/* Ensure the attribute names are preserved */
if (tle && tle->resname != NULL)
{
appendStringInfoString(buf, sep);
appendStringInfoString(buf, quote_identifier(tle->resname));
}
else
{
/* Fallback to the default behavior */
get_rule_sortgroupclause(srt->tleSortGroupRef, query->targetList, false, context);
}

sep = ", ";
}
appendStringInfoChar(buf, ')');
Expand Down

0 comments on commit 8011824

Please sign in to comment.