Skip to content

Commit

Permalink
Fix insert select planner crash
Browse files Browse the repository at this point in the history 
The following query was crashing the backend:

```
INSERT INTO field_indirection_test_1 (
  int_col, ct1_col.int_1,ct1_col.int_2
) SELECT 0, 1, 2;
-- crash
```

En passant, added more tests with sublink in distributed_types and found another
query with wrong behavior:

```
INSERT INTO domain_indirection_test (f1,f3.if1) SELECT 0, 1;
ERROR:  could not find a conversion path from type 23 to 17619
-- not the expected ERROR
```

Fixed them by using strip_implicit_coercions() on target entry expression
before checking for the presence of a subscript or fieldstore, else we
fail to find the existing ones and wrongly accept to execute unsafe
query.
  • Loading branch information
@c2main
c2main committed Feb 27, 2025
1 parent ab7c13b commit 900833a
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 3 deletions.
7 changes: 5 additions & 2 deletions src/backend/distributed/planner/insert_select_planner.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1071,10 +1071,13 @@ ReorderInsertSelectTargetLists(Query *originalQuery, RangeTblEntry *insertRte,
TargetEntry *newSubqueryTargetEntry = NULL;
AttrNumber originalAttrNo = get_attnum(insertRelationId,
oldInsertTargetEntry->resname);
Node *expr;

/* we need to explore the underlying expression */
expr = (Node *) oldInsertTargetEntry->expr;
expr = strip_implicit_coercions(expr);
/* see transformInsertRow() for the details */
if (IsA(oldInsertTargetEntry->expr, SubscriptingRef) ||
IsA(oldInsertTargetEntry->expr, FieldStore))
if (IsA(expr, SubscriptingRef) || IsA(expr, FieldStore))
{
ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg(
Expand Down
29 changes: 28 additions & 1 deletion src/test/regress/expected/distributed_types.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,12 @@ HINT: Use the column name to insert or update the composite type as a single va
INSERT INTO field_indirection_test_1 (int_col, ct1_col.int_1) VALUES (0, 1);
ERROR: inserting or modifying composite type fields is not supported
HINT: Use the column name to insert or update the composite type as a single value
INSERT INTO field_indirection_test_1 (int_col, ct1_col.int_1, ct1_col.int_2) SELECT 0, 1, 2;
ERROR: cannot plan distributed INSERT INTO ... SELECT query
HINT: Do not use array references and field stores on the INSERT target list.
INSERT INTO field_indirection_test_1 (int_col, ct1_col.int_1) SELECT 0, 1;
ERROR: cannot plan distributed INSERT INTO ... SELECT query
HINT: Do not use array references and field stores on the INSERT target list.
CREATE TYPE ct2 as (int_2 int, text_1 text, int_1 int);
CREATE TABLE field_indirection_test_2 (int_col int, ct2_col ct2, ct1_col ct1);
SELECT create_distributed_table('field_indirection_test_2', 'int_col');
Expand All @@ -399,10 +405,17 @@ INSERT INTO field_indirection_test_2 (ct2_col.int_1, int_col, ct2_col.text_1, ct
VALUES (0, 1, 'text1', 2), (3, 4, 'text1', 5);
ERROR: inserting or modifying composite type fields is not supported
HINT: Use the column name to insert or update the composite type as a single value
INSERT INTO field_indirection_test_2 (ct2_col.int_1, int_col, ct2_col.text_1, ct1_col.int_2)
SELECT * FROM (VALUES (0, 1, 'text1', 2), (3, 4, 'text1', 5));
ERROR: cannot plan distributed INSERT INTO ... SELECT query
HINT: Do not use array references and field stores on the INSERT target list.
-- not supported (field indirection in update)
UPDATE field_indirection_test_2 SET (ct2_col.text_1, ct1_col.int_2) = ('text2', 10) WHERE int_col=4;
ERROR: inserting or modifying composite type fields is not supported
HINT: Use the column name to insert or update the composite type as a single value
UPDATE field_indirection_test_2 SET (ct2_col.text_1, ct1_col.int_2) = (SELECT 'text2', 10) WHERE int_col=4;
ERROR: inserting or modifying composite type fields is not supported
HINT: Use the column name to insert or update the composite type as a single value
CREATE TYPE two_ints as (if1 int, if2 int);
CREATE DOMAIN domain AS two_ints CHECK ((VALUE).if1 > 0);
CREATE TABLE domain_indirection_test (f1 int, f3 domain, domain_array domain[]);
Expand All @@ -416,22 +429,36 @@ SELECT create_distributed_table('domain_indirection_test', 'f1');
INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) VALUES (0, 1, 2);
ERROR: inserting or modifying composite type fields is not supported
HINT: Use the column name to insert or update the composite type as a single value
INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) SELECT 0, 1, 2;
ERROR: cannot plan distributed INSERT INTO ... SELECT query
HINT: Do not use array references and field stores on the INSERT target list.
INSERT INTO domain_indirection_test (f1,f3.if1) VALUES (0, 1);
ERROR: inserting or modifying composite type fields is not supported
HINT: Use the column name to insert or update the composite type as a single value
INSERT INTO domain_indirection_test (f1,f3.if1) SELECT 0, 1;
ERROR: cannot plan distributed INSERT INTO ... SELECT query
HINT: Do not use array references and field stores on the INSERT target list.
UPDATE domain_indirection_test SET domain_array[0].if2 = 5;
ERROR: inserting or modifying composite type fields is not supported
HINT: Use the column name to insert or update the composite type as a single value
UPDATE domain_indirection_test SET domain_array[0].if2 = (SELECT 5);
ERROR: inserting or modifying composite type fields is not supported
HINT: Use the column name to insert or update the composite type as a single value
-- below are supported as we don't do any field indirection
INSERT INTO field_indirection_test_2 (ct2_col, int_col, ct1_col)
VALUES ('(1, "text1", 2)', 3, '(4, 5)'), ('(6, "text2", 7)', 8, '(9, 10)');
INSERT INTO field_indirection_test_2 (ct2_col, int_col, ct1_col)
SELECT * FROM (VALUES ('(1, "text1", 2)'::ct2, 3, '(4, 5)'::ct1), ('(6, "text2", 7)'::ct2, 8, '(9, 10)'::ct1));
UPDATE field_indirection_test_2 SET (ct2_col, ct1_col) = ('(10, "text10", 20)', '(40, 50)') WHERE int_col=8;
UPDATE field_indirection_test_2 SET (ct2_col, ct1_col) = (SELECT '(10, "text10", 20)'::ct2, '(40, 50)'::ct1) WHERE int_col=8;
SELECT * FROM field_indirection_test_2 ORDER BY 1,2,3;
int_col | ct2_col | ct1_col
---------------------------------------------------------------------
3 | (1," text1",2) | (4,5)
3 | (1," text1",2) | (4,5)
8 | (10," text10",20) | (40,50)
(2 rows)
8 | (10," text10",20) | (40,50)
(4 rows)

-- test different ddl propagation modes
SET citus.create_object_propagation TO deferred;
Expand Down
11 changes: 11 additions & 0 deletions src/test/regress/sql/distributed_types.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,8 @@ SELECT create_distributed_table('field_indirection_test_1', 'int_col');
-- not supported (field indirection in single row insert)
INSERT INTO field_indirection_test_1 (int_col, ct1_col.int_1, ct1_col.int_2) VALUES (0, 1, 2);
INSERT INTO field_indirection_test_1 (int_col, ct1_col.int_1) VALUES (0, 1);
INSERT INTO field_indirection_test_1 (int_col, ct1_col.int_1, ct1_col.int_2) SELECT 0, 1, 2;
INSERT INTO field_indirection_test_1 (int_col, ct1_col.int_1) SELECT 0, 1;

CREATE TYPE ct2 as (int_2 int, text_1 text, int_1 int);
CREATE TABLE field_indirection_test_2 (int_col int, ct2_col ct2, ct1_col ct1);
Expand All @@ -253,9 +255,12 @@ SELECT create_distributed_table('field_indirection_test_2', 'int_col');
-- not supported (field indirection in multi row insert)
INSERT INTO field_indirection_test_2 (ct2_col.int_1, int_col, ct2_col.text_1, ct1_col.int_2)
VALUES (0, 1, 'text1', 2), (3, 4, 'text1', 5);
INSERT INTO field_indirection_test_2 (ct2_col.int_1, int_col, ct2_col.text_1, ct1_col.int_2)
SELECT * FROM (VALUES (0, 1, 'text1', 2), (3, 4, 'text1', 5));

-- not supported (field indirection in update)
UPDATE field_indirection_test_2 SET (ct2_col.text_1, ct1_col.int_2) = ('text2', 10) WHERE int_col=4;
UPDATE field_indirection_test_2 SET (ct2_col.text_1, ct1_col.int_2) = (SELECT 'text2', 10) WHERE int_col=4;

CREATE TYPE two_ints as (if1 int, if2 int);
CREATE DOMAIN domain AS two_ints CHECK ((VALUE).if1 > 0);
Expand All @@ -264,13 +269,19 @@ SELECT create_distributed_table('domain_indirection_test', 'f1');

-- not supported (field indirection to underlying composite type)
INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) VALUES (0, 1, 2);
INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) SELECT 0, 1, 2;
INSERT INTO domain_indirection_test (f1,f3.if1) VALUES (0, 1);
INSERT INTO domain_indirection_test (f1,f3.if1) SELECT 0, 1;
UPDATE domain_indirection_test SET domain_array[0].if2 = 5;
UPDATE domain_indirection_test SET domain_array[0].if2 = (SELECT 5);

-- below are supported as we don't do any field indirection
INSERT INTO field_indirection_test_2 (ct2_col, int_col, ct1_col)
VALUES ('(1, "text1", 2)', 3, '(4, 5)'), ('(6, "text2", 7)', 8, '(9, 10)');
INSERT INTO field_indirection_test_2 (ct2_col, int_col, ct1_col)
SELECT * FROM (VALUES ('(1, "text1", 2)'::ct2, 3, '(4, 5)'::ct1), ('(6, "text2", 7)'::ct2, 8, '(9, 10)'::ct1));
UPDATE field_indirection_test_2 SET (ct2_col, ct1_col) = ('(10, "text10", 20)', '(40, 50)') WHERE int_col=8;
UPDATE field_indirection_test_2 SET (ct2_col, ct1_col) = (SELECT '(10, "text10", 20)'::ct2, '(40, 50)'::ct1) WHERE int_col=8;

SELECT * FROM field_indirection_test_2 ORDER BY 1,2,3;

Expand Down

0 comments on commit 900833a

Please sign in to comment.