Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 393430f

Browse filesBrowse files
tglsfdctglsfdc
committed
Print the correct aliases for DML target tables in ruleutils.
ruleutils.c blindly printed the user-given alias (or nothing if there hadn't been one) for the target table of INSERT/UPDATE/DELETE queries. That works a large percentage of the time, but not always: for queries appearing in WITH, it's possible that we chose a different alias to avoid conflict with outer-scope names. Since the chosen alias would be used in any Var references to the target table, this'd lead to an inconsistent printout with consequences such as dump/restore failures. The correct logic for printing (or not) a relation alias was embedded in get_from_clause_item. Factor it out to a separate function so that we don't need a jointree node to use it. (Only a limited part of that function can be reached from these new call sites, but this seems like the cleanest non-duplicative factorization.) In passing, I got rid of a redundant "\d+ rules_src" step in rules.sql. Initial report from Jonathan Katz; thanks to Vignesh C for analysis. This has been broken for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/e947fa21-24b2-f922-375a-d4f763ef3e4b@postgresql.org Discussion: https://postgr.es/m/CALDaNm1MMntjmT_NJGp-Z=xbF02qHGAyuSHfYHias3TqQbPF2w@mail.gmail.com
1 parent 881a917 commit 393430f
Copy full SHA for 393430f

File tree

3 files changed

+130
-84
lines changed
Filter options

3 files changed

+130
-84
lines changed

‎src/backend/utils/adt/ruleutils.c

Copy file name to clipboardExpand all lines: src/backend/utils/adt/ruleutils.c
+88-66Lines changed: 88 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,8 @@ static void get_from_clause(Query *query, const char *prefix,
475475
deparse_context *context);
476476
static void get_from_clause_item(Node *jtnode, Query *query,
477477
deparse_context *context);
478+
static void get_rte_alias(RangeTblEntry *rte, int varno, bool use_as,
479+
deparse_context *context);
478480
static void get_column_alias_list(deparse_columns *colinfo,
479481
deparse_context *context);
480482
static void get_from_clause_coldeflist(RangeTblFunction *rtfunc,
@@ -6648,12 +6650,14 @@ get_insert_query_def(Query *query, deparse_context *context,
66486650
context->indentLevel += PRETTYINDENT_STD;
66496651
appendStringInfoChar(buf, ' ');
66506652
}
6651-
appendStringInfo(buf, "INSERT INTO %s ",
6653+
appendStringInfo(buf, "INSERT INTO %s",
66526654
generate_relation_name(rte->relid, NIL));
6653-
/* INSERT requires AS keyword for target alias */
6654-
if (rte->alias != NULL)
6655-
appendStringInfo(buf, "AS %s ",
6656-
quote_identifier(rte->alias->aliasname));
6655+
6656+
/* Print the relation alias, if needed; INSERT requires explicit AS */
6657+
get_rte_alias(rte, query->resultRelation, true, context);
6658+
6659+
/* always want a space here */
6660+
appendStringInfoChar(buf, ' ');
66576661

66586662
/*
66596663
* Add the insert-column-names list. Any indirection decoration needed on
@@ -6835,9 +6839,10 @@ get_update_query_def(Query *query, deparse_context *context,
68356839
appendStringInfo(buf, "UPDATE %s%s",
68366840
only_marker(rte),
68376841
generate_relation_name(rte->relid, NIL));
6838-
if (rte->alias != NULL)
6839-
appendStringInfo(buf, " %s",
6840-
quote_identifier(rte->alias->aliasname));
6842+
6843+
/* Print the relation alias, if needed */
6844+
get_rte_alias(rte, query->resultRelation, false, context);
6845+
68416846
appendStringInfoString(buf, " SET ");
68426847

68436848
/* Deparse targetlist */
@@ -7043,9 +7048,9 @@ get_delete_query_def(Query *query, deparse_context *context,
70437048
appendStringInfo(buf, "DELETE FROM %s%s",
70447049
only_marker(rte),
70457050
generate_relation_name(rte->relid, NIL));
7046-
if (rte->alias != NULL)
7047-
appendStringInfo(buf, " %s",
7048-
quote_identifier(rte->alias->aliasname));
7051+
7052+
/* Print the relation alias, if needed */
7053+
get_rte_alias(rte, query->resultRelation, false, context);
70497054

70507055
/* Add the USING clause if given */
70517056
get_from_clause(query, " USING ", context);
@@ -10887,10 +10892,8 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
1088710892
{
1088810893
int varno = ((RangeTblRef *) jtnode)->rtindex;
1088910894
RangeTblEntry *rte = rt_fetch(varno, query->rtable);
10890-
char *refname = get_rtable_name(varno, context);
1089110895
deparse_columns *colinfo = deparse_columns_fetch(varno, dpns);
1089210896
RangeTblFunction *rtfunc1 = NULL;
10893-
bool printalias;
1089410897

1089510898
if (rte->lateral)
1089610899
appendStringInfoString(buf, "LATERAL ");
@@ -11027,59 +11030,7 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
1102711030
}
1102811031

1102911032
/* Print the relation alias, if needed */
11030-
printalias = false;
11031-
if (rte->alias != NULL)
11032-
{
11033-
/* Always print alias if user provided one */
11034-
printalias = true;
11035-
}
11036-
else if (colinfo->printaliases)
11037-
{
11038-
/* Always print alias if we need to print column aliases */
11039-
printalias = true;
11040-
}
11041-
else if (rte->rtekind == RTE_RELATION)
11042-
{
11043-
/*
11044-
* No need to print alias if it's same as relation name (this
11045-
* would normally be the case, but not if set_rtable_names had to
11046-
* resolve a conflict).
11047-
*/
11048-
if (strcmp(refname, get_relation_name(rte->relid)) != 0)
11049-
printalias = true;
11050-
}
11051-
else if (rte->rtekind == RTE_FUNCTION)
11052-
{
11053-
/*
11054-
* For a function RTE, always print alias. This covers possible
11055-
* renaming of the function and/or instability of the
11056-
* FigureColname rules for things that aren't simple functions.
11057-
* Note we'd need to force it anyway for the columndef list case.
11058-
*/
11059-
printalias = true;
11060-
}
11061-
else if (rte->rtekind == RTE_SUBQUERY ||
11062-
rte->rtekind == RTE_VALUES)
11063-
{
11064-
/*
11065-
* For a subquery, always print alias. This makes the output SQL
11066-
* spec-compliant, even though we allow the alias to be omitted on
11067-
* input.
11068-
*/
11069-
printalias = true;
11070-
}
11071-
else if (rte->rtekind == RTE_CTE)
11072-
{
11073-
/*
11074-
* No need to print alias if it's same as CTE name (this would
11075-
* normally be the case, but not if set_rtable_names had to
11076-
* resolve a conflict).
11077-
*/
11078-
if (strcmp(refname, rte->ctename) != 0)
11079-
printalias = true;
11080-
}
11081-
if (printalias)
11082-
appendStringInfo(buf, " %s", quote_identifier(refname));
11033+
get_rte_alias(rte, varno, false, context);
1108311034

1108411035
/* Print the column definitions or aliases, if needed */
1108511036
if (rtfunc1 && rtfunc1->funccolnames != NIL)
@@ -11217,6 +11168,77 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
1121711168
(int) nodeTag(jtnode));
1121811169
}
1121911170

11171+
/*
11172+
* get_rte_alias - print the relation's alias, if needed
11173+
*
11174+
* If printed, the alias is preceded by a space, or by " AS " if use_as is true.
11175+
*/
11176+
static void
11177+
get_rte_alias(RangeTblEntry *rte, int varno, bool use_as,
11178+
deparse_context *context)
11179+
{
11180+
deparse_namespace *dpns = (deparse_namespace *) linitial(context->namespaces);
11181+
char *refname = get_rtable_name(varno, context);
11182+
deparse_columns *colinfo = deparse_columns_fetch(varno, dpns);
11183+
bool printalias = false;
11184+
11185+
if (rte->alias != NULL)
11186+
{
11187+
/* Always print alias if user provided one */
11188+
printalias = true;
11189+
}
11190+
else if (colinfo->printaliases)
11191+
{
11192+
/* Always print alias if we need to print column aliases */
11193+
printalias = true;
11194+
}
11195+
else if (rte->rtekind == RTE_RELATION)
11196+
{
11197+
/*
11198+
* No need to print alias if it's same as relation name (this would
11199+
* normally be the case, but not if set_rtable_names had to resolve a
11200+
* conflict).
11201+
*/
11202+
if (strcmp(refname, get_relation_name(rte->relid)) != 0)
11203+
printalias = true;
11204+
}
11205+
else if (rte->rtekind == RTE_FUNCTION)
11206+
{
11207+
/*
11208+
* For a function RTE, always print alias. This covers possible
11209+
* renaming of the function and/or instability of the FigureColname
11210+
* rules for things that aren't simple functions. Note we'd need to
11211+
* force it anyway for the columndef list case.
11212+
*/
11213+
printalias = true;
11214+
}
11215+
else if (rte->rtekind == RTE_SUBQUERY ||
11216+
rte->rtekind == RTE_VALUES)
11217+
{
11218+
/*
11219+
* For a subquery, always print alias. This makes the output
11220+
* SQL-spec-compliant, even though we allow such aliases to be omitted
11221+
* on input.
11222+
*/
11223+
printalias = true;
11224+
}
11225+
else if (rte->rtekind == RTE_CTE)
11226+
{
11227+
/*
11228+
* No need to print alias if it's same as CTE name (this would
11229+
* normally be the case, but not if set_rtable_names had to resolve a
11230+
* conflict).
11231+
*/
11232+
if (strcmp(refname, rte->ctename) != 0)
11233+
printalias = true;
11234+
}
11235+
11236+
if (printalias)
11237+
appendStringInfo(context->buf, "%s%s",
11238+
use_as ? " AS " : " ",
11239+
quote_identifier(refname));
11240+
}
11241+
1122011242
/*
1122111243
* get_column_alias_list - print column alias list for an RTE
1122211244
*

‎src/test/regress/expected/rules.out

Copy file name to clipboardExpand all lines: src/test/regress/expected/rules.out
+30-17Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2998,28 +2998,21 @@ select * from rules_log;
29982998
(16 rows)
29992999

30003000
create rule r4 as on delete to rules_src do notify rules_src_deletion;
3001-
\d+ rules_src
3002-
Table "public.rules_src"
3003-
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
3004-
--------+---------+-----------+----------+---------+---------+--------------+-------------
3005-
f1 | integer | | | | plain | |
3006-
f2 | integer | | | 0 | plain | |
3007-
Rules:
3008-
r1 AS
3009-
ON UPDATE TO rules_src DO INSERT INTO rules_log (f1, f2, tag, id) VALUES (old.f1,old.f2,'old'::text,DEFAULT), (new.f1,new.f2,'new'::text,DEFAULT)
3010-
r2 AS
3011-
ON UPDATE TO rules_src DO VALUES (old.f1,old.f2,'old'::text), (new.f1,new.f2,'new'::text)
3012-
r3 AS
3013-
ON INSERT TO rules_src DO INSERT INTO rules_log (f1, f2, tag, id) VALUES (NULL::integer,NULL::integer,'-'::text,DEFAULT), (new.f1,new.f2,'new'::text,DEFAULT)
3014-
r4 AS
3015-
ON DELETE TO rules_src DO
3016-
NOTIFY rules_src_deletion
3017-
30183001
--
30193002
-- Ensure an aliased target relation for insert is correctly deparsed.
30203003
--
30213004
create rule r5 as on insert to rules_src do instead insert into rules_log AS trgt SELECT NEW.* RETURNING trgt.f1, trgt.f2;
30223005
create rule r6 as on update to rules_src do instead UPDATE rules_log AS trgt SET tag = 'updated' WHERE trgt.f1 = new.f1;
3006+
--
3007+
-- Check deparse disambiguation of INSERT/UPDATE/DELETE targets.
3008+
--
3009+
create rule r7 as on delete to rules_src do instead
3010+
with wins as (insert into int4_tbl as trgt values (0) returning *),
3011+
wupd as (update int4_tbl trgt set f1 = f1+1 returning *),
3012+
wdel as (delete from int4_tbl trgt where f1 = 0 returning *)
3013+
insert into rules_log AS trgt select old.* from wins, wupd, wdel
3014+
returning trgt.f1, trgt.f2;
3015+
-- check display of all rules added above
30233016
\d+ rules_src
30243017
Table "public.rules_src"
30253018
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
@@ -3044,6 +3037,26 @@ Rules:
30443037
r6 AS
30453038
ON UPDATE TO rules_src DO INSTEAD UPDATE rules_log trgt SET tag = 'updated'::text
30463039
WHERE trgt.f1 = new.f1
3040+
r7 AS
3041+
ON DELETE TO rules_src DO INSTEAD WITH wins AS (
3042+
INSERT INTO int4_tbl AS trgt_1 (f1)
3043+
VALUES (0)
3044+
RETURNING trgt_1.f1
3045+
), wupd AS (
3046+
UPDATE int4_tbl trgt_1 SET f1 = trgt_1.f1 + 1
3047+
RETURNING trgt_1.f1
3048+
), wdel AS (
3049+
DELETE FROM int4_tbl trgt_1
3050+
WHERE trgt_1.f1 = 0
3051+
RETURNING trgt_1.f1
3052+
)
3053+
INSERT INTO rules_log AS trgt (f1, f2) SELECT old.f1,
3054+
old.f2
3055+
FROM wins,
3056+
wupd,
3057+
wdel
3058+
RETURNING trgt.f1,
3059+
trgt.f2
30473060

30483061
--
30493062
-- Also check multiassignment deparsing.

‎src/test/regress/sql/rules.sql

Copy file name to clipboardExpand all lines: src/test/regress/sql/rules.sql
+12-1Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1015,13 +1015,24 @@ insert into rules_src values(22,23), (33,default);
10151015
select * from rules_src;
10161016
select * from rules_log;
10171017
create rule r4 as on delete to rules_src do notify rules_src_deletion;
1018-
\d+ rules_src
10191018

10201019
--
10211020
-- Ensure an aliased target relation for insert is correctly deparsed.
10221021
--
10231022
create rule r5 as on insert to rules_src do instead insert into rules_log AS trgt SELECT NEW.* RETURNING trgt.f1, trgt.f2;
10241023
create rule r6 as on update to rules_src do instead UPDATE rules_log AS trgt SET tag = 'updated' WHERE trgt.f1 = new.f1;
1024+
1025+
--
1026+
-- Check deparse disambiguation of INSERT/UPDATE/DELETE targets.
1027+
--
1028+
create rule r7 as on delete to rules_src do instead
1029+
with wins as (insert into int4_tbl as trgt values (0) returning *),
1030+
wupd as (update int4_tbl trgt set f1 = f1+1 returning *),
1031+
wdel as (delete from int4_tbl trgt where f1 = 0 returning *)
1032+
insert into rules_log AS trgt select old.* from wins, wupd, wdel
1033+
returning trgt.f1, trgt.f2;
1034+
1035+
-- check display of all rules added above
10251036
\d+ rules_src
10261037

10271038
--

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.