external
/
citus
mirror of https://github.com/citusdata/citus.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix mis-deparsing of shard query in "output-table column" name conflict (#7932) 

DESCRIPTION: Fixes a bug in deparsing of shard query in case of
"output-table column" name conflict

If an `ORDER BY` item in `SELECT` is a bare identifier, the parser
_first seeks it as an output column name_ of the `SELECT` (for SQL92
compatibility).  However, ruleutils.c is expecting the SQL99
interpretation _where such a name is an input column name_.  So it's
possible to produce an incorrect display of a view in the (admittedly
pretty ill-advised) case where some other column is renamed in the
`SELECT` output list to match an `ORDER BY` column.

The `DISTINCT ON` expressions are interpreted using the same rules as
for `ORDER BY`.
We had an issue reported that actually uses `DISTINCT ON`: #7684 
Since Citus uses ruleutils deparsing logic to create the shard queries,
it would not
table-qualify the column names as needed.

PG17 fixed this https://github.com/postgres/postgres/commit/a7eb633563c
by table-qualifying such names in the dumped view text. Therefore,
Citus doesn't reproduce the issue in PG17, since PG17 table-qualifies
the column names when needed, and the produced shard queries are
correct.

This PR applies the PG17 patch to `ruleutils_15.c` and `ruleutils_16.c`.
Even though we generally try to avoid modifying the ruleutils files, in
this case
we are applying a Postgres patch that `ruleutils_17.c` already has:

897d996b8f

Thanks @c2main for your discussion and idea in the issue.
Fixes #7684
pull/7935/head
Naisila Puka 2025-03-19 14:21:30 +03:00 committed by GitHub
parent 1c09469dd2
commit 4b4fa22b64
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 419 additions and 186 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

223
src/backend/distributed/deparser/ruleutils_15.c
View File

