Skip to content

Commit 32aad6b

Browse files
BenoitRanqueBenoitRanque
authored
Support setting array columns to empty array instead of setting those to null (#762)
<!-- The PR description should answer 2 (maybe 3) important questions: --> ### What <!-- What is this PR trying to accomplish (and why, if it's not obvious)? --> Attempting to set an array column to `[]` would instead set the column to `NULL`. This happened because we process the incoming values using `array_agg`, which returns null if aggregating over no rows. <!-- Consider: do we need to add a changelog entry? --> ### How <!-- How is it trying to accomplish it (what are the implementation steps)? --> We wrap the array aggregation in `COALESCE`, with a default value of an empty array cast to the right type. The new behavior when updating an array column is now - setting to an array with elements works as before - setting to null works as before - setting to an empty array works as expected, wheras before it would be set to null. The PR also adds snapshot tests for all 3 cases. The snapshotted SQL has been tested and verified to behave as intended. Some other snapshots are also updated due to the change in generated SQL. It seems we use the functions in `values.rs` every time we have some kind of json input. This is not what I set out to fix, and there could be issues if we're relying on the null behavior here, but I don't think we are.
1 parent b8cdbf9 commit 32aad6b

17 files changed

+849
-76
lines changed

‎changelog.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
### Fixed
1010

11+
- Fixed array column updates to properly handle empty arrays instead of incorrectly setting them to null.
12+
1113
## [v2.1.1] - 2025-03-12
1214

1315
### Changed

‎crates/query-engine/translation/src/translation/query/values.rs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,11 +155,26 @@ pub(crate) fn translate_projected(
155155
let converted_element_exp =
156156
translate_projected(env, state, type_name, element_expression)?;
157157

158+
// Create an empty array of the correct type to use as the default value
159+
let empty_array = sql::ast::Expression::Cast {
160+
expression: Box::new(sql::ast::Expression::Value(sql::ast::Value::Array(vec![]))),
161+
r#type: sql::ast::ScalarType::ArrayType(type_to_ast_scalar_type_name(
162+
env, type_name,
163+
)?),
164+
};
165+
158166
let mut result_select = sql::helpers::simple_select(vec![(
159167
element_column,
168+
// Use COALESCE to return an empty array instead of NULL when array_agg returns NULL
160169
sql::ast::Expression::FunctionCall {
161-
function: sql::ast::Function::Unknown("array_agg".to_string()),
162-
args: vec![converted_element_exp],
170+
function: sql::ast::Function::Coalesce,
171+
args: vec![
172+
sql::ast::Expression::FunctionCall {
173+
function: sql::ast::Function::Unknown("array_agg".to_string()),
174+
args: vec![converted_element_exp],
175+
},
176+
empty_array,
177+
],
163178
},
164179
)]);
165180

