diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 00eda1b34c..2a624a9b9c 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -50,7 +50,6 @@ #include "optimizer/optimizer.h" #include "parser/parse_agg.h" #include "parser/parse_func.h" -#include "parser/parse_node.h" #include "parser/parse_oper.h" #include "parser/parse_relation.h" #include "parser/parser.h" @@ -114,14 +113,14 @@ typedef struct { StringInfo buf; /* output buffer to append to */ List *namespaces; /* List of deparse_namespace nodes */ + List *targetList; /* Current query level's SELECT targetlist */ List *windowClause; /* Current query level's WINDOW clause */ - List *windowTList; /* targetlist for resolving WINDOW clause */ int prettyFlags; /* enabling of pretty-print functions */ int wrapColumn; /* max line length, or -1 for no limit */ int indentLevel; /* current indent level for pretty-print */ bool varprefix; /* true to print prefixes on Vars */ - ParseExprKind special_exprkind; /* set only for exprkinds needing special - * handling */ + bool in_group_by; /* deparsing GROUP BY clause? */ + bool var_in_order_by; /* deparsing simple Var in ORDER BY? */ Bitmapset *appendparents; /* if not null, map child Vars of these relids * back to the parent rel */ } deparse_context; @@ -515,7 +514,7 @@ static char *generate_qualified_relation_name(Oid relid); static char *generate_function_name(Oid funcid, int nargs, List *argnames, Oid *argtypes, bool has_variadic, bool *use_variadic_p, - ParseExprKind special_exprkind); + bool in_group_by); static char *generate_operator_name(Oid operid, Oid arg1, Oid arg2); static void add_cast_to(StringInfo buf, Oid typid); static char *generate_qualified_type_name(Oid typid); @@ -1094,13 +1093,14 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty) /* Set up context with one-deep namespace stack */ context.buf = &buf; context.namespaces = list_make1(&dpns); + context.targetList = NIL; context.windowClause = NIL; - context.windowTList = NIL; context.varprefix = true; context.prettyFlags = GET_PRETTY_FLAGS(pretty); context.wrapColumn = WRAP_COLUMN_DEFAULT; context.indentLevel = PRETTYINDENT_STD; - context.special_exprkind = EXPR_KIND_NONE; + context.in_group_by = false; + context.var_in_order_by = false; context.appendparents = NULL; get_rule_expr(qual, &context, false); @@ -1111,7 +1111,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty) appendStringInfo(&buf, "EXECUTE FUNCTION %s(", generate_function_name(trigrec->tgfoid, 0, NIL, NULL, - false, NULL, EXPR_KIND_NONE)); + false, NULL, false)); if (trigrec->tgnargs > 0) { @@ -2992,7 +2992,7 @@ pg_get_functiondef(PG_FUNCTION_ARGS) appendStringInfo(&buf, " SUPPORT %s", generate_function_name(proc->prosupport, 1, NIL, argtypes, - false, NULL, EXPR_KIND_NONE)); + false, NULL, false)); } if (oldlen != buf.len) @@ -3632,13 +3632,14 @@ deparse_expression_pretty(Node *expr, List *dpcontext, initStringInfo(&buf); context.buf = &buf; context.namespaces = dpcontext; + context.targetList = NIL; context.windowClause = NIL; - context.windowTList = NIL; context.varprefix = forceprefix; context.prettyFlags = prettyFlags; context.wrapColumn = WRAP_COLUMN_DEFAULT; context.indentLevel = startIndent; - context.special_exprkind = EXPR_KIND_NONE; + context.in_group_by = false; + context.var_in_order_by = false; context.appendparents = NULL; get_rule_expr(expr, &context, showimplicit); @@ -5283,13 +5284,14 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc, context.buf = buf; context.namespaces = list_make1(&dpns); + context.targetList = NIL; context.windowClause = NIL; - context.windowTList = NIL; context.varprefix = (list_length(query->rtable) != 1); context.prettyFlags = prettyFlags; context.wrapColumn = WRAP_COLUMN_DEFAULT; context.indentLevel = PRETTYINDENT_STD; - context.special_exprkind = EXPR_KIND_NONE; + context.in_group_by = false; + context.var_in_order_by = false; context.appendparents = NULL; set_deparse_for_query(&dpns, query, NIL); @@ -5451,14 +5453,15 @@ get_query_def(Query *query, StringInfo buf, List *parentnamespace, context.buf = buf; context.namespaces = lcons(&dpns, list_copy(parentnamespace)); + context.targetList = NIL; context.windowClause = NIL; - context.windowTList = NIL; context.varprefix = (parentnamespace != NIL || list_length(query->rtable) != 1); context.prettyFlags = prettyFlags; context.wrapColumn = wrapColumn; context.indentLevel = startIndent; - context.special_exprkind = EXPR_KIND_NONE; + context.in_group_by = false; + context.var_in_order_by = false; context.appendparents = NULL; set_deparse_for_query(&dpns, query, parentnamespace); @@ -5691,19 +5694,21 @@ get_select_query_def(Query *query, deparse_context *context, TupleDesc resultDesc, bool colNamesVisible) { StringInfo buf = context->buf; + List *save_targetlist; List *save_windowclause; - List *save_windowtlist; bool force_colno; ListCell *l; /* Insert the WITH clause if given */ get_with_clause(query, context); + /* We may need to consult the SELECT targetlist */ + save_targetlist = context->targetList; + context->targetList = query->targetList; + /* Set up context for possible window functions */ save_windowclause = context->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 @@ -5809,8 +5814,8 @@ get_select_query_def(Query *query, deparse_context *context, } } + context->targetList = save_targetlist; context->windowClause = save_windowclause; - context->windowTList = save_windowtlist; } /* @@ -5961,15 +5966,15 @@ get_basic_select_query(Query *query, deparse_context *context, /* Add the GROUP BY clause if given */ if (query->groupClause != NULL || query->groupingSets != NULL) { - ParseExprKind save_exprkind; + bool save_in_group_by; appendContextKeyword(context, " GROUP BY ", -PRETTYINDENT_STD, PRETTYINDENT_STD, 1); if (query->groupDistinct) appendStringInfoString(buf, "DISTINCT "); - save_exprkind = context->special_exprkind; - context->special_exprkind = EXPR_KIND_GROUP_BY; + save_in_group_by = context->in_group_by; + context->in_group_by = true; if (query->groupingSets == NIL) { @@ -5997,7 +6002,7 @@ get_basic_select_query(Query *query, deparse_context *context, } } - context->special_exprkind = save_exprkind; + context->in_group_by = save_in_group_by; } /* Add the HAVING clause if given */ @@ -6306,20 +6311,32 @@ get_rule_sortgroupclause(Index ref, List *tlist, bool force_colno, * Use column-number form if requested by caller. Otherwise, if * expression is a constant, force it to be dumped with an explicit cast * as decoration --- this is because a simple integer constant is - * ambiguous (and will be misinterpreted by findTargetlistEntry()) if we - * dump it without any decoration. 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. + * ambiguous (and will be misinterpreted by findTargetlistEntrySQL92()) if + * we dump it without any decoration. Similarly, if it's just a Var, + * there is risk of misinterpretation if the column name is reassigned in + * 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) { Assert(!tle->resjunk); 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); - else if (!expr || IsA(expr, Var)) - get_rule_expr(expr, context, true); + else if (IsA(expr, Var)) + { + /* Tell get_variable to check for name conflict */ + bool save_var_in_order_by = context->var_in_order_by; + + context->var_in_order_by = true; + (void) get_variable((Var *) expr, 0, false, context); + context->var_in_order_by = save_var_in_order_by; + } else { /* @@ -7307,6 +7324,7 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context) deparse_columns *colinfo; char *refname; char *attname; + bool need_prefix; /* Find appropriate nesting depth */ netlevelsup = var->varlevelsup + levelsup; @@ -7502,7 +7520,31 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context) 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->var_in_order_by && !context->in_group_by && !need_prefix) + { + foreach_node(TargetEntry, tle, context->targetList) + { + if (!tle->resjunk && tle->resname && + strcmp(tle->resname, attname) == 0 && + !equal(var, tle->expr)) + { + need_prefix = true; + break; + } + } + } + + if (refname && need_prefix) { appendStringInfoString(buf, quote_identifier(refname)); appendStringInfoChar(buf, '.'); @@ -10463,7 +10505,7 @@ get_func_expr(FuncExpr *expr, deparse_context *context, argnames, argtypes, expr->funcvariadic, &use_variadic, - context->special_exprkind)); + context->in_group_by)); nargs = 0; foreach(l, expr->args) { @@ -10533,7 +10575,7 @@ get_agg_expr_helper(Aggref *aggref, deparse_context *context, funcname = generate_function_name(aggref->aggfnoid, nargs, NIL, argtypes, aggref->aggvariadic, &use_variadic, - context->special_exprkind); + context->in_group_by); /* Print the aggregate name, schema-qualified if needed */ appendStringInfo(buf, "%s(%s", funcname, @@ -10674,7 +10716,7 @@ get_windowfunc_expr_helper(WindowFunc *wfunc, deparse_context *context, if (!funcname) funcname = generate_function_name(wfunc->winfnoid, nargs, argnames, argtypes, false, NULL, - context->special_exprkind); + context->in_group_by); appendStringInfo(buf, "%s(", funcname); @@ -10713,7 +10755,7 @@ get_windowfunc_expr_helper(WindowFunc *wfunc, deparse_context *context, if (wc->name) appendStringInfoString(buf, quote_identifier(wc->name)); else - get_rule_windowspec(wc, context->windowTList, context); + get_rule_windowspec(wc, context->targetList, context); break; } } @@ -12420,7 +12462,7 @@ get_tablesample_def(TableSampleClause *tablesample, deparse_context *context) appendStringInfo(buf, " TABLESAMPLE %s (", generate_function_name(tablesample->tsmhandler, 1, NIL, argtypes, - false, NULL, EXPR_KIND_NONE)); + false, NULL, false)); nargs = 0; foreach(l, tablesample->args) @@ -12840,12 +12882,14 @@ generate_qualified_relation_name(Oid relid) * the output. For non-FuncExpr cases, has_variadic should be false and * use_variadic_p can be NULL. * + * in_group_by must be true if we're deparsing a GROUP BY clause. + * * The result includes all necessary quoting and schema-prefixing. */ static char * generate_function_name(Oid funcid, int nargs, List *argnames, Oid *argtypes, bool has_variadic, bool *use_variadic_p, - ParseExprKind special_exprkind) + bool in_group_by) { char *result; HeapTuple proctup; @@ -12870,9 +12914,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 - * qualification in some special cases. + * qualification of some function names within GROUP BY. */ - if (special_exprkind == EXPR_KIND_GROUP_BY) + if (in_group_by) { if (strcmp(proname, "cube") == 0 || strcmp(proname, "rollup") == 0) force_qualify = true; diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out index f3f8c7b5a2..7b42911d2f 100644 --- a/src/test/regress/expected/create_view.out +++ b/src/test/regress/expected/create_view.out @@ -824,6 +824,22 @@ View definition: FROM temp_view_test.tx1 tx1_1 WHERE tx1.y1 = tx1_1.f1)); +-- Test correct deparsing of ORDER BY when there is a name conflict +create view aliased_order_by as +select x1 as x2, x2 as x1 from tt1 + order by x2; +\d+ aliased_order_by + View "testviewschm2.aliased_order_by" + Column | Type | Collation | Nullable | Default | Storage | Description +--------+---------+-----------+----------+---------+---------+------------- + x2 | integer | | | | plain | + x1 | integer | | | | plain | +View definition: + SELECT x1 AS x2, + x2 AS x1 + FROM tt1 + ORDER BY tt1.x1; + -- Test aliasing of joins create view view_of_joins as select * from @@ -2248,7 +2264,7 @@ drop cascades to view aliased_view_2 drop cascades to view aliased_view_3 drop cascades to view aliased_view_4 DROP SCHEMA testviewschm2 CASCADE; -NOTICE: drop cascades to 79 other objects +NOTICE: drop cascades to 80 other objects DETAIL: drop cascades to table t1 drop cascades to view temporal1 drop cascades to view temporal2 @@ -2275,6 +2291,7 @@ drop cascades to view mysecview9 drop cascades to view unspecified_types drop cascades to table tt1 drop cascades to table tx1 +drop cascades to view aliased_order_by drop cascades to view view_of_joins drop cascades to table tbl1a drop cascades to view view_of_joins_2a diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql index 3a78be1b0c..7030744881 100644 --- a/src/test/regress/sql/create_view.sql +++ b/src/test/regress/sql/create_view.sql @@ -373,6 +373,14 @@ ALTER TABLE tmp1 RENAME TO tx1; \d+ aliased_view_3 \d+ aliased_view_4 +-- Test correct deparsing of ORDER BY when there is a name conflict + +create view aliased_order_by as +select x1 as x2, x2 as x1 from tt1 + order by x2; + +\d+ aliased_order_by + -- Test aliasing of joins create view view_of_joins as