Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commit82c87af

Browse files
committed
Make INSERT-from-multiple-VALUES-rows handle domain target columns.
Commita3c7a99 fixed some cases involving target columns that arearrays or composites by applying transformAssignedExpr to the VALUESentries, and then stripping off any assignment ArrayRefs orFieldStores that the transformation added. But I forgot about domainsover arrays or composites :-(. Such cases would either fail withsurprising complaints about mismatched datatypes, or insert unexpectedcoercions that could lead to odd results. To fix, extend thestripping logic to get rid of CoerceToDomain if it's atop an ArrayRefor FieldStore.While poking at this, I realized that there's a poorly documented andnot-at-all-tested behavior nearby: we coerce each VALUES column tothe domain type separately, and rely on the rewriter to merge thoseoperations so that the domain constraints are checked only once.If that merging did not happen, it's entirely possible that we'd getunexpected domain constraint failures due to checking apartially-updated container value. There's no bug there, but whilewe're here let's improve the commentary about it and add some testcases that explicitly exercise that behavior.Per bug #18393 from Pablo Kharo. Back-patch to all supportedbranches.Discussion:https://postgr.es/m/18393-65fedb1a0de9260d@postgresql.org
1 parentdc1503d commit82c87af

File tree

5 files changed

+227
-11
lines changed

5 files changed

+227
-11
lines changed

‎src/backend/parser/analyze.c

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -995,17 +995,28 @@ transformInsertRow(ParseState *pstate, List *exprlist,
995995

996996
if (strip_indirection)
997997
{
998+
/*
999+
* We need to remove top-level FieldStores and SubscriptingRefs,
1000+
* as well as any CoerceToDomain appearing above one of those ---
1001+
* but not a CoerceToDomain that isn't above one of those.
1002+
*/
9981003
while (expr)
9991004
{
1000-
if (IsA(expr,FieldStore))
1005+
Expr*subexpr=expr;
1006+
1007+
while (IsA(subexpr,CoerceToDomain))
1008+
{
1009+
subexpr= ((CoerceToDomain*)subexpr)->arg;
1010+
}
1011+
if (IsA(subexpr,FieldStore))
10011012
{
1002-
FieldStore*fstore= (FieldStore*)expr;
1013+
FieldStore*fstore= (FieldStore*)subexpr;
10031014

10041015
expr= (Expr*)linitial(fstore->newvals);
10051016
}
1006-
elseif (IsA(expr,SubscriptingRef))
1017+
elseif (IsA(subexpr,SubscriptingRef))
10071018
{
1008-
SubscriptingRef*sbsref= (SubscriptingRef*)expr;
1019+
SubscriptingRef*sbsref= (SubscriptingRef*)subexpr;
10091020

10101021
if (sbsref->refassgnexpr==NULL)
10111022
break;

‎src/backend/parser/parse_target.c

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -813,7 +813,16 @@ transformAssignmentIndirection(ParseState *pstate,
813813
fstore->fieldnums=list_make1_int(attnum);
814814
fstore->resulttype=baseTypeId;
815815

816-
/* If target is a domain, apply constraints */
816+
/*
817+
* If target is a domain, apply constraints. Notice that this
818+
* isn't totally right: the expression tree we build would check
819+
* the domain's constraints on a composite value with only this
820+
* one field populated or updated, possibly leading to an unwanted
821+
* failure. The rewriter will merge together any subfield
822+
* assignments to the same table column, resulting in the domain's
823+
* constraints being checked only once after we've assigned to all
824+
* the fields that the INSERT or UPDATE means to.
825+
*/
817826
if (baseTypeId!=targetTypeId)
818827
returncoerce_to_domain((Node*)fstore,
819828
baseTypeId,baseTypeMod,
@@ -943,7 +952,12 @@ transformAssignmentSubscripts(ParseState *pstate,
943952
subscripts,
944953
rhs);
945954

946-
/* If target was a domain over container, need to coerce up to the domain */
955+
/*
956+
* If target was a domain over container, need to coerce up to the domain.
957+
* As in transformAssignmentIndirection, this coercion is premature if the
958+
* query assigns to multiple elements of the container; but we'll fix that
959+
* during query rewrite.
960+
*/
947961
if (containerType!=targetTypeId)
948962
{
949963
Oidresulttype=exprType(result);

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,9 +1029,9 @@ process_matched_tle(TargetEntry *src_tle,
10291029
* resulting in each assignment containing a CoerceToDomain node over a
10301030
* FieldStore or SubscriptingRef. These should have matching target
10311031
* domains, so we strip them and reconstitute a single CoerceToDomain over
1032-
* the combined FieldStore/SubscriptingRef nodes. (Notice that this has the
1033-
* result that the domain's checks are applied only after we do all the
1034-
* field or element updates, not after each one. This is arguably desirable.)
1032+
* the combined FieldStore/SubscriptingRef nodes. (Notice that this has
1033+
*theresult that the domain's checks are applied only after we do all
1034+
*thefield or element updates, not after each one. This is desirable.)
10351035
*----------
10361036
*/
10371037
src_expr= (Node*)src_tle->expr;

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

Lines changed: 115 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,121 @@ Rules:
159159

160160
drop table inserttest2;
161161
drop table inserttest;
162-
drop type insert_test_type;
162+
-- Make the same tests with domains over the array and composite fields
163+
create domain insert_pos_ints as int[] check (value[1] > 0);
164+
create domain insert_test_domain as insert_test_type
165+
check ((value).if2[1] is not null);
166+
create table inserttesta (f1 int, f2 insert_pos_ints);
167+
create table inserttestb (f3 insert_test_domain, f4 insert_test_domain[]);
168+
insert into inserttesta (f2[1], f2[2]) values (1,2);
169+
insert into inserttesta (f2[1], f2[2]) values (3,4), (5,6);
170+
insert into inserttesta (f2[1], f2[2]) select 7,8;
171+
insert into inserttesta (f2[1], f2[2]) values (1,default); -- not supported
172+
ERROR: cannot set an array element to DEFAULT
173+
LINE 1: insert into inserttesta (f2[1], f2[2]) values (1,default);
174+
^
175+
insert into inserttesta (f2[1], f2[2]) values (0,2);
176+
ERROR: value for domain insert_pos_ints violates check constraint "insert_pos_ints_check"
177+
insert into inserttesta (f2[1], f2[2]) values (3,4), (0,6);
178+
ERROR: value for domain insert_pos_ints violates check constraint "insert_pos_ints_check"
179+
insert into inserttesta (f2[1], f2[2]) select 0,8;
180+
ERROR: value for domain insert_pos_ints violates check constraint "insert_pos_ints_check"
181+
insert into inserttestb (f3.if1, f3.if2) values (1,array['foo']);
182+
insert into inserttestb (f3.if1, f3.if2) values (1,'{foo}'), (2,'{bar}');
183+
insert into inserttestb (f3.if1, f3.if2) select 3, '{baz,quux}';
184+
insert into inserttestb (f3.if1, f3.if2) values (1,default); -- not supported
185+
ERROR: cannot set a subfield to DEFAULT
186+
LINE 1: insert into inserttestb (f3.if1, f3.if2) values (1,default);
187+
^
188+
insert into inserttestb (f3.if1, f3.if2) values (1,array[null]);
189+
ERROR: value for domain insert_test_domain violates check constraint "insert_test_domain_check"
190+
insert into inserttestb (f3.if1, f3.if2) values (1,'{null}'), (2,'{bar}');
191+
ERROR: value for domain insert_test_domain violates check constraint "insert_test_domain_check"
192+
insert into inserttestb (f3.if1, f3.if2) select 3, '{null,quux}';
193+
ERROR: value for domain insert_test_domain violates check constraint "insert_test_domain_check"
194+
insert into inserttestb (f3.if2[1], f3.if2[2]) values ('foo', 'bar');
195+
insert into inserttestb (f3.if2[1], f3.if2[2]) values ('foo', 'bar'), ('baz', 'quux');
196+
insert into inserttestb (f3.if2[1], f3.if2[2]) select 'bear', 'beer';
197+
insert into inserttestb (f3, f4[1].if2[1], f4[1].if2[2]) values (row(1,'{x}'), 'foo', 'bar');
198+
insert into inserttestb (f3, f4[1].if2[1], f4[1].if2[2]) values (row(1,'{x}'), 'foo', 'bar'), (row(2,'{y}'), 'baz', 'quux');
199+
insert into inserttestb (f3, f4[1].if2[1], f4[1].if2[2]) select row(1,'{x}')::insert_test_domain, 'bear', 'beer';
200+
select * from inserttesta;
201+
f1 | f2
202+
----+-------
203+
| {1,2}
204+
| {3,4}
205+
| {5,6}
206+
| {7,8}
207+
(4 rows)
208+
209+
select * from inserttestb;
210+
f3 | f4
211+
------------------+------------------------
212+
(1,{foo}) |
213+
(1,{foo}) |
214+
(2,{bar}) |
215+
(3,"{baz,quux}") |
216+
(,"{foo,bar}") |
217+
(,"{foo,bar}") |
218+
(,"{baz,quux}") |
219+
(,"{bear,beer}") |
220+
(1,{x}) | {"(,\"{foo,bar}\")"}
221+
(1,{x}) | {"(,\"{foo,bar}\")"}
222+
(2,{y}) | {"(,\"{baz,quux}\")"}
223+
(1,{x}) | {"(,\"{bear,beer}\")"}
224+
(12 rows)
225+
226+
-- also check reverse-listing
227+
create table inserttest2 (f1 bigint, f2 text);
228+
create rule irule1 as on insert to inserttest2 do also
229+
insert into inserttestb (f3.if2[1], f3.if2[2])
230+
values (new.f1,new.f2);
231+
create rule irule2 as on insert to inserttest2 do also
232+
insert into inserttestb (f4[1].if1, f4[1].if2[2])
233+
values (1,'fool'),(new.f1,new.f2);
234+
create rule irule3 as on insert to inserttest2 do also
235+
insert into inserttestb (f4[1].if1, f4[1].if2[2])
236+
select new.f1, new.f2;
237+
\d+ inserttest2
238+
Table "public.inserttest2"
239+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
240+
--------+--------+-----------+----------+---------+----------+--------------+-------------
241+
f1 | bigint | | | | plain | |
242+
f2 | text | | | | extended | |
243+
Rules:
244+
irule1 AS
245+
ON INSERT TO inserttest2 DO INSERT INTO inserttestb (f3.if2[1], f3.if2[2])
246+
VALUES (new.f1, new.f2)
247+
irule2 AS
248+
ON INSERT TO inserttest2 DO INSERT INTO inserttestb (f4[1].if1, f4[1].if2[2]) VALUES (1,'fool'::text), (new.f1,new.f2)
249+
irule3 AS
250+
ON INSERT TO inserttest2 DO INSERT INTO inserttestb (f4[1].if1, f4[1].if2[2]) SELECT new.f1,
251+
new.f2
252+
253+
drop table inserttest2;
254+
drop table inserttesta;
255+
drop table inserttestb;
256+
drop domain insert_pos_ints;
257+
drop domain insert_test_domain;
258+
-- Verify that multiple inserts to subfields of a domain-over-container
259+
-- check the domain constraints only on the finished value
260+
create domain insert_nnarray as int[]
261+
check (value[1] is not null and value[2] is not null);
262+
create domain insert_test_domain as insert_test_type
263+
check ((value).if1 is not null and (value).if2 is not null);
264+
create table inserttesta (f1 insert_nnarray);
265+
insert into inserttesta (f1[1]) values (1); -- fail
266+
ERROR: value for domain insert_nnarray violates check constraint "insert_nnarray_check"
267+
insert into inserttesta (f1[1], f1[2]) values (1, 2);
268+
create table inserttestb (f1 insert_test_domain);
269+
insert into inserttestb (f1.if1) values (1); -- fail
270+
ERROR: value for domain insert_test_domain violates check constraint "insert_test_domain_check"
271+
insert into inserttestb (f1.if1, f1.if2) values (1, '{foo}');
272+
drop table inserttesta;
273+
drop table inserttestb;
274+
drop domain insert_nnarray;
275+
drop type insert_test_type cascade;
276+
NOTICE: drop cascades to type insert_test_domain
163277
-- direct partition inserts should check partition bound constraint
164278
create table range_parted (
165279
a text,

‎src/test/regress/sql/insert.sql

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,84 @@ create rule irule3 as on insert to inserttest2 do also
8383

8484
droptable inserttest2;
8585
droptable inserttest;
86-
droptype insert_test_type;
86+
87+
-- Make the same tests with domains over the array and composite fields
88+
89+
createdomaininsert_pos_intsasint[]check (value[1]>0);
90+
91+
createdomaininsert_test_domainas insert_test_type
92+
check ((value).if2[1]is not null);
93+
94+
createtableinserttesta (f1int, f2 insert_pos_ints);
95+
createtableinserttestb (f3 insert_test_domain, f4 insert_test_domain[]);
96+
97+
insert into inserttesta (f2[1], f2[2])values (1,2);
98+
insert into inserttesta (f2[1], f2[2])values (3,4), (5,6);
99+
insert into inserttesta (f2[1], f2[2])select7,8;
100+
insert into inserttesta (f2[1], f2[2])values (1,default);-- not supported
101+
insert into inserttesta (f2[1], f2[2])values (0,2);
102+
insert into inserttesta (f2[1], f2[2])values (3,4), (0,6);
103+
insert into inserttesta (f2[1], f2[2])select0,8;
104+
105+
insert into inserttestb (f3.if1,f3.if2)values (1,array['foo']);
106+
insert into inserttestb (f3.if1,f3.if2)values (1,'{foo}'), (2,'{bar}');
107+
insert into inserttestb (f3.if1,f3.if2)select3,'{baz,quux}';
108+
insert into inserttestb (f3.if1,f3.if2)values (1,default);-- not supported
109+
insert into inserttestb (f3.if1,f3.if2)values (1,array[null]);
110+
insert into inserttestb (f3.if1,f3.if2)values (1,'{null}'), (2,'{bar}');
111+
insert into inserttestb (f3.if1,f3.if2)select3,'{null,quux}';
112+
113+
insert into inserttestb (f3.if2[1],f3.if2[2])values ('foo','bar');
114+
insert into inserttestb (f3.if2[1],f3.if2[2])values ('foo','bar'), ('baz','quux');
115+
insert into inserttestb (f3.if2[1],f3.if2[2])select'bear','beer';
116+
117+
insert into inserttestb (f3, f4[1].if2[1], f4[1].if2[2])values (row(1,'{x}'),'foo','bar');
118+
insert into inserttestb (f3, f4[1].if2[1], f4[1].if2[2])values (row(1,'{x}'),'foo','bar'), (row(2,'{y}'),'baz','quux');
119+
insert into inserttestb (f3, f4[1].if2[1], f4[1].if2[2])select row(1,'{x}')::insert_test_domain,'bear','beer';
120+
121+
select*from inserttesta;
122+
select*from inserttestb;
123+
124+
-- also check reverse-listing
125+
createtableinserttest2 (f1bigint, f2text);
126+
createruleirule1ason insert to inserttest2 do also
127+
insert into inserttestb (f3.if2[1],f3.if2[2])
128+
values (new.f1,new.f2);
129+
createruleirule2ason insert to inserttest2 do also
130+
insert into inserttestb (f4[1].if1, f4[1].if2[2])
131+
values (1,'fool'),(new.f1,new.f2);
132+
createruleirule3ason insert to inserttest2 do also
133+
insert into inserttestb (f4[1].if1, f4[1].if2[2])
134+
selectnew.f1,new.f2;
135+
\d+ inserttest2
136+
137+
droptable inserttest2;
138+
droptable inserttesta;
139+
droptable inserttestb;
140+
dropdomain insert_pos_ints;
141+
dropdomain insert_test_domain;
142+
143+
-- Verify that multiple inserts to subfields of a domain-over-container
144+
-- check the domain constraints only on the finished value
145+
146+
createdomaininsert_nnarrayasint[]
147+
check (value[1]is not nulland value[2]is not null);
148+
149+
createdomaininsert_test_domainas insert_test_type
150+
check ((value).if1is not nulland (value).if2is not null);
151+
152+
createtableinserttesta (f1 insert_nnarray);
153+
insert into inserttesta (f1[1])values (1);-- fail
154+
insert into inserttesta (f1[1], f1[2])values (1,2);
155+
156+
createtableinserttestb (f1 insert_test_domain);
157+
insert into inserttestb (f1.if1)values (1);-- fail
158+
insert into inserttestb (f1.if1,f1.if2)values (1,'{foo}');
159+
160+
droptable inserttesta;
161+
droptable inserttestb;
162+
dropdomain insert_nnarray;
163+
droptype insert_test_type cascade;
87164

88165
-- direct partition inserts should check partition bound constraint
89166
createtablerange_parted (

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp