From ae6aa961aeb0545bd05b9be3b3f79bc79f190293 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Mon, 7 Feb 2022 09:04:02 +0100 Subject: [PATCH] Use JOIN USING aliases in ruleutils.c When reverse-compiling a query, ruleutils.c has some complicated code to handle the join output columns of a JOIN USING join. There used to be no way to qualify those columns, and so if there was a naming conflict anywhere in the query, those output columns had to be renamed to be unique throughout the query. Since PostgreSQL 14, we have a new feature that allows adding an alias to a JOIN USING clause. This provides a better solution to this problem. This patch changes the logic in ruleutils.c so that if naming conflicts with JOIN USING output columns are found in the query, these JOIN USING aliases with generated names are attached everywhere and the columns are then qualified everywhere. --- src/backend/utils/adt/ruleutils.c | 102 ++++++++---- src/test/regress/expected/create_view.out | 194 +++++++++++----------- 2 files changed, 169 insertions(+), 127 deletions(-) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index b16526e65e..5f18995d3d 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -138,11 +138,6 @@ typedef struct * array of AppendRelInfo nodes, indexed by child relid. We use that to map * child-table Vars to their inheritance parents. * - * In some cases we need to make names of merged JOIN USING columns unique - * across the whole query, not only per-RTE. If so, unique_using is true - * and using_names is a list of C strings representing names already assigned - * to USING columns. - * * When deparsing plan trees, there is always just a single item in the * deparse_namespace list (since a plan tree never contains Vars with * varlevelsup > 0). We store the Plan node that is the immediate @@ -157,13 +152,13 @@ typedef struct { List *rtable; /* List of RangeTblEntry nodes */ List *rtable_names; /* Parallel list of names for RTEs */ + List *join_using_aliases; /* Parallel list of join using aliases */ List *rtable_columns; /* Parallel list of deparse_columns structs */ List *subplans; /* List of Plan trees for SubPlans */ List *ctes; /* List of CommonTableExpr nodes */ AppendRelInfo **appendrels; /* Array of AppendRelInfo nodes, or NULL */ /* Workspace for column alias assignment: */ bool unique_using; /* Are we making USING names globally unique */ - List *using_names; /* List of assigned names for USING columns */ /* Remaining fields are used only when deparsing a Plan tree: */ Plan *plan; /* immediate parent of current expression */ List *ancestors; /* ancestors of plan */ @@ -3841,6 +3836,7 @@ set_rtable_names(deparse_namespace *dpns, List *parent_namespaces, { RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc); char *refname; + char *join_using_alias; /* Just in case this takes an unreasonable amount of time ... */ CHECK_FOR_INTERRUPTS(); @@ -3871,6 +3867,16 @@ set_rtable_names(deparse_namespace *dpns, List *parent_namespaces, refname = rte->eref->aliasname; } + if (rte->rtekind == RTE_JOIN && rte->joinmergedcols) + { + if (rte->join_using_alias) + join_using_alias = rte->join_using_alias->aliasname; + else + join_using_alias = "ju"; + } + else + join_using_alias = NULL; + /* * If the selected name isn't unique, append digits to make it so, and * make a new hash entry for it once we've got a unique name. For a @@ -3918,7 +3924,49 @@ set_rtable_names(deparse_namespace *dpns, List *parent_namespaces, } } + if (join_using_alias) + { + hentry = (NameHashEntry *) hash_search(names_hash, + join_using_alias, + HASH_ENTER, + &found); + if (found) + { + /* Name already in use, must choose a new one */ + int refnamelen = strlen(join_using_alias); + char *modname = (char *) palloc(refnamelen + 16); + NameHashEntry *hentry2; + + do + { + hentry->counter++; + for (;;) + { + memcpy(modname, join_using_alias, refnamelen); + sprintf(modname + refnamelen, "_%d", hentry->counter); + if (strlen(modname) < NAMEDATALEN) + break; + /* drop chars from refname to keep all the digits */ + refnamelen = pg_mbcliplen(join_using_alias, refnamelen, + refnamelen - 1); + } + hentry2 = (NameHashEntry *) hash_search(names_hash, + modname, + HASH_ENTER, + &found); + } while (found); + hentry2->counter = 0; /* init new hash entry */ + join_using_alias = modname; + } + else + { + /* Name not previously used, need only initialize hentry */ + hentry->counter = 0; + } + } + dpns->rtable_names = lappend(dpns->rtable_names, refname); + dpns->join_using_aliases = lappend(dpns->join_using_aliases, join_using_alias); rtindex++; } @@ -4219,9 +4267,6 @@ set_using_names(deparse_namespace *dpns, Node *jtnode, List *parentUsing) colname = strVal(list_nth(rte->alias->colnames, i)); /* Make it appropriately unique */ colname = make_colname_unique(colname, dpns, colinfo); - if (dpns->unique_using) - dpns->using_names = lappend(dpns->using_names, - colname); /* Save it as output column name, too */ colinfo->colnames[i] = colname; } @@ -4705,7 +4750,6 @@ colname_is_unique(const char *colname, deparse_namespace *dpns, deparse_columns *colinfo) { int i; - ListCell *lc; /* Check against already-assigned column aliases within RTE */ for (i = 0; i < colinfo->num_cols; i++) @@ -4728,24 +4772,6 @@ colname_is_unique(const char *colname, deparse_namespace *dpns, return false; } - /* Also check against USING-column names that must be globally unique */ - foreach(lc, dpns->using_names) - { - char *oldname = (char *) lfirst(lc); - - if (strcmp(oldname, colname) == 0) - return false; - } - - /* Also check against names already assigned for parent-join USING cols */ - foreach(lc, colinfo->parentUsing) - { - char *oldname = (char *) lfirst(lc); - - if (strcmp(oldname, colname) == 0) - return false; - } - return true; } @@ -7047,6 +7073,7 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context) AttrNumber varattno; deparse_columns *colinfo; char *refname; + char *join_using_alias; char *attname; /* Find appropriate nesting depth */ @@ -7130,6 +7157,10 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context) rte = rt_fetch(varno, dpns->rtable); refname = (char *) list_nth(dpns->rtable_names, varno - 1); + if (dpns->join_using_aliases) + join_using_alias = (char *) list_nth(dpns->join_using_aliases, varno - 1); + else + join_using_alias = NULL; colinfo = deparse_columns_fetch(varno, dpns); attnum = varattno; } @@ -7241,6 +7272,11 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context) appendStringInfoString(buf, quote_identifier(refname)); appendStringInfoChar(buf, '.'); } + else if (join_using_alias && (context->varprefix || attname == NULL)) + { + appendStringInfoString(buf, quote_identifier(join_using_alias)); + appendStringInfoChar(buf, '.'); + } if (attname) appendStringInfoString(buf, quote_identifier(attname)); else @@ -11022,6 +11058,7 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context) JoinExpr *j = (JoinExpr *) jtnode; deparse_columns *colinfo = deparse_columns_fetch(j->rtindex, dpns); bool need_paren_on_right; + char *join_using_alias = list_nth(dpns->join_using_aliases, j->rtindex - 1); need_paren_on_right = PRETTY_PAREN(context) && !IsA(j->rarg, RangeTblRef) && @@ -11094,9 +11131,14 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context) } appendStringInfoChar(buf, ')'); - if (j->join_using_alias) + /* + * If it's a generated JOIN USING alias, only print it if we + * really need it to qualify some column. + */ + if (j->join_using_alias || + (join_using_alias && dpns->unique_using)) appendStringInfo(buf, " AS %s", - quote_identifier(j->join_using_alias->aliasname)); + quote_identifier(join_using_alias)); } else if (j->quals) { diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out index 509e930fc7..0eb68cb6d7 100644 --- a/src/test/regress/expected/create_view.out +++ b/src/test/regress/expected/create_view.out @@ -910,17 +910,17 @@ select pg_get_viewdef('v2a', true); (1 row) select pg_get_viewdef('v3', true); - pg_get_viewdef -------------------------------- - SELECT b, + - tt3.c, + - tt2.a, + - tt3.ax, + - tt4.ay, + - tt4.q + - FROM tt2 + - JOIN tt3 USING (b, c) + - FULL JOIN tt4 USING (b); + pg_get_viewdef +--------------------------------------- + SELECT ju_1.b, + + tt3.c, + + tt2.a, + + tt3.ax, + + tt4.ay, + + tt4.q + + FROM tt2 + + JOIN tt3 USING (b, c) AS ju + + FULL JOIN tt4 USING (b) AS ju_1; (1 row) alter table tt2 add column d int; @@ -976,17 +976,17 @@ select pg_get_viewdef('v2a', true); (1 row) select pg_get_viewdef('v3', true); - pg_get_viewdef -------------------------------- - SELECT b, + - tt3.c, + - tt2.a, + - tt3.ax, + - tt4.ay, + - tt4.q + - FROM tt2 + - JOIN tt3 USING (b, c) + - FULL JOIN tt4 USING (b); + pg_get_viewdef +--------------------------------------- + SELECT ju_1.b, + + tt3.c, + + tt2.a, + + tt3.ax, + + tt4.ay, + + tt4.q + + FROM tt2 + + JOIN tt3 USING (b, c) AS ju + + FULL JOIN tt4 USING (b) AS ju_1; (1 row) alter table tt3 rename c to d; @@ -1041,17 +1041,17 @@ select pg_get_viewdef('v2a', true); (1 row) select pg_get_viewdef('v3', true); - pg_get_viewdef ------------------------------------------- - SELECT b, + - tt3.c, + - tt2.a, + - tt3.ax, + - tt4.ay, + - tt4.q + - FROM tt2 + - JOIN tt3 tt3(ax, b, c) USING (b, c)+ - FULL JOIN tt4 USING (b); + pg_get_viewdef +------------------------------------------------ + SELECT ju_1.b, + + tt3.c, + + tt2.a, + + tt3.ax, + + tt4.ay, + + tt4.q + + FROM tt2 + + JOIN tt3 tt3(ax, b, c) USING (b, c) AS ju+ + FULL JOIN tt4 USING (b) AS ju_1; (1 row) alter table tt3 add column c int; @@ -1107,17 +1107,17 @@ select pg_get_viewdef('v2a', true); (1 row) select pg_get_viewdef('v3', true); - pg_get_viewdef --------------------------------------------------- - SELECT b, + - tt3.c, + - tt2.a, + - tt3.ax, + - tt4.ay, + - tt4.q + - FROM tt2 + - JOIN tt3 tt3(ax, b, c, c_1, e) USING (b, c)+ - FULL JOIN tt4 USING (b); + pg_get_viewdef +-------------------------------------------------------- + SELECT ju_1.b, + + tt3.c, + + tt2.a, + + tt3.ax, + + tt4.ay, + + tt4.q + + FROM tt2 + + JOIN tt3 tt3(ax, b, c, c_1, e) USING (b, c) AS ju+ + FULL JOIN tt4 USING (b) AS ju_1; (1 row) alter table tt2 drop column d; @@ -1172,17 +1172,17 @@ select pg_get_viewdef('v2a', true); (1 row) select pg_get_viewdef('v3', true); - pg_get_viewdef --------------------------------------------------- - SELECT b, + - tt3.c, + - tt2.a, + - tt3.ax, + - tt4.ay, + - tt4.q + - FROM tt2 + - JOIN tt3 tt3(ax, b, c, c_1, e) USING (b, c)+ - FULL JOIN tt4 USING (b); + pg_get_viewdef +-------------------------------------------------------- + SELECT ju_1.b, + + tt3.c, + + tt2.a, + + tt3.ax, + + tt4.ay, + + tt4.q + + FROM tt2 + + JOIN tt3 tt3(ax, b, c, c_1, e) USING (b, c) AS ju+ + FULL JOIN tt4 USING (b) AS ju_1; (1 row) create table tt5 (a int, b int); @@ -1276,14 +1276,14 @@ select pg_get_viewdef('vv2', true); v.e + FROM ( VALUES (1,2,3,4,5)) v(a, b, c, d, e)+ UNION ALL + - SELECT x AS a, + + SELECT ju.x AS a, + tt7.y AS b, + tt8.z AS c, + - tt8x.x_1 AS d, + + tt8x.x AS d, + tt8x.z AS e + FROM tt7 + - FULL JOIN tt8 USING (x), + - tt8 tt8x(x_1, z); + FULL JOIN tt8 USING (x) AS ju, + + tt8 tt8x; (1 row) create view vv3 as @@ -1303,16 +1303,16 @@ select pg_get_viewdef('vv3', true); v.f + FROM ( VALUES (1,2,3,4,5,6)) v(a, b, c, x, e, f)+ UNION ALL + - SELECT x AS a, + + SELECT ju.x AS a, + tt7.y AS b, + tt8.z AS c, + - x_1 AS x, + + ju_1.x, + tt7x.y AS e, + tt8x.z AS f + FROM tt7 + - FULL JOIN tt8 USING (x), + - tt7 tt7x(x_1, y) + - FULL JOIN tt8 tt8x(x_1, z) USING (x_1); + FULL JOIN tt8 USING (x) AS ju, + + tt7 tt7x + + FULL JOIN tt8 tt8x USING (x) AS ju_1; (1 row) create view vv4 as @@ -1333,18 +1333,18 @@ select pg_get_viewdef('vv4', true); v.g + FROM ( VALUES (1,2,3,4,5,6,7)) v(a, b, c, x, e, f, g)+ UNION ALL + - SELECT x AS a, + + SELECT ju.x AS a, + tt7.y AS b, + tt8.z AS c, + - x_1 AS x, + + ju_2.x, + tt7x.y AS e, + tt8x.z AS f, + tt8y.z AS g + FROM tt7 + - FULL JOIN tt8 USING (x), + - tt7 tt7x(x_1, y) + - FULL JOIN tt8 tt8x(x_1, z) USING (x_1) + - FULL JOIN tt8 tt8y(x_1, z) USING (x_1); + FULL JOIN tt8 USING (x) AS ju, + + tt7 tt7x + + FULL JOIN tt8 tt8x USING (x) AS ju_1 + + FULL JOIN tt8 tt8y USING (x) AS ju_2; (1 row) alter table tt7 add column zz int; @@ -1361,14 +1361,14 @@ select pg_get_viewdef('vv2', true); v.e + FROM ( VALUES (1,2,3,4,5)) v(a, b, c, d, e)+ UNION ALL + - SELECT x AS a, + + SELECT ju.x AS a, + tt7.y AS b, + tt8.z AS c, + - tt8x.x_1 AS d, + + tt8x.x AS d, + tt8x.z AS e + FROM tt7 + - FULL JOIN tt8 USING (x), + - tt8 tt8x(x_1, z, z2); + FULL JOIN tt8 USING (x) AS ju, + + tt8 tt8x; (1 row) select pg_get_viewdef('vv3', true); @@ -1382,16 +1382,16 @@ select pg_get_viewdef('vv3', true); v.f + FROM ( VALUES (1,2,3,4,5,6)) v(a, b, c, x, e, f)+ UNION ALL + - SELECT x AS a, + + SELECT ju.x AS a, + tt7.y AS b, + tt8.z AS c, + - x_1 AS x, + + ju_1.x, + tt7x.y AS e, + tt8x.z AS f + FROM tt7 + - FULL JOIN tt8 USING (x), + - tt7 tt7x(x_1, y, z) + - FULL JOIN tt8 tt8x(x_1, z, z2) USING (x_1); + FULL JOIN tt8 USING (x) AS ju, + + tt7 tt7x + + FULL JOIN tt8 tt8x USING (x) AS ju_1; (1 row) select pg_get_viewdef('vv4', true); @@ -1406,18 +1406,18 @@ select pg_get_viewdef('vv4', true); v.g + FROM ( VALUES (1,2,3,4,5,6,7)) v(a, b, c, x, e, f, g)+ UNION ALL + - SELECT x AS a, + + SELECT ju.x AS a, + tt7.y AS b, + tt8.z AS c, + - x_1 AS x, + + ju_2.x, + tt7x.y AS e, + tt8x.z AS f, + tt8y.z AS g + FROM tt7 + - FULL JOIN tt8 USING (x), + - tt7 tt7x(x_1, y, z) + - FULL JOIN tt8 tt8x(x_1, z, z2) USING (x_1) + - FULL JOIN tt8 tt8y(x_1, z, z2) USING (x_1); + FULL JOIN tt8 USING (x) AS ju, + + tt7 tt7x + + FULL JOIN tt8 tt8x USING (x) AS ju_1 + + FULL JOIN tt8 tt8y USING (x) AS ju_2; (1 row) -- Implicit coercions in a JOIN USING create issues similar to FULL JOIN @@ -1438,14 +1438,14 @@ select pg_get_viewdef('vv2a', true); v.e + FROM ( VALUES (now(),2,3,now(),5)) v(a, b, c, d, e)+ UNION ALL + - SELECT x AS a, + + SELECT ju.x AS a, + tt7a.y AS b, + tt8a.z AS c, + - tt8ax.x_1 AS d, + + tt8ax.x AS d, + tt8ax.z AS e + FROM tt7a + - LEFT JOIN tt8a USING (x), + - tt8a tt8ax(x_1, z); + LEFT JOIN tt8a USING (x) AS ju, + + tt8a tt8ax; (1 row) -- @@ -1498,14 +1498,14 @@ select pg_get_viewdef('vv6', true); alter table tt11 add column z int; select pg_get_viewdef('vv6', true); - pg_get_viewdef ------------------------------- - SELECT tt11.x, + - tt11.y, + - tt12.z, + - tt13.q + - FROM tt11 tt11(x, y, z_1)+ - JOIN tt12 USING (x) + + pg_get_viewdef +--------------------------- + SELECT tt11.x, + + tt11.y, + + tt12.z, + + tt13.q + + FROM tt11 + + JOIN tt12 USING (x) + JOIN tt13 USING (z); (1 row) -- 2.35.1