‎crates/query-engine/translation/tests/goldenfiles/mutations/v2_update_array_column_empty/configuration.json

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
{
2+
"version": "5",
3+
"$schema": "../../../../../../../static/configuration.schema.json",
4+
"connectionSettings": {
5+
"connectionUri": {
6+
"variable": "CONNECTION_URI"
7+
},
8+
"poolSettings": {
9+
"maxConnections": 50,
10+
"poolTimeout": 30,
11+
"idleTimeout": 180,
12+
"checkConnectionAfterIdle": 60,
13+
"connectionLifetime": 600
14+
},
15+
"isolationLevel": "ReadCommitted"
16+
},
17+
"metadata": {
18+
"tables": {
19+
"items": {
20+
"schemaName": "public",
21+
"tableName": "items",
22+
"columns": {
23+
"id": {
24+
"name": "id",
25+
"type": {
26+
"scalarType": "int4"
27+
},
28+
"nullable": "nonNullable",
29+
"description": null
30+
},
31+
"name": {
32+
"name": "name",
33+
"type": {
34+
"scalarType": "text"
35+
},
36+
"nullable": "nullable",
37+
"description": null
38+
},
39+
"tags": {
40+
"name": "tags",
41+
"type": {
42+
"arrayType": {
43+
"scalarType": "text"
44+
}
45+
},
46+
"nullable": "nullable",
47+
"description": null
48+
}
49+
},
50+
"uniquenessConstraints": {
51+
"items_pkey": ["id"]
52+
},
53+
"foreignRelations": {},
54+
"description": null
55+
}
56+
},
57+
"types": {
58+
"scalar": {
59+
"int4": {
60+
"typeName": "int4",
61+
"schemaName": "pg_catalog",
62+
"description": null,
63+
"aggregateFunctions": {},
64+
"comparisonOperators": {},
65+
"typeRepresentation": "int32"
66+
},
67+
"text": {
68+
"typeName": "text",
69+
"schemaName": "pg_catalog",
70+
"description": null,
71+
"aggregateFunctions": {},
72+
"comparisonOperators": {},
73+
"typeRepresentation": "string"
74+
}
75+
},
76+
"composite": {}
77+
},
78+
"nativeOperations": {
79+
"queries": {},
80+
"mutations": {}
81+
}
82+
},
83+
"introspectionOptions": {
84+
"excludedSchemas": [
85+
"information_schema",
86+
"pg_catalog",
87+
"tiger",
88+
"crdb_internal",
89+
"columnar",
90+
"columnar_internal"
91+
],
92+
"unqualifiedSchemasForTables": ["public"],
93+
"unqualifiedSchemasForTypesAndProcedures": [
94+
"public",
95+
"pg_catalog",
96+
"tiger"
97+
],
98+
"typeRepresentations": {
99+
"bit": "string",
100+
"bool": "boolean",
101+
"bpchar": "string",
102+
"char": "string",
103+
"date": "date",
104+
"float4": "float32",
105+
"float8": "float64",
106+
"int2": "int16",
107+
"int4": "int32",
108+
"int8": "int64AsString",
109+
"numeric": "bigDecimalAsString",
110+
"text": "string",
111+
"time": "time",
112+
"timestamp": "timestamp",
113+
"timestamptz": "timestamptz",
114+
"timetz": "timetz",
115+
"uuid": "uUID",
116+
"varchar": "string"
117+
}
118+
},
119+
"mutationsVersion": "v2",
120+
"mutationsPrefix": null
121+
}

‎crates/query-engine/translation/tests/goldenfiles/mutations/v2_update_array_column_empty/request.json

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
{
2+
"$schema": "../../../../../../../static/mutation.schema.json",
3+
"operations": [
4+
{
5+
"type": "procedure",
6+
"name": "v2_update_items_by_id",
7+
"arguments": {
8+
"key_id": 1,
9+
"update_columns": {
10+
"tags": {
11+
"_set": []
12+
}
13+
}
14+
},
15+
"fields": {
16+
"type": "object",
17+
"fields": {
18+
"affected_rows": {
19+
"type": "column",
20+
"column": "affected_rows"
21+
},
22+
"returning": {
23+
"type": "column",
24+
"column": "returning",
25+
"fields": {
26+
"type": "array",
27+
"fields": {
28+
"type": "object",
29+
"fields": {
30+
"id": {
31+
"type": "column",
32+
"column": "id"
33+
},
34+
"tags": {
35+
"type": "column",
36+
"column": "tags"
37+
}
38+
}
39+
}
40+
}
41+
}
42+
}
43+
}
44+
}
45+
],
46+
"collection_relationships": {}
47+
}

‎crates/query-engine/translation/tests/goldenfiles/mutations/v2_update_array_column_null/configuration.json

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
{
2+
"version": "5",
3+
"$schema": "../../../../../../../static/configuration.schema.json",
4+
"connectionSettings": {
5+
"connectionUri": {
6+
"variable": "CONNECTION_URI"
7+
},
8+
"poolSettings": {
9+
"maxConnections": 50,
10+
"poolTimeout": 30,
11+
"idleTimeout": 180,
12+
"checkConnectionAfterIdle": 60,
13+
"connectionLifetime": 600
14+
},
15+
"isolationLevel": "ReadCommitted"
16+
},
17+
"metadata": {
18+
"tables": {
19+
"items": {
20+
"schemaName": "public",
21+
"tableName": "items",
22+
"columns": {
23+
"id": {
24+
"name": "id",
25+
"type": {
26+
"scalarType": "int4"
27+
},
28+
"nullable": "nonNullable",
29+
"description": null
30+
},
31+
"name": {
32+
"name": "name",
33+
"type": {
34+
"scalarType": "text"
35+
},
36+
"nullable": "nullable",
37+
"description": null
38+
},
39+
"tags": {
40+
"name": "tags",
41+
"type": {
42+
"arrayType": {
43+
"scalarType": "text"
44+
}
45+
},
46+
"nullable": "nullable",
47+
"description": null
48+
}
49+
},
50+
"uniquenessConstraints": {
51+
"items_pkey": ["id"]
52+
},
53+
"foreignRelations": {},
54+
"description": null
55+
}
56+
},
57+
"types": {
58+
"scalar": {
59+
"int4": {
60+
"typeName": "int4",
61+
"schemaName": "pg_catalog",
62+
"description": null,
63+
"aggregateFunctions": {},
64+
"comparisonOperators": {},
65+
"typeRepresentation": "int32"
66+
},
67+
"text": {
68+
"typeName": "text",
69+
"schemaName": "pg_catalog",
70+
"description": null,
71+
"aggregateFunctions": {},
72+
"comparisonOperators": {},
73+
"typeRepresentation": "string"
74+
}
75+
},
76+
"composite": {}
77+
},
78+
"nativeOperations": {
79+
"queries": {},
80+
"mutations": {}
81+
}
82+
},
83+
"introspectionOptions": {
84+
"excludedSchemas": [
85+
"information_schema",
86+
"pg_catalog",
87+
"tiger",
88+
"crdb_internal",
89+
"columnar",
90+
"columnar_internal"
91+
],
92+
"unqualifiedSchemasForTables": ["public"],
93+
"unqualifiedSchemasForTypesAndProcedures": [
94+
"public",
95+
"pg_catalog",
96+
"tiger"
97+
],
98+
"typeRepresentations": {
99+
"bit": "string",
100+
"bool": "boolean",
101+
"bpchar": "string",
102+
"char": "string",
103+
"date": "date",
104+
"float4": "float32",
105+
"float8": "float64",
106+
"int2": "int16",
107+
"int4": "int32",
108+
"int8": "int64AsString",
109+
"numeric": "bigDecimalAsString",
110+
"text": "string",
111+
"time": "time",
112+
"timestamp": "timestamp",
113+
"timestamptz": "timestamptz",
114+
"timetz": "timetz",
115+
"uuid": "uUID",
116+
"varchar": "string"
117+
}
118+
},
119+
"mutationsVersion": "v2",
120+
"mutationsPrefix": null
121+
}

‎crates/query-engine/translation/tests/goldenfiles/mutations/v2_update_array_column_null/request.json

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
{
2+
"$schema": "../../../../../../../static/mutation.schema.json",
3+
"operations": [
4+
{
5+
"type": "procedure",
6+
"name": "v2_update_items_by_id",
7+
"arguments": {
8+
"key_id": 1,
9+
"update_columns": {
10+
"tags": {
11+
"_set": null
12+
}
13+
}
14+
},
15+
"fields": {
16+
"type": "object",
17+
"fields": {
18+
"affected_rows": {
19+
"type": "column",
20+
"column": "affected_rows"
21+
},
22+
"returning": {
23+
"type": "column",
24+
"column": "returning",
25+
"fields": {
26+
"type": "array",
27+
"fields": {
28+
"type": "object",
29+
"fields": {
30+
"id": {
31+
"type": "column",
32+
"column": "id"
33+
},
34+
"tags": {
35+
"type": "column",
36+
"column": "tags"
37+
}
38+
}
39+
}
40+
}
41+
}
42+
}
43+
}
44+
}
45+
],
46+
"collection_relationships": {}
47+
}

0 commit comments

Comments
 (0)