@ -67,7 +67,6 @@
#include "parser/parse_node.h" #include "parser/parse_node.h"
#include "parser/parse_agg.h" #include "parser/parse_agg.h"
#include "parser/parse_func.h" #include "parser/parse_func.h"
#include "parser/parse_node.h"
#include "parser/parse_oper.h" #include "parser/parse_oper.h"
#include "parser/parse_relation.h" #include "parser/parse_relation.h"
#include "parser/parser.h" #include "parser/parser.h"
@ -123,16 +122,18 @@ typedef struct
{ {
StringInfo buf; /* output buffer to append to */ StringInfo buf; /* output buffer to append to */
List *namespaces; /* List of deparse_namespace nodes */ List *namespaces; /* List of deparse_namespace nodes */
TupleDesc resultDesc; /* if top level of a view, the view's tupdesc */
List *targetList; /* Current query level's SELECT targetlist */
List *windowClause; /* Current query level's WINDOW clause */ List *windowClause; /* Current query level's WINDOW clause */
List *windowTList; /* targetlist for resolving WINDOW clause */
int prettyFlags; /* enabling of pretty-print functions */ int prettyFlags; /* enabling of pretty-print functions */
int wrapColumn; /* max line length, or -1 for no limit */ int wrapColumn; /* max line length, or -1 for no limit */
int indentLevel; /* current indent level for prettyprint */ int indentLevel; /* current indent level for prettyprint */
bool varprefix; /* true to print prefixes on Vars */ bool varprefix; /* true to print prefixes on Vars */
Oid distrelid; /* the distributed table being modified, if valid */ Oid distrelid; /* the distributed table being modified, if valid */
int64 shardid; /* a distributed table's shardid, if positive */ int64 shardid; /* a distributed table's shardid, if positive */
ParseExprKind special_exprkind; /* set only for exprkinds needing special bool colNamesVisible; /* do we care about output column names? */
* handling */ bool inGroupBy; /* deparsing GROUP BY clause? */
bool varInOrderBy; /* deparsing simple Var in ORDER BY? */
Bitmapset *appendparents; /* if not null, map child Vars of these relids Bitmapset *appendparents; /* if not null, map child Vars of these relids
* back to the parent rel */ * back to the parent rel */
} deparse_context; } deparse_context;
@ -364,27 +365,19 @@ static void get_query_def_extended(Query *query, StringInfo buf,
int startIndent); int startIndent);
static void get_values_def(List *values_lists, deparse_context *context); static void get_values_def(List *values_lists, deparse_context *context);
static void get_with_clause(Query *query, deparse_context *context); static void get_with_clause(Query *query, deparse_context *context);
static void get_select_query_def(Query *query, deparse_context *context, static void get_select_query_def(Query *query, deparse_context *context);
TupleDesc resultDesc, bool colNamesVisible); static void get_insert_query_def(Query *query, deparse_context *context);
static void get_insert_query_def(Query *query, deparse_context *context, static void get_update_query_def(Query *query, deparse_context *context);
bool colNamesVisible);
static void get_update_query_def(Query *query, deparse_context *context,
bool colNamesVisible);
static void get_merge_query_def(Query *query, deparse_context *context);
static void get_update_query_targetlist_def(Query *query, List *targetList, static void get_update_query_targetlist_def(Query *query, List *targetList,
deparse_context *context, deparse_context *context,
RangeTblEntry *rte); RangeTblEntry *rte);
static void get_delete_query_def(Query *query, deparse_context *context, static void get_delete_query_def(Query *query, deparse_context *context);
bool colNamesVisible); static void get_merge_query_def(Query *query, deparse_context *context);
static void get_utility_query_def(Query *query, deparse_context *context); static void get_utility_query_def(Query *query, deparse_context *context);
static void get_basic_select_query(Query *query, deparse_context *context, static void get_basic_select_query(Query *query, deparse_context *context);
TupleDesc resultDesc, bool colNamesVisible); static void get_target_list(List *targetList, deparse_context *context);
static void get_target_list(List *targetList, deparse_context *context,
TupleDesc resultDesc, bool colNamesVisible);
static void get_setop_query(Node *setOp, Query *query, static void get_setop_query(Node *setOp, Query *query,
deparse_context *context, deparse_context *context);
TupleDesc resultDesc, bool colNamesVisible);
static Node *get_rule_sortgroupclause(Index ref, List *tlist, static Node *get_rule_sortgroupclause(Index ref, List *tlist,
bool force_colno, bool force_colno,
deparse_context *context); deparse_context *context);
@ -462,7 +455,7 @@ static char *generate_fragment_name(char *schemaName, char *tableName);
static char *generate_function_name(Oid funcid, int nargs, static char *generate_function_name(Oid funcid, int nargs,
List *argnames, Oid *argtypes, List *argnames, Oid *argtypes,
bool has_variadic, bool *use_variadic_p, bool has_variadic, bool *use_variadic_p,
ParseExprKind special_exprkind); bool inGroupBy);
static List *get_insert_column_names_list(List *targetList, StringInfo buf, deparse_context *context, RangeTblEntry *rte); static List *get_insert_column_names_list(List *targetList, StringInfo buf, deparse_context *context, RangeTblEntry *rte);
#define only_marker(rte) ((rte)->inh ? "" : "ONLY ") #define only_marker(rte) ((rte)->inh ? "" : "ONLY ")
@ -636,13 +629,16 @@ pg_get_rule_expr(Node *expression)
context.buf = buffer; context.buf = buffer;
context.namespaces = NIL; context.namespaces = NIL;
context.resultDesc = NULL;
context.targetList = NIL;
context.windowClause = NIL; context.windowClause = NIL;
context.windowTList = NIL;
context.varprefix = false; context.varprefix = false;
context.prettyFlags = 0; context.prettyFlags = 0;
context.wrapColumn = WRAP_COLUMN_DEFAULT; context.wrapColumn = WRAP_COLUMN_DEFAULT;
context.indentLevel = 0; context.indentLevel = 0;
context.special_exprkind = EXPR_KIND_NONE; context.colNamesVisible = true;
context.inGroupBy = false;
context.varInOrderBy = false;
context.distrelid = InvalidOid; context.distrelid = InvalidOid;
context.shardid = INVALID_SHARD_ID; context.shardid = INVALID_SHARD_ID;
@ -2066,14 +2062,17 @@ get_query_def_extended(Query *query, StringInfo buf, List *parentnamespace,
context.buf = buf; context.buf = buf;
context.namespaces = lcons(&dpns, list_copy(parentnamespace)); context.namespaces = lcons(&dpns, list_copy(parentnamespace));
context.resultDesc = NULL;
context.targetList = NIL;
context.windowClause = NIL; context.windowClause = NIL;
context.windowTList = NIL;
context.varprefix = (parentnamespace != NIL || context.varprefix = (parentnamespace != NIL ||
list_length(query->rtable) != 1); list_length(query->rtable) != 1);
context.prettyFlags = prettyFlags; context.prettyFlags = prettyFlags;
context.wrapColumn = wrapColumn; context.wrapColumn = wrapColumn;
context.indentLevel = startIndent; context.indentLevel = startIndent;
context.special_exprkind = EXPR_KIND_NONE; context.colNamesVisible = true;
context.inGroupBy = false;
context.varInOrderBy = false;
context.appendparents = NULL; context.appendparents = NULL;
context.distrelid = distrelid; context.distrelid = distrelid;
context.shardid = shardid; context.shardid = shardid;
@ -2083,19 +2082,21 @@ get_query_def_extended(Query *query, StringInfo buf, List *parentnamespace,
switch (query->commandType) switch (query->commandType)
{ {
case CMD_SELECT: case CMD_SELECT:
get_select_query_def(query, &context, resultDesc, colNamesVisible); /* We set context.resultDesc only if it's a SELECT */
context.resultDesc = resultDesc;
get_select_query_def(query, &context);
break; break;
case CMD_UPDATE: case CMD_UPDATE:
get_update_query_def(query, &context, colNamesVisible); get_update_query_def(query, &context);
break; break;
case CMD_INSERT: case CMD_INSERT:
get_insert_query_def(query, &context, colNamesVisible); get_insert_query_def(query, &context);
break; break;
case CMD_DELETE: case CMD_DELETE:
get_delete_query_def(query, &context, colNamesVisible); get_delete_query_def(query, &context);
break; break;
case CMD_MERGE: case CMD_MERGE:
@ -2307,23 +2308,18 @@ get_with_clause(Query *query, deparse_context *context)
* ---------- * ----------
*/ */
static void static void
get_select_query_def(Query *query, deparse_context *context, get_select_query_def(Query *query, deparse_context *context)
TupleDesc resultDesc, bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
List *save_windowclause;
List *save_windowtlist;
bool force_colno; bool force_colno;
ListCell *l; ListCell *l;
/* Insert the WITH clause if given */ /* Insert the WITH clause if given */
get_with_clause(query, context); get_with_clause(query, context);
/* Set up context for possible window functions */ /* Subroutines may need to consult the SELECT targetlist and windowClause */
save_windowclause = context->windowClause; context->targetList = query->targetList;
context->windowClause = query->windowClause; context->windowClause = query->windowClause;
save_windowtlist = context->windowTList;
context->windowTList = query->targetList;
/* /*
* If the Query node has a setOperations tree, then it's the top level of * If the Query node has a setOperations tree, then it's the top level of
@ -2332,14 +2328,13 @@ get_select_query_def(Query *query, deparse_context *context,
*/ */
if (query->setOperations) if (query->setOperations)
{ {
get_setop_query(query->setOperations, query, context, resultDesc, get_setop_query(query->setOperations, query, context);
colNamesVisible);
/* ORDER BY clauses must be simple in this case */ /* ORDER BY clauses must be simple in this case */
force_colno = true; force_colno = true;
} }
else else
{ {
get_basic_select_query(query, context, resultDesc, colNamesVisible); get_basic_select_query(query, context);
force_colno = false; force_colno = false;
} }
@ -2429,9 +2424,6 @@ get_select_query_def(Query *query, deparse_context *context,
appendStringInfoString(buf, " SKIP LOCKED"); appendStringInfoString(buf, " SKIP LOCKED");
} }
} }
context->windowClause = save_windowclause;
context->windowTList = save_windowtlist;
} }
/* /*
@ -2506,8 +2498,7 @@ get_simple_values_rte(Query *query, TupleDesc resultDesc)
} }
static void static void
get_basic_select_query(Query *query, deparse_context *context, get_basic_select_query(Query *query, deparse_context *context)
TupleDesc resultDesc, bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
RangeTblEntry *values_rte; RangeTblEntry *values_rte;
@ -2525,7 +2516,7 @@ get_basic_select_query(Query *query, deparse_context *context,
* VALUES part. This reverses what transformValuesClause() did at parse * VALUES part. This reverses what transformValuesClause() did at parse
* time. * time.
*/ */
values_rte = get_simple_values_rte(query, resultDesc); values_rte = get_simple_values_rte(query, context->resultDesc);
if (values_rte) if (values_rte)
{ {
get_values_def(values_rte->values_lists, context); get_values_def(values_rte->values_lists, context);
@ -2563,7 +2554,7 @@ get_basic_select_query(Query *query, deparse_context *context,
} }
/* Then we tell what to select (the targetlist) */ /* Then we tell what to select (the targetlist) */
get_target_list(query->targetList, context, resultDesc, colNamesVisible); get_target_list(query->targetList, context);
/* Add the FROM clause if needed */ /* Add the FROM clause if needed */
get_from_clause(query, " FROM ", context); get_from_clause(query, " FROM ", context);
@ -2579,15 +2570,15 @@ get_basic_select_query(Query *query, deparse_context *context,
/* Add the GROUP BY clause if given */ /* Add the GROUP BY clause if given */
if (query->groupClause != NULL || query->groupingSets != NULL) if (query->groupClause != NULL || query->groupingSets != NULL)
{ {
ParseExprKind save_exprkind; bool save_ingroupby;
appendContextKeyword(context, " GROUP BY ", appendContextKeyword(context, " GROUP BY ",
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1); -PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
if (query->groupDistinct) if (query->groupDistinct)
appendStringInfoString(buf, "DISTINCT "); appendStringInfoString(buf, "DISTINCT ");
save_exprkind = context->special_exprkind; save_ingroupby = context->inGroupBy;
context->special_exprkind = EXPR_KIND_GROUP_BY; context->inGroupBy = true;
if (query->groupingSets == NIL) if (query->groupingSets == NIL)
{ {
@ -2615,7 +2606,7 @@ get_basic_select_query(Query *query, deparse_context *context,
} }
} }
context->special_exprkind = save_exprkind; context->inGroupBy = save_ingroupby;
} }
/* Add the HAVING clause if given */ /* Add the HAVING clause if given */
@ -2634,14 +2625,11 @@ get_basic_select_query(Query *query, deparse_context *context,
/* ---------- /* ----------
* get_target_list - Parse back a SELECT target list * get_target_list - Parse back a SELECT target list
* *
* This is also used for RETURNING lists in INSERT/UPDATE/DELETE. * This is also used for RETURNING lists in INSERT/UPDATE/DELETE/MERGE.
*
* resultDesc and colNamesVisible are as for get_query_def()
* ---------- * ----------
*/ */
static void static void
get_target_list(List *targetList, deparse_context *context, get_target_list(List *targetList, deparse_context *context)
TupleDesc resultDesc, bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
StringInfoData targetbuf; StringInfoData targetbuf;
@ -2698,7 +2686,7 @@ get_target_list(List *targetList, deparse_context *context,
* assigned column name explicitly. Otherwise, show it only if * assigned column name explicitly. Otherwise, show it only if
* it's not FigureColname's fallback. * it's not FigureColname's fallback.
*/ */
attname = colNamesVisible ? NULL : "?column?"; attname = context->colNamesVisible ? NULL : "?column?";
} }
/* /*
@ -2707,8 +2695,9 @@ get_target_list(List *targetList, deparse_context *context,
* effects of any column RENAME that's been done on the view). * effects of any column RENAME that's been done on the view).
* Otherwise, just use what we can find in the TLE. * Otherwise, just use what we can find in the TLE.
*/ */
if (resultDesc && colno <= resultDesc->natts) if (context->resultDesc && colno <= context->resultDesc->natts)
colname = NameStr(TupleDescAttr(resultDesc, colno - 1)->attname); colname = NameStr(TupleDescAttr(context->resultDesc,
colno - 1)->attname);
else else
colname = tle->resname; colname = tle->resname;
@ -2776,8 +2765,7 @@ get_target_list(List *targetList, deparse_context *context,
} }
static void static void
get_setop_query(Node *setOp, Query *query, deparse_context *context, get_setop_query(Node *setOp, Query *query, deparse_context *context)
TupleDesc resultDesc, bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
bool need_paren; bool need_paren;
@ -2802,8 +2790,8 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
subquery->limitCount); subquery->limitCount);
if (need_paren) if (need_paren)
appendStringInfoChar(buf, '('); appendStringInfoChar(buf, '(');
get_query_def(subquery, buf, context->namespaces, resultDesc, get_query_def(subquery, buf, context->namespaces,
colNamesVisible, context->resultDesc, context->colNamesVisible,
context->prettyFlags, context->wrapColumn, context->prettyFlags, context->wrapColumn,
context->indentLevel); context->indentLevel);
if (need_paren) if (need_paren)
@ -2813,6 +2801,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
{ {
SetOperationStmt *op = (SetOperationStmt *) setOp; SetOperationStmt *op = (SetOperationStmt *) setOp;
int subindent; int subindent;
bool save_colnamesvisible;
/* /*
* We force parens when nesting two SetOperationStmts, except when the * We force parens when nesting two SetOperationStmts, except when the
@ -2846,7 +2835,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
else else
subindent = 0; subindent = 0;
get_setop_query(op->larg, query, context, resultDesc, colNamesVisible); get_setop_query(op->larg, query, context);
if (need_paren) if (need_paren)
appendContextKeyword(context, ") ", -subindent, 0, 0); appendContextKeyword(context, ") ", -subindent, 0, 0);
@ -2890,7 +2879,13 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
subindent = 0; subindent = 0;
appendContextKeyword(context, "", subindent, 0, 0); appendContextKeyword(context, "", subindent, 0, 0);
get_setop_query(op->rarg, query, context, resultDesc, false); /*
* The output column names of the RHS sub-select don't matter.
*/
save_colnamesvisible = context->colNamesVisible;
context->colNamesVisible = false;
get_setop_query(op->rarg, query, context);
context->colNamesVisible = save_colnamesvisible;
if (PRETTY_INDENT(context)) if (PRETTY_INDENT(context))
context->indentLevel -= subindent; context->indentLevel -= subindent;
@ -2924,20 +2919,31 @@ get_rule_sortgroupclause(Index ref, List *tlist, bool force_colno,
* Use column-number form if requested by caller. Otherwise, if * Use column-number form if requested by caller. Otherwise, if
* expression is a constant, force it to be dumped with an explicit cast * expression is a constant, force it to be dumped with an explicit cast
* as decoration --- this is because a simple integer constant is * as decoration --- this is because a simple integer constant is
* ambiguous (and will be misinterpreted by findTargetlistEntry()) if we * ambiguous (and will be misinterpreted by findTargetlistEntrySQL92()) if
* dump it without any decoration. If it's anything more complex than a * we dump it without any decoration. Similarly, if it's just a Var,
* simple Var, then force extra parens around it, to ensure it can't be * there is risk of misinterpretation if the column name is reassigned in
* misinterpreted as a cube() or rollup() construct. * the SELECT list, so we may need to force table qualification. And, if
* it's anything more complex than a simple Var, then force extra parens
* around it, to ensure it can't be misinterpreted as a cube() or rollup()
* construct.
*/ */
if (force_colno) if (force_colno)
{ {
Assert(!tle->resjunk); Assert(!tle->resjunk);
appendStringInfo(buf, "%d", tle->resno); appendStringInfo(buf, "%d", tle->resno);
} }
else if (expr && IsA(expr, Const)) else if (!expr)
/* do nothing, probably can't happen */ ;
else if (IsA(expr, Const))
get_const_expr((Const *) expr, context, 1); get_const_expr((Const *) expr, context, 1);
else if (!expr || IsA(expr, Var)) else if (IsA(expr, Var))
get_rule_expr(expr, context, true); {
/* Tell get_variable to check for name conflict */
bool save_varinorderby = context->varInOrderBy;
context->varInOrderBy = true;
(void) get_variable((Var *) expr, 0, false, context);
context->varInOrderBy = save_varinorderby;
}
else else
{ {
/* /*
@ -3225,8 +3231,7 @@ get_rule_windowspec(WindowClause *wc, List *targetList,
* ---------- * ----------
*/ */
static void static void
get_insert_query_def(Query *query, deparse_context *context, get_insert_query_def(Query *query, deparse_context *context)
bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
RangeTblEntry *select_rte = NULL; RangeTblEntry *select_rte = NULL;
@ -3405,7 +3410,7 @@ get_insert_query_def(Query *query, deparse_context *context,
{ {
appendContextKeyword(context, " RETURNING", appendContextKeyword(context, " RETURNING",
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1); -PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
get_target_list(query->returningList, context, NULL, colNamesVisible); get_target_list(query->returningList, context);
} }
} }
@ -3414,8 +3419,7 @@ get_insert_query_def(Query *query, deparse_context *context,
* ---------- * ----------
*/ */
static void static void
get_update_query_def(Query *query, deparse_context *context, get_update_query_def(Query *query, deparse_context *context)
bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
RangeTblEntry *rte; RangeTblEntry *rte;
@ -3485,7 +3489,7 @@ get_update_query_def(Query *query, deparse_context *context,
{ {
appendContextKeyword(context, " RETURNING", appendContextKeyword(context, " RETURNING",
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1); -PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
get_target_list(query->returningList, context, NULL, colNamesVisible); get_target_list(query->returningList, context);
} }
} }
@ -3645,8 +3649,7 @@ get_update_query_targetlist_def(Query *query, List *targetList,
* ---------- * ----------
*/ */
static void static void
get_delete_query_def(Query *query, deparse_context *context, get_delete_query_def(Query *query, deparse_context *context)
bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
RangeTblEntry *rte; RangeTblEntry *rte;
@ -3711,7 +3714,7 @@ get_delete_query_def(Query *query, deparse_context *context,
{ {
appendContextKeyword(context, " RETURNING", appendContextKeyword(context, " RETURNING",
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1); -PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
get_target_list(query->returningList, context, NULL, colNamesVisible); get_target_list(query->returningList, context);
} }
} }
@ -3963,6 +3966,7 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
deparse_columns *colinfo; deparse_columns *colinfo;
char *refname; char *refname;
char *attname; char *attname;
bool need_prefix;
/* Find appropriate nesting depth */ /* Find appropriate nesting depth */
netlevelsup = var->varlevelsup + levelsup; netlevelsup = var->varlevelsup + levelsup;
@ -4163,7 +4167,42 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
attname = get_rte_attribute_name(rte, attnum); attname = get_rte_attribute_name(rte, attnum);
} }
if (refname && (context->varprefix || attname == NULL)) need_prefix = (context->varprefix || attname == NULL);
/*
* If we're considering a plain Var in an ORDER BY (but not GROUP BY)
* clause, we may need to add a table-name prefix to prevent
* findTargetlistEntrySQL92 from misinterpreting the name as an
* output-column name. To avoid cluttering the output with unnecessary
* prefixes, do so only if there is a name match to a SELECT tlist item
* that is different from the Var.
*/
if (context->varInOrderBy && !context->inGroupBy && !need_prefix)
{
int colno = 0;
ListCell *l;
foreach(l, context->targetList)
{
TargetEntry *tle = (TargetEntry *) lfirst(l);
char *colname;
if (tle->resjunk)
continue; /* ignore junk entries */
colno++;
/* This must match colname-choosing logic in get_target_list() */
if (context->resultDesc && colno <= context->resultDesc->natts)
colname = NameStr(TupleDescAttr(context->resultDesc,
colno - 1)->attname);
else
colname = tle->resname;
if (colname && strcmp(colname, attname) == 0 &&
!equal(var, tle->expr))
{
need_prefix = true;
break;
}
}
}
if (refname && need_prefix)
{ {
appendStringInfoString(buf, quote_identifier(refname)); appendStringInfoString(buf, quote_identifier(refname));
appendStringInfoChar(buf, '.'); appendStringInfoChar(buf, '.');
@ -6727,7 +6766,7 @@ get_func_expr(FuncExpr *expr, deparse_context *context,
argnames, argtypes, argnames, argtypes,
expr->funcvariadic, expr->funcvariadic,
&use_variadic, &use_variadic,
context->special_exprkind)); context->inGroupBy));
nargs = 0; nargs = 0;
foreach(l, expr->args) foreach(l, expr->args)
{ {
@ -6770,7 +6809,7 @@ get_proc_expr(CallStmt *stmt, deparse_context *context,
namedArgList, argumentTypes, namedArgList, argumentTypes,
stmt->funcexpr->funcvariadic, stmt->funcexpr->funcvariadic,
&use_variadic, &use_variadic,
context->special_exprkind)); context->inGroupBy));
int argNumber = 0; int argNumber = 0;
foreach(argumentCell, finalArgumentList) foreach(argumentCell, finalArgumentList)
{ {
@ -6832,7 +6871,7 @@ get_agg_expr(Aggref *aggref, deparse_context *context,
NIL, argtypes, NIL, argtypes,
aggref->aggvariadic, aggref->aggvariadic,
&use_variadic, &use_variadic,
context->special_exprkind), context->inGroupBy),
(aggref->aggdistinct != NIL) ? "DISTINCT " : ""); (aggref->aggdistinct != NIL) ? "DISTINCT " : "");
if (AGGKIND_IS_ORDERED_SET(aggref->aggkind)) if (AGGKIND_IS_ORDERED_SET(aggref->aggkind))
@ -6941,7 +6980,7 @@ get_windowfunc_expr(WindowFunc *wfunc, deparse_context *context)
generate_function_name(wfunc->winfnoid, nargs, generate_function_name(wfunc->winfnoid, nargs,
argnames, argtypes, argnames, argtypes,
false, NULL, false, NULL,
context->special_exprkind)); context->inGroupBy));
/* winstar can be set only in zero-argument aggregates */ /* winstar can be set only in zero-argument aggregates */
if (wfunc->winstar) if (wfunc->winstar)
@ -6966,7 +7005,7 @@ get_windowfunc_expr(WindowFunc *wfunc, deparse_context *context)
if (wc->name) if (wc->name)
appendStringInfoString(buf, quote_identifier(wc->name)); appendStringInfoString(buf, quote_identifier(wc->name));
else else
get_rule_windowspec(wc, context->windowTList, context); get_rule_windowspec(wc, context->targetList, context);
break; break;
} }
} }
@ -8271,7 +8310,7 @@ get_tablesample_def(TableSampleClause *tablesample, deparse_context *context)
appendStringInfo(buf, " TABLESAMPLE %s (", appendStringInfo(buf, " TABLESAMPLE %s (",
generate_function_name(tablesample->tsmhandler, 1, generate_function_name(tablesample->tsmhandler, 1,
NIL, argtypes, NIL, argtypes,
false, NULL, EXPR_KIND_NONE)); false, NULL, false));
nargs = 0; nargs = 0;
foreach(l, tablesample->args) foreach(l, tablesample->args)
@ -8618,12 +8657,14 @@ generate_fragment_name(char *schemaName, char *tableName)
* the output. For non-FuncExpr cases, has_variadic should be false and * the output. For non-FuncExpr cases, has_variadic should be false and
* use_variadic_p can be NULL. * use_variadic_p can be NULL.
* *
* inGroupBy must be true if we're deparsing a GROUP BY clause.
*
* The result includes all necessary quoting and schema-prefixing. * The result includes all necessary quoting and schema-prefixing.
*/ */
static char * static char *
generate_function_name(Oid funcid, int nargs, List *argnames, Oid *argtypes, generate_function_name(Oid funcid, int nargs, List *argnames, Oid *argtypes,
bool has_variadic, bool *use_variadic_p, bool has_variadic, bool *use_variadic_p,
ParseExprKind special_exprkind) bool inGroupBy)
{ {
char *result; char *result;
HeapTuple proctup; HeapTuple proctup;
@ -8648,9 +8689,9 @@ generate_function_name(Oid funcid, int nargs, List *argnames, Oid *argtypes,
/* /*
* Due to parser hacks to avoid needing to reserve CUBE, we need to force * Due to parser hacks to avoid needing to reserve CUBE, we need to force
* qualification in some special cases. * qualification of some function names within GROUP BY.
*/ */
if (special_exprkind == EXPR_KIND_GROUP_BY) if (inGroupBy)
{ {
if (strcmp(proname, "cube") == 0 || strcmp(proname, "rollup") == 0) if (strcmp(proname, "cube") == 0 || strcmp(proname, "rollup") == 0)
force_qualify = true; force_qualify = true;

228
src/backend/distributed/deparser/ruleutils_16.c
View File

@ -67,7 +67,6 @@
#include "parser/parse_node.h" #include "parser/parse_node.h"
#include "parser/parse_agg.h" #include "parser/parse_agg.h"
#include "parser/parse_func.h" #include "parser/parse_func.h"
#include "parser/parse_node.h"
#include "parser/parse_oper.h" #include "parser/parse_oper.h"
#include "parser/parse_relation.h" #include "parser/parse_relation.h"
#include "parser/parser.h" #include "parser/parser.h"
@ -123,16 +122,18 @@ typedef struct
{ {
StringInfo buf; /* output buffer to append to */ StringInfo buf; /* output buffer to append to */
List *namespaces; /* List of deparse_namespace nodes */ List *namespaces; /* List of deparse_namespace nodes */
TupleDesc resultDesc; /* if top level of a view, the view's tupdesc */
List *targetList; /* Current query level's SELECT targetlist */
List *windowClause; /* Current query level's WINDOW clause */ List *windowClause; /* Current query level's WINDOW clause */
List *windowTList; /* targetlist for resolving WINDOW clause */
int prettyFlags; /* enabling of pretty-print functions */ int prettyFlags; /* enabling of pretty-print functions */
int wrapColumn; /* max line length, or -1 for no limit */ int wrapColumn; /* max line length, or -1 for no limit */
int indentLevel; /* current indent level for prettyprint */ int indentLevel; /* current indent level for prettyprint */
bool varprefix; /* true to print prefixes on Vars */ bool varprefix; /* true to print prefixes on Vars */
Oid distrelid; /* the distributed table being modified, if valid */ Oid distrelid; /* the distributed table being modified, if valid */
int64 shardid; /* a distributed table's shardid, if positive */ int64 shardid; /* a distributed table's shardid, if positive */
ParseExprKind special_exprkind; /* set only for exprkinds needing special bool colNamesVisible; /* do we care about output column names? */
* handling */ bool inGroupBy; /* deparsing GROUP BY clause? */
bool varInOrderBy; /* deparsing simple Var in ORDER BY? */
Bitmapset *appendparents; /* if not null, map child Vars of these relids Bitmapset *appendparents; /* if not null, map child Vars of these relids
* back to the parent rel */ * back to the parent rel */
} deparse_context; } deparse_context;
@ -364,27 +365,19 @@ static void get_query_def_extended(Query *query, StringInfo buf,
int startIndent); int startIndent);
static void get_values_def(List *values_lists, deparse_context *context); static void get_values_def(List *values_lists, deparse_context *context);
static void get_with_clause(Query *query, deparse_context *context); static void get_with_clause(Query *query, deparse_context *context);
static void get_select_query_def(Query *query, deparse_context *context, static void get_select_query_def(Query *query, deparse_context *context);
TupleDesc resultDesc, bool colNamesVisible); static void get_insert_query_def(Query *query, deparse_context *context);
static void get_insert_query_def(Query *query, deparse_context *context, static void get_update_query_def(Query *query, deparse_context *context);
bool colNamesVisible);
static void get_update_query_def(Query *query, deparse_context *context,
bool colNamesVisible);
static void get_update_query_targetlist_def(Query *query, List *targetList, static void get_update_query_targetlist_def(Query *query, List *targetList,
deparse_context *context, deparse_context *context,
RangeTblEntry *rte); RangeTblEntry *rte);
static void get_delete_query_def(Query *query, deparse_context *context, static void get_delete_query_def(Query *query, deparse_context *context);
bool colNamesVisible); static void get_merge_query_def(Query *query, deparse_context *context);
static void get_merge_query_def(Query *query, deparse_context *context,
bool colNamesVisible);
static void get_utility_query_def(Query *query, deparse_context *context); static void get_utility_query_def(Query *query, deparse_context *context);
static void get_basic_select_query(Query *query, deparse_context *context, static void get_basic_select_query(Query *query, deparse_context *context);
TupleDesc resultDesc, bool colNamesVisible); static void get_target_list(List *targetList, deparse_context *context);
static void get_target_list(List *targetList, deparse_context *context,
TupleDesc resultDesc, bool colNamesVisible);
static void get_setop_query(Node *setOp, Query *query, static void get_setop_query(Node *setOp, Query *query,
deparse_context *context, deparse_context *context);
TupleDesc resultDesc, bool colNamesVisible);
static Node *get_rule_sortgroupclause(Index ref, List *tlist, static Node *get_rule_sortgroupclause(Index ref, List *tlist,
bool force_colno, bool force_colno,
deparse_context *context); deparse_context *context);
@ -479,7 +472,7 @@ static char *generate_fragment_name(char *schemaName, char *tableName);
static char *generate_function_name(Oid funcid, int nargs, static char *generate_function_name(Oid funcid, int nargs,
List *argnames, Oid *argtypes, List *argnames, Oid *argtypes,
bool has_variadic, bool *use_variadic_p, bool has_variadic, bool *use_variadic_p,
ParseExprKind special_exprkind); bool inGroupBy);
static List *get_insert_column_names_list(List *targetList, StringInfo buf, deparse_context *context, RangeTblEntry *rte); static List *get_insert_column_names_list(List *targetList, StringInfo buf, deparse_context *context, RangeTblEntry *rte);
#define only_marker(rte) ((rte)->inh ? "" : "ONLY ") #define only_marker(rte) ((rte)->inh ? "" : "ONLY ")
@ -653,13 +646,16 @@ pg_get_rule_expr(Node *expression)
context.buf = buffer; context.buf = buffer;
context.namespaces = NIL; context.namespaces = NIL;
context.resultDesc = NULL;
context.targetList = NIL;
context.windowClause = NIL; context.windowClause = NIL;
context.windowTList = NIL;
context.varprefix = false; context.varprefix = false;
context.prettyFlags = 0; context.prettyFlags = 0;
context.wrapColumn = WRAP_COLUMN_DEFAULT; context.wrapColumn = WRAP_COLUMN_DEFAULT;
context.indentLevel = 0; context.indentLevel = 0;
context.special_exprkind = EXPR_KIND_NONE; context.colNamesVisible = true;
context.inGroupBy = false;
context.varInOrderBy = false;
context.distrelid = InvalidOid; context.distrelid = InvalidOid;
context.shardid = INVALID_SHARD_ID; context.shardid = INVALID_SHARD_ID;
@ -2080,14 +2076,17 @@ get_query_def_extended(Query *query, StringInfo buf, List *parentnamespace,
context.buf = buf; context.buf = buf;
context.namespaces = lcons(&dpns, list_copy(parentnamespace)); context.namespaces = lcons(&dpns, list_copy(parentnamespace));
context.resultDesc = NULL;
context.targetList = NIL;
context.windowClause = NIL; context.windowClause = NIL;
context.windowTList = NIL;
context.varprefix = (parentnamespace != NIL || context.varprefix = (parentnamespace != NIL ||
list_length(query->rtable) != 1); list_length(query->rtable) != 1);
context.prettyFlags = prettyFlags; context.prettyFlags = prettyFlags;
context.wrapColumn = wrapColumn; context.wrapColumn = wrapColumn;
context.indentLevel = startIndent; context.indentLevel = startIndent;
context.special_exprkind = EXPR_KIND_NONE; context.colNamesVisible = true;
context.inGroupBy = false;
context.varInOrderBy = false;
context.appendparents = NULL; context.appendparents = NULL;
context.distrelid = distrelid; context.distrelid = distrelid;
context.shardid = shardid; context.shardid = shardid;
@ -2097,23 +2096,25 @@ get_query_def_extended(Query *query, StringInfo buf, List *parentnamespace,
switch (query->commandType) switch (query->commandType)
{ {
case CMD_SELECT: case CMD_SELECT:
get_select_query_def(query, &context, resultDesc, colNamesVisible); /* We set context.resultDesc only if it's a SELECT */
context.resultDesc = resultDesc;
get_select_query_def(query, &context);
break; break;
case CMD_UPDATE: case CMD_UPDATE:
get_update_query_def(query, &context, colNamesVisible); get_update_query_def(query, &context);
break; break;
case CMD_INSERT: case CMD_INSERT:
get_insert_query_def(query, &context, colNamesVisible); get_insert_query_def(query, &context);
break; break;
case CMD_DELETE: case CMD_DELETE:
get_delete_query_def(query, &context, colNamesVisible); get_delete_query_def(query, &context);
break; break;
case CMD_MERGE: case CMD_MERGE:
get_merge_query_def(query, &context, colNamesVisible); get_merge_query_def(query, &context);
break; break;
case CMD_NOTHING: case CMD_NOTHING:
@ -2321,23 +2322,18 @@ get_with_clause(Query *query, deparse_context *context)
* ---------- * ----------
*/ */
static void static void
get_select_query_def(Query *query, deparse_context *context, get_select_query_def(Query *query, deparse_context *context)
TupleDesc resultDesc, bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
List *save_windowclause;
List *save_windowtlist;
bool force_colno; bool force_colno;
ListCell *l; ListCell *l;
/* Insert the WITH clause if given */ /* Insert the WITH clause if given */
get_with_clause(query, context); get_with_clause(query, context);
/* Set up context for possible window functions */ /* Subroutines may need to consult the SELECT targetlist and windowClause */
save_windowclause = context->windowClause; context->targetList = query->targetList;
context->windowClause = query->windowClause; context->windowClause = query->windowClause;
save_windowtlist = context->windowTList;
context->windowTList = query->targetList;
/* /*
* If the Query node has a setOperations tree, then it's the top level of * If the Query node has a setOperations tree, then it's the top level of
@ -2346,14 +2342,13 @@ get_select_query_def(Query *query, deparse_context *context,
*/ */
if (query->setOperations) if (query->setOperations)
{ {
get_setop_query(query->setOperations, query, context, resultDesc, get_setop_query(query->setOperations, query, context);
colNamesVisible);
/* ORDER BY clauses must be simple in this case */ /* ORDER BY clauses must be simple in this case */
force_colno = true; force_colno = true;
} }
else else
{ {
get_basic_select_query(query, context, resultDesc, colNamesVisible); get_basic_select_query(query, context);
force_colno = false; force_colno = false;
} }
@ -2443,9 +2438,6 @@ get_select_query_def(Query *query, deparse_context *context,
appendStringInfoString(buf, " SKIP LOCKED"); appendStringInfoString(buf, " SKIP LOCKED");
} }
} }
context->windowClause = save_windowclause;
context->windowTList = save_windowtlist;
} }
/* /*
@ -2520,8 +2512,7 @@ get_simple_values_rte(Query *query, TupleDesc resultDesc)
} }
static void static void
get_basic_select_query(Query *query, deparse_context *context, get_basic_select_query(Query *query, deparse_context *context)
TupleDesc resultDesc, bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
RangeTblEntry *values_rte; RangeTblEntry *values_rte;
@ -2539,7 +2530,7 @@ get_basic_select_query(Query *query, deparse_context *context,
* VALUES part. This reverses what transformValuesClause() did at parse * VALUES part. This reverses what transformValuesClause() did at parse
* time. * time.
*/ */
values_rte = get_simple_values_rte(query, resultDesc); values_rte = get_simple_values_rte(query, context->resultDesc);
if (values_rte) if (values_rte)
{ {
get_values_def(values_rte->values_lists, context); get_values_def(values_rte->values_lists, context);
@ -2577,7 +2568,7 @@ get_basic_select_query(Query *query, deparse_context *context,
} }
/* Then we tell what to select (the targetlist) */ /* Then we tell what to select (the targetlist) */
get_target_list(query->targetList, context, resultDesc, colNamesVisible); get_target_list(query->targetList, context);
/* Add the FROM clause if needed */ /* Add the FROM clause if needed */
get_from_clause(query, " FROM ", context); get_from_clause(query, " FROM ", context);
@ -2593,15 +2584,15 @@ get_basic_select_query(Query *query, deparse_context *context,
/* Add the GROUP BY clause if given */ /* Add the GROUP BY clause if given */
if (query->groupClause != NULL || query->groupingSets != NULL) if (query->groupClause != NULL || query->groupingSets != NULL)
{ {
ParseExprKind save_exprkind; bool save_ingroupby;
appendContextKeyword(context, " GROUP BY ", appendContextKeyword(context, " GROUP BY ",
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1); -PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
if (query->groupDistinct) if (query->groupDistinct)
appendStringInfoString(buf, "DISTINCT "); appendStringInfoString(buf, "DISTINCT ");
save_exprkind = context->special_exprkind; save_ingroupby = context->inGroupBy;
context->special_exprkind = EXPR_KIND_GROUP_BY; context->inGroupBy = true;
if (query->groupingSets == NIL) if (query->groupingSets == NIL)
{ {
@ -2629,7 +2620,7 @@ get_basic_select_query(Query *query, deparse_context *context,
} }
} }
context->special_exprkind = save_exprkind; context->inGroupBy = save_ingroupby;
} }
/* Add the HAVING clause if given */ /* Add the HAVING clause if given */
@ -2648,14 +2639,11 @@ get_basic_select_query(Query *query, deparse_context *context,
/* ---------- /* ----------
* get_target_list - Parse back a SELECT target list * get_target_list - Parse back a SELECT target list
* *
* This is also used for RETURNING lists in INSERT/UPDATE/DELETE. * This is also used for RETURNING lists in INSERT/UPDATE/DELETE/MERGE.
*
* resultDesc and colNamesVisible are as for get_query_def()
* ---------- * ----------
*/ */
static void static void
get_target_list(List *targetList, deparse_context *context, get_target_list(List *targetList, deparse_context *context)
TupleDesc resultDesc, bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
StringInfoData targetbuf; StringInfoData targetbuf;
@ -2712,7 +2700,7 @@ get_target_list(List *targetList, deparse_context *context,
* assigned column name explicitly. Otherwise, show it only if * assigned column name explicitly. Otherwise, show it only if
* it's not FigureColname's fallback. * it's not FigureColname's fallback.
*/ */
attname = colNamesVisible ? NULL : "?column?"; attname = context->colNamesVisible ? NULL : "?column?";
} }
/* /*
@ -2721,8 +2709,9 @@ get_target_list(List *targetList, deparse_context *context,
* effects of any column RENAME that's been done on the view). * effects of any column RENAME that's been done on the view).
* Otherwise, just use what we can find in the TLE. * Otherwise, just use what we can find in the TLE.
*/ */
if (resultDesc && colno <= resultDesc->natts) if (context->resultDesc && colno <= context->resultDesc->natts)
colname = NameStr(TupleDescAttr(resultDesc, colno - 1)->attname); colname = NameStr(TupleDescAttr(context->resultDesc,
colno - 1)->attname);
else else
colname = tle->resname; colname = tle->resname;
@ -2790,8 +2779,7 @@ get_target_list(List *targetList, deparse_context *context,
} }
static void static void
get_setop_query(Node *setOp, Query *query, deparse_context *context, get_setop_query(Node *setOp, Query *query, deparse_context *context)
TupleDesc resultDesc, bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
bool need_paren; bool need_paren;
@ -2816,8 +2804,8 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
subquery->limitCount); subquery->limitCount);
if (need_paren) if (need_paren)
appendStringInfoChar(buf, '('); appendStringInfoChar(buf, '(');
get_query_def(subquery, buf, context->namespaces, resultDesc, get_query_def(subquery, buf, context->namespaces,
colNamesVisible, context->resultDesc, context->colNamesVisible,
context->prettyFlags, context->wrapColumn, context->prettyFlags, context->wrapColumn,
context->indentLevel); context->indentLevel);
if (need_paren) if (need_paren)
@ -2827,6 +2815,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
{ {
SetOperationStmt *op = (SetOperationStmt *) setOp; SetOperationStmt *op = (SetOperationStmt *) setOp;
int subindent; int subindent;
bool save_colnamesvisible;
/* /*
* We force parens when nesting two SetOperationStmts, except when the * We force parens when nesting two SetOperationStmts, except when the
@ -2860,7 +2849,7 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
else else
subindent = 0; subindent = 0;
get_setop_query(op->larg, query, context, resultDesc, colNamesVisible); get_setop_query(op->larg, query, context);
if (need_paren) if (need_paren)
appendContextKeyword(context, ") ", -subindent, 0, 0); appendContextKeyword(context, ") ", -subindent, 0, 0);
@ -2904,7 +2893,13 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
subindent = 0; subindent = 0;
appendContextKeyword(context, "", subindent, 0, 0); appendContextKeyword(context, "", subindent, 0, 0);
get_setop_query(op->rarg, query, context, resultDesc, false); /*
* The output column names of the RHS sub-select don't matter.
*/
save_colnamesvisible = context->colNamesVisible;
context->colNamesVisible = false;
get_setop_query(op->rarg, query, context);
context->colNamesVisible = save_colnamesvisible;
if (PRETTY_INDENT(context)) if (PRETTY_INDENT(context))
context->indentLevel -= subindent; context->indentLevel -= subindent;
@ -2938,20 +2933,31 @@ get_rule_sortgroupclause(Index ref, List *tlist, bool force_colno,
* Use column-number form if requested by caller. Otherwise, if * Use column-number form if requested by caller. Otherwise, if
* expression is a constant, force it to be dumped with an explicit cast * expression is a constant, force it to be dumped with an explicit cast
* as decoration --- this is because a simple integer constant is * as decoration --- this is because a simple integer constant is
* ambiguous (and will be misinterpreted by findTargetlistEntry()) if we * ambiguous (and will be misinterpreted by findTargetlistEntrySQL92()) if
* dump it without any decoration. If it's anything more complex than a * we dump it without any decoration. Similarly, if it's just a Var,
* simple Var, then force extra parens around it, to ensure it can't be * there is risk of misinterpretation if the column name is reassigned in
* misinterpreted as a cube() or rollup() construct. * the SELECT list, so we may need to force table qualification. And, if
* it's anything more complex than a simple Var, then force extra parens
* around it, to ensure it can't be misinterpreted as a cube() or rollup()
* construct.
*/ */
if (force_colno) if (force_colno)
{ {
Assert(!tle->resjunk); Assert(!tle->resjunk);
appendStringInfo(buf, "%d", tle->resno); appendStringInfo(buf, "%d", tle->resno);
} }
else if (expr && IsA(expr, Const)) else if (!expr)
/* do nothing, probably can't happen */ ;
else if (IsA(expr, Const))
get_const_expr((Const *) expr, context, 1); get_const_expr((Const *) expr, context, 1);
else if (!expr || IsA(expr, Var)) else if (IsA(expr, Var))
get_rule_expr(expr, context, true); {
/* Tell get_variable to check for name conflict */
bool save_varinorderby = context->varInOrderBy;
context->varInOrderBy = true;
(void) get_variable((Var *) expr, 0, false, context);
context->varInOrderBy = save_varinorderby;
}
else else
{ {
/* /*
@ -3240,8 +3246,7 @@ get_rule_windowspec(WindowClause *wc, List *targetList,
* ---------- * ----------
*/ */
static void static void
get_insert_query_def(Query *query, deparse_context *context, get_insert_query_def(Query *query, deparse_context *context)
bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
RangeTblEntry *select_rte = NULL; RangeTblEntry *select_rte = NULL;
@ -3422,7 +3427,7 @@ get_insert_query_def(Query *query, deparse_context *context,
{ {
appendContextKeyword(context, " RETURNING", appendContextKeyword(context, " RETURNING",
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1); -PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
get_target_list(query->returningList, context, NULL, colNamesVisible); get_target_list(query->returningList, context);
} }
} }
@ -3431,8 +3436,7 @@ get_insert_query_def(Query *query, deparse_context *context,
* ---------- * ----------
*/ */
static void static void
get_update_query_def(Query *query, deparse_context *context, get_update_query_def(Query *query, deparse_context *context)
bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
RangeTblEntry *rte; RangeTblEntry *rte;
@ -3501,7 +3505,7 @@ get_update_query_def(Query *query, deparse_context *context,
{ {
appendContextKeyword(context, " RETURNING", appendContextKeyword(context, " RETURNING",
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1); -PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
get_target_list(query->returningList, context, NULL, colNamesVisible); get_target_list(query->returningList, context);
} }
} }
@ -3661,8 +3665,7 @@ get_update_query_targetlist_def(Query *query, List *targetList,
* ---------- * ----------
*/ */
static void static void
get_delete_query_def(Query *query, deparse_context *context, get_delete_query_def(Query *query, deparse_context *context)
bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
RangeTblEntry *rte; RangeTblEntry *rte;
@ -3726,7 +3729,7 @@ get_delete_query_def(Query *query, deparse_context *context,
{ {
appendContextKeyword(context, " RETURNING", appendContextKeyword(context, " RETURNING",
-PRETTYINDENT_STD, PRETTYINDENT_STD, 1); -PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
get_target_list(query->returningList, context, NULL, colNamesVisible); get_target_list(query->returningList, context);
} }
} }
@ -3736,8 +3739,7 @@ get_delete_query_def(Query *query, deparse_context *context,
* ---------- * ----------
*/ */
static void static void
get_merge_query_def(Query *query, deparse_context *context, get_merge_query_def(Query *query, deparse_context *context)
bool colNamesVisible)
{ {
StringInfo buf = context->buf; StringInfo buf = context->buf;
RangeTblEntry *rte; RangeTblEntry *rte;
@ -3977,6 +3979,7 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
deparse_columns *colinfo; deparse_columns *colinfo;
char *refname; char *refname;
char *attname; char *attname;
bool need_prefix;
/* Find appropriate nesting depth */ /* Find appropriate nesting depth */
netlevelsup = var->varlevelsup + levelsup; netlevelsup = var->varlevelsup + levelsup;
@ -4177,7 +4180,42 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
attname = get_rte_attribute_name(rte, attnum); attname = get_rte_attribute_name(rte, attnum);
} }
if (refname && (context->varprefix || attname == NULL)) need_prefix = (context->varprefix || attname == NULL);
/*
* If we're considering a plain Var in an ORDER BY (but not GROUP BY)
* clause, we may need to add a table-name prefix to prevent
* findTargetlistEntrySQL92 from misinterpreting the name as an
* output-column name. To avoid cluttering the output with unnecessary
* prefixes, do so only if there is a name match to a SELECT tlist item
* that is different from the Var.
*/
if (context->varInOrderBy && !context->inGroupBy && !need_prefix)
{
int colno = 0;
ListCell *l;
foreach(l, context->targetList)
{
TargetEntry *tle = (TargetEntry *) lfirst(l);
char *colname;
if (tle->resjunk)
continue; /* ignore junk entries */
colno++;
/* This must match colname-choosing logic in get_target_list() */
if (context->resultDesc && colno <= context->resultDesc->natts)
colname = NameStr(TupleDescAttr(context->resultDesc,
colno - 1)->attname);
else
colname = tle->resname;
if (colname && strcmp(colname, attname) == 0 &&
!equal(var, tle->expr))
{
need_prefix = true;
break;
}
}
}
if (refname && need_prefix)
{ {
appendStringInfoString(buf, quote_identifier(refname)); appendStringInfoString(buf, quote_identifier(refname));
appendStringInfoChar(buf, '.'); appendStringInfoChar(buf, '.');
@ -6775,7 +6813,7 @@ get_func_expr(FuncExpr *expr, deparse_context *context,
argnames, argtypes, argnames, argtypes,
expr->funcvariadic, expr->funcvariadic,
&use_variadic, &use_variadic,
context->special_exprkind)); context->inGroupBy));
nargs = 0; nargs = 0;
foreach(l, expr->args) foreach(l, expr->args)
{ {
@ -6818,7 +6856,7 @@ get_proc_expr(CallStmt *stmt, deparse_context *context,
namedArgList, argumentTypes, namedArgList, argumentTypes,
stmt->funcexpr->funcvariadic, stmt->funcexpr->funcvariadic,
&use_variadic, &use_variadic,
context->special_exprkind)); context->inGroupBy));
int argNumber = 0; int argNumber = 0;
foreach(argumentCell, finalArgumentList) foreach(argumentCell, finalArgumentList)
{ {
@ -6891,7 +6929,7 @@ get_agg_expr_helper(Aggref *aggref, deparse_context *context,
funcname = generate_function_name(aggref->aggfnoid, nargs, NIL, funcname = generate_function_name(aggref->aggfnoid, nargs, NIL,
argtypes, aggref->aggvariadic, argtypes, aggref->aggvariadic,
&use_variadic, &use_variadic,
context->special_exprkind); context->inGroupBy);
/* Print the aggregate name, schema-qualified if needed */ /* Print the aggregate name, schema-qualified if needed */
appendStringInfo(buf, "%s(%s", funcname, appendStringInfo(buf, "%s(%s", funcname,
@ -7032,7 +7070,7 @@ get_windowfunc_expr_helper(WindowFunc *wfunc, deparse_context *context,
if (!funcname) if (!funcname)
funcname = generate_function_name(wfunc->winfnoid, nargs, argnames, funcname = generate_function_name(wfunc->winfnoid, nargs, argnames,
argtypes, false, NULL, argtypes, false, NULL,
context->special_exprkind); context->inGroupBy);
appendStringInfo(buf, "%s(", funcname); appendStringInfo(buf, "%s(", funcname);
@ -7071,7 +7109,7 @@ get_windowfunc_expr_helper(WindowFunc *wfunc, deparse_context *context,
if (wc->name) if (wc->name)
appendStringInfoString(buf, quote_identifier(wc->name)); appendStringInfoString(buf, quote_identifier(wc->name));
else else
get_rule_windowspec(wc, context->windowTList, context); get_rule_windowspec(wc, context->targetList, context);
break; break;
} }
} }
@ -8547,7 +8585,7 @@ get_tablesample_def(TableSampleClause *tablesample, deparse_context *context)
appendStringInfo(buf, " TABLESAMPLE %s (", appendStringInfo(buf, " TABLESAMPLE %s (",
generate_function_name(tablesample->tsmhandler, 1, generate_function_name(tablesample->tsmhandler, 1,
NIL, argtypes, NIL, argtypes,
false, NULL, EXPR_KIND_NONE)); false, NULL, false));
nargs = 0; nargs = 0;
foreach(l, tablesample->args) foreach(l, tablesample->args)
@ -8894,12 +8932,14 @@ generate_fragment_name(char *schemaName, char *tableName)
* the output. For non-FuncExpr cases, has_variadic should be false and * the output. For non-FuncExpr cases, has_variadic should be false and
* use_variadic_p can be NULL. * use_variadic_p can be NULL.
* *
* inGroupBy must be true if we're deparsing a GROUP BY clause.
*
* The result includes all necessary quoting and schema-prefixing. * The result includes all necessary quoting and schema-prefixing.
*/ */
static char * static char *
generate_function_name(Oid funcid, int nargs, List *argnames, Oid *argtypes, generate_function_name(Oid funcid, int nargs, List *argnames, Oid *argtypes,
bool has_variadic, bool *use_variadic_p, bool has_variadic, bool *use_variadic_p,
ParseExprKind special_exprkind) bool inGroupBy)
{ {
char *result; char *result;
HeapTuple proctup; HeapTuple proctup;
@ -8924,9 +8964,9 @@ generate_function_name(Oid funcid, int nargs, List *argnames, Oid *argtypes,
/* /*
* Due to parser hacks to avoid needing to reserve CUBE, we need to force * Due to parser hacks to avoid needing to reserve CUBE, we need to force
* qualification in some special cases. * qualification of some function names within GROUP BY.
*/ */
if (special_exprkind == EXPR_KIND_GROUP_BY) if (inGroupBy)
{ {
if (strcmp(proname, "cube") == 0 || strcmp(proname, "rollup") == 0) if (strcmp(proname, "cube") == 0 || strcmp(proname, "rollup") == 0)
force_qualify = true; force_qualify = true;

93
src/test/regress/expected/multi_alias.out
View File

@ -168,9 +168,100 @@ DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
6 | 7 | 8 | 6 | 7 | 8 |
(3 rows) (3 rows)
RESET citus.log_remote_commands;
-- test everything on https://github.com/citusdata/citus/issues/7684
CREATE TABLE test_name_prefix (
attribute1 varchar(255),
attribute2 varchar(255),
attribute3 varchar(255)
);
INSERT INTO test_name_prefix (attribute1, attribute2, attribute3)
VALUES ('Phone', 'John', 'A'),
('Phone', 'Eric', 'A'),
('Tablet','Eric', 'B');
-- vanilla Postgres result
-- with DISTINCT ON (T.attribute1, T.attribute2)
-- we have 3 distinct groups of 1 row each
-- (Phone, John) (Phone, Eric) and (Tablet, Eric)
SELECT DISTINCT ON (T.attribute1, T.attribute2)
T.attribute1 AS attribute1,
T.attribute3 AS attribute2,
T.attribute2 AS attribute3
FROM test_name_prefix T ORDER BY T.attribute1, T.attribute2;
attribute1 | attribute2 | attribute3
---------------------------------------------------------------------
Phone | A | Eric
Phone | A | John
Tablet | B | Eric
(3 rows)
-- vanilla Postgres result
-- changes when we remove the table-name prefix to attribute2
-- in this case it uses the output column name,
-- which is actually T.attribute3 (AS attribute2)
-- so, with DISTINCT ON (T.attribute1, T.attribute3)
-- we have only 2 distinct groups
-- (Phone, A) and (Tablet, B)
SELECT DISTINCT ON (T.attribute1, attribute2)
T.attribute1 AS attribute1,
T.attribute3 AS attribute2, -- name match in output column name
T.attribute2 AS attribute3
FROM test_name_prefix T ORDER BY T.attribute1, attribute2;
attribute1 | attribute2 | attribute3
---------------------------------------------------------------------
Phone | A | John
Tablet | B | Eric
(2 rows)
-- now, let's verify the distributed query scenario
SELECT create_distributed_table('test_name_prefix', 'attribute1');
NOTICE: Copying data from local table...
NOTICE: copying the data has completed
DETAIL: The local data in the table is no longer visible, but is still on disk.
HINT: To remove the local data, run: SELECT truncate_local_data_after_distributing_table($$alias.test_name_prefix$$)
create_distributed_table
---------------------------------------------------------------------
(1 row)
SET citus.log_remote_commands TO on;
-- make sure we preserve the table-name prefix to attribute2
-- when building the shard query
-- (before this patch we wouldn't preserve T.attribute2)
-- note that we only need to preserve T.attribute2, not T.attribute1
-- because there is no confusion there
SELECT DISTINCT ON (T.attribute1, T.attribute2)
T.attribute1 AS attribute1,
T.attribute3 AS attribute2,
T.attribute2 AS attribute3
FROM test_name_prefix T ORDER BY T.attribute1, T.attribute2;
NOTICE: issuing SELECT DISTINCT ON (attribute1, t.attribute2) attribute1, attribute3 AS attribute2, attribute2 AS attribute3 FROM alias.test_name_prefix_90630803 t ORDER BY attribute1, t.attribute2
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
attribute1 | attribute2 | attribute3
---------------------------------------------------------------------
Phone | A | Eric
Phone | A | John
Tablet | B | Eric
(3 rows)
-- here Citus will replace attribute2 with T.attribute3
SELECT DISTINCT ON (T.attribute1, attribute2)
T.attribute1 AS attribute1,
T.attribute3 AS attribute2,
T.attribute2 AS attribute3
FROM test_name_prefix T ORDER BY T.attribute1, attribute2;
NOTICE: issuing SELECT DISTINCT ON (attribute1, t.attribute3) attribute1, attribute3 AS attribute2, attribute2 AS attribute3 FROM alias.test_name_prefix_90630803 t ORDER BY attribute1, t.attribute3
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
attribute1 | attribute2 | attribute3
---------------------------------------------------------------------
Phone | A | John
Tablet | B | Eric
(2 rows)
RESET citus.log_remote_commands; RESET citus.log_remote_commands;
DROP SCHEMA alias CASCADE; DROP SCHEMA alias CASCADE;
NOTICE: drop cascades to 3 other objects NOTICE: drop cascades to 4 other objects
DETAIL: drop cascades to table alias_test DETAIL: drop cascades to table alias_test
drop cascades to table test drop cascades to table test
drop cascades to table test_2 drop cascades to table test_2
drop cascades to table test_name_prefix

61
src/test/regress/sql/multi_alias.sql
View File

@ -63,4 +63,65 @@ SELECT *
ORDER BY a, d; ORDER BY a, d;
RESET citus.log_remote_commands; RESET citus.log_remote_commands;
-- test everything on https://github.com/citusdata/citus/issues/7684
CREATE TABLE test_name_prefix (
attribute1 varchar(255),
attribute2 varchar(255),
attribute3 varchar(255)
);
INSERT INTO test_name_prefix (attribute1, attribute2, attribute3)
VALUES ('Phone', 'John', 'A'),
('Phone', 'Eric', 'A'),
('Tablet','Eric', 'B');
-- vanilla Postgres result
-- with DISTINCT ON (T.attribute1, T.attribute2)
-- we have 3 distinct groups of 1 row each
-- (Phone, John) (Phone, Eric) and (Tablet, Eric)
SELECT DISTINCT ON (T.attribute1, T.attribute2)
T.attribute1 AS attribute1,
T.attribute3 AS attribute2,
T.attribute2 AS attribute3
FROM test_name_prefix T ORDER BY T.attribute1, T.attribute2;
-- vanilla Postgres result
-- changes when we remove the table-name prefix to attribute2
-- in this case it uses the output column name,
-- which is actually T.attribute3 (AS attribute2)
-- so, with DISTINCT ON (T.attribute1, T.attribute3)
-- we have only 2 distinct groups
-- (Phone, A) and (Tablet, B)
SELECT DISTINCT ON (T.attribute1, attribute2)
T.attribute1 AS attribute1,
T.attribute3 AS attribute2, -- name match in output column name
T.attribute2 AS attribute3
FROM test_name_prefix T ORDER BY T.attribute1, attribute2;
-- now, let's verify the distributed query scenario
SELECT create_distributed_table('test_name_prefix', 'attribute1');
SET citus.log_remote_commands TO on;
-- make sure we preserve the table-name prefix to attribute2
-- when building the shard query
-- (before this patch we wouldn't preserve T.attribute2)
-- note that we only need to preserve T.attribute2, not T.attribute1
-- because there is no confusion there
SELECT DISTINCT ON (T.attribute1, T.attribute2)
T.attribute1 AS attribute1,
T.attribute3 AS attribute2,
T.attribute2 AS attribute3
FROM test_name_prefix T ORDER BY T.attribute1, T.attribute2;
-- here Citus will replace attribute2 with T.attribute3
SELECT DISTINCT ON (T.attribute1, attribute2)
T.attribute1 AS attribute1,
T.attribute3 AS attribute2,
T.attribute2 AS attribute3
FROM test_name_prefix T ORDER BY T.attribute1, attribute2;
RESET citus.log_remote_commands;
DROP SCHEMA alias CASCADE; DROP SCHEMA alias CASCADE;