public inbox for [email protected]
help / color / mirror / Atom feedUse JOIN USING aliases in ruleutils.c
3+ messages / 2 participants
[nested] [flat]
* Use JOIN USING aliases in ruleutils.c
@ 2021-12-20 11:30 Peter Eisentraut <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Peter Eisentraut @ 2021-12-20 11:30 UTC (permalink / raw)
To: pgsql-hackers
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.
I made it so that new JOIN USING aliases are only created if needed in
the query, since we already have the logic of has_dangerous_join_using()
to compute when that is needed. We could probably do away with that too
and always use them, but I think that would be surprising and not what
people want.
The regression test changes illustrate the effects very well.
This is PoC-level right now. You will find blatant code duplication in
set_rtable_names(), and for now I have only commented out some code that
could be removed, not actually removed it.
From da15f7571f43e8eef279c3e66530d67ae9b5ef7f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Mon, 20 Dec 2021 11:37:15 +0100
Subject: [PATCH v1] 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 everwhere.
---
src/backend/utils/adt/ruleutils.c | 84 +++++++++-
src/test/regress/expected/create_view.out | 194 +++++++++++-----------
2 files changed, 178 insertions(+), 100 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 8da525c715..6e8f7efc4e 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -157,13 +157,16 @@ 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 */
+#if 0
List *using_names; /* List of assigned names for USING columns */
+#endif
/* Remaining fields are used only when deparsing a Plan tree: */
Plan *plan; /* immediate parent of current expression */
List *ancestors; /* ancestors of plan */
@@ -3788,6 +3791,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();
@@ -3818,6 +3822,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
@@ -3865,7 +3879,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++;
}
@@ -4166,9 +4222,11 @@ 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 0
if (dpns->unique_using)
dpns->using_names = lappend(dpns->using_names,
colname);
+#endif
/* Save it as output column name, too */
colinfo->colnames[i] = colname;
}
@@ -4652,7 +4710,7 @@ colname_is_unique(const char *colname, deparse_namespace *dpns,
deparse_columns *colinfo)
{
int i;
- ListCell *lc;
+ //ListCell *lc;
/* Check against already-assigned column aliases within RTE */
for (i = 0; i < colinfo->num_cols; i++)
@@ -4675,6 +4733,7 @@ colname_is_unique(const char *colname, deparse_namespace *dpns,
return false;
}
+#if 0
/* Also check against USING-column names that must be globally unique */
foreach(lc, dpns->using_names)
{
@@ -4683,7 +4742,9 @@ colname_is_unique(const char *colname, deparse_namespace *dpns,
if (strcmp(oldname, colname) == 0)
return false;
}
+#endif
+#if 0
/* Also check against names already assigned for parent-join USING cols */
foreach(lc, colinfo->parentUsing)
{
@@ -4692,6 +4753,7 @@ colname_is_unique(const char *colname, deparse_namespace *dpns,
if (strcmp(oldname, colname) == 0)
return false;
}
+#endif
return true;
}
@@ -6994,6 +7056,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 */
@@ -7077,6 +7140,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;
}
@@ -7188,6 +7255,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
@@ -10959,6 +11031,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) &&
@@ -11031,9 +11104,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 f50ef76685..11a64112bc 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.34.1
Attachments:
[text/plain] v1-0001-Use-JOIN-USING-aliases-in-ruleutils.c.patch (21.4K, ../../[email protected]/2-v1-0001-Use-JOIN-USING-aliases-in-ruleutils.c.patch)
download | inline diff:
From da15f7571f43e8eef279c3e66530d67ae9b5ef7f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Mon, 20 Dec 2021 11:37:15 +0100
Subject: [PATCH v1] 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 everwhere.
---
src/backend/utils/adt/ruleutils.c | 84 +++++++++-
src/test/regress/expected/create_view.out | 194 +++++++++++-----------
2 files changed, 178 insertions(+), 100 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 8da525c715..6e8f7efc4e 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -157,13 +157,16 @@ 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 */
+#if 0
List *using_names; /* List of assigned names for USING columns */
+#endif
/* Remaining fields are used only when deparsing a Plan tree: */
Plan *plan; /* immediate parent of current expression */
List *ancestors; /* ancestors of plan */
@@ -3788,6 +3791,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();
@@ -3818,6 +3822,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
@@ -3865,7 +3879,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++;
}
@@ -4166,9 +4222,11 @@ 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 0
if (dpns->unique_using)
dpns->using_names = lappend(dpns->using_names,
colname);
+#endif
/* Save it as output column name, too */
colinfo->colnames[i] = colname;
}
@@ -4652,7 +4710,7 @@ colname_is_unique(const char *colname, deparse_namespace *dpns,
deparse_columns *colinfo)
{
int i;
- ListCell *lc;
+ //ListCell *lc;
/* Check against already-assigned column aliases within RTE */
for (i = 0; i < colinfo->num_cols; i++)
@@ -4675,6 +4733,7 @@ colname_is_unique(const char *colname, deparse_namespace *dpns,
return false;
}
+#if 0
/* Also check against USING-column names that must be globally unique */
foreach(lc, dpns->using_names)
{
@@ -4683,7 +4742,9 @@ colname_is_unique(const char *colname, deparse_namespace *dpns,
if (strcmp(oldname, colname) == 0)
return false;
}
+#endif
+#if 0
/* Also check against names already assigned for parent-join USING cols */
foreach(lc, colinfo->parentUsing)
{
@@ -4692,6 +4753,7 @@ colname_is_unique(const char *colname, deparse_namespace *dpns,
if (strcmp(oldname, colname) == 0)
return false;
}
+#endif
return true;
}
@@ -6994,6 +7056,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 */
@@ -7077,6 +7140,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;
}
@@ -7188,6 +7255,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
@@ -10959,6 +11031,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) &&
@@ -11031,9 +11104,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 f50ef76685..11a64112bc 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.34.1
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Use JOIN USING aliases in ruleutils.c
@ 2022-03-23 15:14 Tom Lane <[email protected]>
2022-03-25 15:09 ` Re: Use JOIN USING aliases in ruleutils.c Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Tom Lane @ 2022-03-23 15:14 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
Peter Eisentraut <[email protected]> writes:
> 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.
I looked this over briefly, and came away fairly dissatisfied.
My big fear is that it will reduce portability of pg_dump output:
views that would have loaded successfully into pre-v14 servers
no longer will, and your chances of porting them to other RDBMSes
probably go down too. Once v13 becomes EOL, that will be less of a
concern, but I think it's a valid objection for awhile yet.
Also, it doesn't seem like we're getting much in return for the
portability hazard. AFAICS the patch actually makes a net addition
of code to ruleutils.c, which perhaps means that you've not worked
hard enough on removing no-longer-needed code. Maybe there's an
argument that the new output is more readable, but these regression
test changes don't look like any huge step forward to me.
> I also just named the generated
> aliases "ju" with numbers added, maybe there are other ideas for how to
> generate these names.
For my taste, names like "join_N" would be better.
On the whole, I'd recommend putting this idea on the back burner
for three or four years more.
regards, tom lane
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Use JOIN USING aliases in ruleutils.c
2022-03-23 15:14 Re: Use JOIN USING aliases in ruleutils.c Tom Lane <[email protected]>
@ 2022-03-25 15:09 ` Peter Eisentraut <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Peter Eisentraut @ 2022-03-25 15:09 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: pgsql-hackers
On 23.03.22 16:14, Tom Lane wrote:
> My big fear is that it will reduce portability of pg_dump output:
> views that would have loaded successfully into pre-v14 servers
> no longer will, and your chances of porting them to other RDBMSes
> probably go down too. Once v13 becomes EOL, that will be less of a
> concern, but I think it's a valid objection for awhile yet.
That's a good point. I'll withdraw the patch for now.
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2022-03-25 15:09 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-12-20 11:30 Use JOIN USING aliases in ruleutils.c Peter Eisentraut <[email protected]>
2022-03-23 15:14 Re: Use JOIN USING aliases in ruleutils.c Tom Lane <[email protected]>
2022-03-25 15:09 ` Peter Eisentraut <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox