github-actions[bot] commented on code in PR #63528:
URL: https://github.com/apache/doris/pull/63528#discussion_r4227387142
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/load/NereidsDataDescription.java:
##########
@@ -599,10 +602,23 @@ private static void validateDefaultValue(List<String>
args, Column column) throw
}
if (args.get(0) != null) {
- ColumnDef.validateDefaultValue(column.getOriginType(),
args.get(0), column.getDefaultValueExprDef());
+ validateColumnValue(column, args.get(0));
}
}
+ private static void validateColumnValue(Column column, String value)
throws AnalysisException {
+ Type type = column.getOriginType();
+ if (type.isComplexType()) {
+ try {
+
ComplexTypeDefaultValue.canonicalize(DataType.fromCatalogType(type), value);
Review Comment:
[P1] Use canonicalized text for explicit complex load mappings. For
`ARRAY<INT>`, `default_value('[1e3]')` or `replace_value(null, '[1e3]')` passes
this validation because `canonicalize` produces `[1000]`, but the result is
discarded. `NereidsLoadScanProvider` keeps the original expression, so BE's
string-to-array cast sends `1e3` to the INT serde; non-strict load inserts a
NULL element (and strict load errors) instead of writing 1000. Replace the
actual mapping argument with the canonical text, and cover both explicit
mapping forms.
##########
be/src/storage/segment/segment.cpp:
##########
@@ -927,10 +927,11 @@ Status Segment::new_default_iterator(const TabletColumn&
tablet_column,
"column_type={}",
tablet_column.unique_id(), tablet_column.name(),
tablet_column.type());
}
+ auto serde = remove_nullable(tablet_column.get_vec_type())->get_serde();
Review Comment:
[P2] Avoid constructing a SerDe for NULL or absent defaults.
`new_default_iterator` now builds a vectorized type and SerDe for every missing
column on each old segment. When a newly added nullable column has no default,
or its default is `NULL`, `DefaultValueColumnIterator::init` only stores a NULL
Field and never uses that SerDe. This adds recursive allocations for nested
columns during normal scan setup, repeated across segments and scans. Create
the SerDe only for non-NULL defaults that need parsing.
##########
regression-test/suites/datatype_p0/complex_types/test_complex_default_value.groovy:
##########
@@ -0,0 +1,245 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_complex_default_value") {
+ sql "DROP TABLE IF EXISTS test_complex_default_value_literal"
+ sql "DROP TABLE IF EXISTS test_complex_default_value_like"
+ sql "DROP TABLE IF EXISTS test_complex_default_value_null"
+ sql "DROP TABLE IF EXISTS test_complex_default_value_alter_base"
+ sql "DROP TABLE IF EXISTS test_complex_default_value_alter_direct"
+ sql "DROP TABLE IF EXISTS test_complex_default_value_bad_alter"
+ sql "DROP TABLE IF EXISTS test_complex_default_value_replace_value"
+
+ def createTableRejects = { String columnDef, String message ->
+ test {
+ sql """
+ CREATE TABLE test_complex_default_value_rejected (
+ k INT,
+ v ${columnDef}
+ )
+ DUPLICATE KEY(k)
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES('replication_num'='1')
+ """
+ exception message
+ }
+ }
+
+ // the default must be a literal of the column's own shape
+ createTableRejects.call("ARRAY<INT> DEFAULT '{}'", "only supports array
literals or DEFAULT NULL")
+ createTableRejects.call("ARRAY<INT> DEFAULT '[1 + 1]'", "only supports
array literals or DEFAULT NULL")
+ createTableRejects.call("MAP<STRING, INT> DEFAULT '[]'", "only supports
map literals or DEFAULT NULL")
+ createTableRejects.call("STRUCT<f1:INT> DEFAULT '[]'", "only supports
struct literals or DEFAULT NULL")
+ createTableRejects.call("""STRUCT<f1:INT, f2:STRING> DEFAULT '{"f1": 1,
"f2": "a"}'""", "only supports struct literals or DEFAULT NULL")
+ createTableRejects.call("JSON DEFAULT '{}'", "only supports DEFAULT NULL")
+ createTableRejects.call("VARIANT DEFAULT '{}'", "only supports DEFAULT
NULL")
+
+ // every nested value is cast to the declared nested type at DDL time
+ createTableRejects.call("""ARRAY<INT> DEFAULT '["bad"]'""", "Invalid
default value")
+ createTableRejects.call("""MAP<INT, INT> DEFAULT '{"bad": 1}'""", "Invalid
default value")
+ createTableRejects.call("""MAP<STRING, INT> DEFAULT '{"bad": "value"}'""",
"Invalid default value")
+ createTableRejects.call("""STRUCT<f1:INT> DEFAULT '{"bad"}'""", "Invalid
default value")
+ createTableRejects.call("""STRUCT<f1:INT, f2:STRING> DEFAULT '{1}'""",
"struct literal has 1 fields but the column has 2")
+ createTableRejects.call("""ARRAY<ARRAY<INT>> DEFAULT '[[1], ["bad"]]'""",
"Invalid default value")
+
+ // nested string values are stored as plain double quoted text, so quotes
and backslashes are rejected
+ createTableRejects.call("""ARRAY<STRING> DEFAULT '["a""b"]'""", "must not
contain quote or backslash")
+ createTableRejects.call("""MAP<STRING, INT> DEFAULT '{"a\\\\\\\\b":
1}'""", "must not contain quote or backslash")
+ createTableRejects.call("""ARRAY<STRING> DEFAULT '["it''s"]'""", "must not
contain quote or backslash")
+
+ // ADD COLUMN validates the default the same way, for both light and
direct schema change
+ sql """
+ CREATE TABLE test_complex_default_value_bad_alter (
+ k INT
+ )
+ DUPLICATE KEY(k)
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES('replication_num'='1', 'light_schema_change'='false')
+ """
+ sql "INSERT INTO test_complex_default_value_bad_alter VALUES (1)"
+ test {
+ sql """ALTER TABLE test_complex_default_value_bad_alter ADD COLUMN v
ARRAY<INT> DEFAULT '["bad"]'"""
+ exception "Invalid default value"
+ }
+ test {
+ sql """ALTER TABLE test_complex_default_value_bad_alter ADD COLUMN v
MAP<INT, INT> DEFAULT '{"bad": 1}'"""
+ exception "Invalid default value"
+ }
+ test {
+ sql """ALTER TABLE test_complex_default_value_bad_alter ADD COLUMN v
STRUCT<f1:INT> DEFAULT '{"bad"}'"""
+ exception "Invalid default value"
+ }
+
+ sql """
+ CREATE TABLE test_complex_default_value_literal (
+ k INT,
+ arr_empty ARRAY<INT> DEFAULT '[]',
+ arr_literal ARRAY<INT> DEFAULT '[1, 2]',
+ map_empty MAP<STRING, INT> DEFAULT '{}',
+ map_literal MAP<STRING, INT> DEFAULT '{"a": 10, "b": 20}',
+ struct_empty STRUCT<f1:INT, f2:STRING> DEFAULT '{}',
+ struct_literal STRUCT<f1:INT, f2:STRING> DEFAULT '{7, "x"}',
+ arr_nested_null ARRAY<INT> DEFAULT '[NULL, nUlL, 5]',
+ map_nested_null MAP<STRING, INT> DEFAULT '{"upper": NULL, "mixed":
nUlL, "value": 6}',
+ struct_nested_null STRUCT<f1:INT, f2:STRING, f3:INT> DEFAULT
'{NULL, NuLl, 7}',
+ arr_typed_date ARRAY<DATEV2> DEFAULT '[DATEV2 "2024-01-01",
"2024-02-02"]',
+ arr_exponent ARRAY<INT> DEFAULT '[1e3, "7"]',
+ arr_bool ARRAY<BOOLEAN> DEFAULT '[true, false]',
+ arr_decimal ARRAY<DECIMAL(10, 2)> DEFAULT '[1.234]',
+ arr_string ARRAY<STRING> DEFAULT '["x,y", "[z]", "{k:v}", "null",
""]',
+ map_repeated_key MAP<STRING, INT> DEFAULT '{"a": 1, "a": 2}',
+ arr_nested ARRAY<ARRAY<INT>> DEFAULT '[[1], [], NULL]'
+ )
+ DUPLICATE KEY(k)
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES('replication_num'='1')
+ """
+
+ sql "INSERT INTO test_complex_default_value_literal(k) VALUES (1)"
+ order_qt_literal_default """
+ SELECT * FROM test_complex_default_value_literal ORDER BY k
+ """
+
+ // SHOW CREATE TABLE renders the canonical default in a form that CREATE
TABLE LIKE can replay
+ def showCreate = sql "SHOW CREATE TABLE test_complex_default_value_literal"
+ def createSql = showCreate[0][1]
+ logger.info("show create table: ${createSql}")
+ assertTrue(createSql.contains('`arr_literal` array<int> NULL DEFAULT "[1,
2]"'))
+ assertTrue(createSql.contains('`map_literal` map<text,int> NULL DEFAULT
\'{"a":10, "b":20}\''))
+ assertTrue(createSql.contains('`arr_typed_date` array<date> NULL DEFAULT
\'["2024-01-01", "2024-02-02"]\''))
+ assertTrue(createSql.contains('`arr_exponent` array<int> NULL DEFAULT
"[1000, 7]"'))
+ assertTrue(createSql.contains('`arr_bool` array<boolean> NULL DEFAULT "[1,
0]"'))
+ assertTrue(createSql.contains('`map_repeated_key` map<text,int> NULL
DEFAULT \'{"a":2}\''))
+ assertTrue(createSql.contains('`arr_string` array<text> NULL DEFAULT
\'["x,y", "[z]", "{k:v}", "null", ""]\''))
+ sql "CREATE TABLE test_complex_default_value_like LIKE
test_complex_default_value_literal"
+ sql "INSERT INTO test_complex_default_value_like(k) VALUES (1)"
+ order_qt_like_default """
+ SELECT * FROM test_complex_default_value_like ORDER BY k
+ """
+
+ sql """
+ CREATE TABLE test_complex_default_value_null (
+ k INT,
+ arr_col ARRAY<INT> DEFAULT NULL,
+ map_col MAP<STRING, INT> DEFAULT NULL,
+ struct_col STRUCT<f:INT> DEFAULT NULL,
+ json_col JSON DEFAULT NULL,
+ variant_col VARIANT DEFAULT NULL
+ )
+ DUPLICATE KEY(k)
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES('replication_num'='1')
+ """
+
+ sql "INSERT INTO test_complex_default_value_null(k) VALUES (1)"
+ order_qt_null_default """
+ SELECT k, arr_col, map_col, struct_col, json_col, variant_col
+ FROM test_complex_default_value_null
+ ORDER BY k
+ """
+
+ // rows written before the columns were added read the defaults through
the BE default value iterator
+ sql """
+ CREATE TABLE test_complex_default_value_alter_base (
+ k INT
+ )
+ DUPLICATE KEY(k)
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES(
+ 'replication_num'='1',
+ 'light_schema_change'='true'
+ )
+ """
+
+ sql "INSERT INTO test_complex_default_value_alter_base VALUES (1)"
+ sql """
+ ALTER TABLE test_complex_default_value_alter_base
+ ADD COLUMN arr_added ARRAY<INT> NOT NULL DEFAULT '[3, 4]',
+ ADD COLUMN map_added MAP<STRING, INT> NOT NULL DEFAULT '{"z": 9}',
+ ADD COLUMN struct_added STRUCT<f1:INT, f2:STRING> NOT NULL DEFAULT
'{8, "y"}',
+ ADD COLUMN arr_typed_date ARRAY<DATEV2> DEFAULT '[DATEV2 "2024-01-01",
"2024-02-02"]',
+ ADD COLUMN arr_exponent ARRAY<INT> DEFAULT '[1e3, "7"]',
+ ADD COLUMN arr_bool ARRAY<BOOLEAN> DEFAULT '[true, false]',
+ ADD COLUMN arr_decimal ARRAY<DECIMAL(10, 2)> DEFAULT '[1.234]',
+ ADD COLUMN arr_string ARRAY<STRING> DEFAULT '["x,y", "[z]", "{k:v}",
"null", ""]',
+ ADD COLUMN map_repeated_key MAP<STRING, INT> DEFAULT '{"a": 1, "a":
2}',
+ ADD COLUMN arr_nested ARRAY<ARRAY<INT>> DEFAULT '[[1], [], NULL]'
+ """
+ waitForSchemaChangeDone {
+ sql """SHOW ALTER TABLE COLUMN WHERE
IndexName='test_complex_default_value_alter_base' ORDER BY createtime DESC
LIMIT 1"""
+ time 600
+ }
+
+ order_qt_alter_default """
+ SELECT * FROM test_complex_default_value_alter_base ORDER BY k
+ """
+
+ sql "INSERT INTO test_complex_default_value_alter_base(k) VALUES (2)"
+ order_qt_alter_default_after_insert """
+ SELECT * FROM test_complex_default_value_alter_base ORDER BY k
+ """
+
+ // direct schema change materializes the defaults into the rewritten rows
+ sql """
+ CREATE TABLE test_complex_default_value_alter_direct (
+ k INT
+ )
+ DUPLICATE KEY(k)
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES(
+ 'replication_num'='1',
+ 'light_schema_change'='false'
+ )
+ """
+
+ sql "INSERT INTO test_complex_default_value_alter_direct VALUES (1)"
+ sql """
+ ALTER TABLE test_complex_default_value_alter_direct
+ ADD COLUMN arr_added ARRAY<INT> NOT NULL DEFAULT '[3, 4]',
Review Comment:
[P2] Make this case trigger a direct row rewrite. Adding these value columns
after the sole short key leaves `sc_directly` and `sc_sorting` false in
`SchemaChangeJob::parse_request`, even with `light_schema_change=false`, so BE
selects `LinkedSchemaChange` and merely links the old rowset. The SELECT then
exercises the missing-column default iterator already covered above, without
testing the new `BlockChanger`/complex `Field` insertion path that this case is
labeled to cover. Add a direct-strategy trigger and assert the rewritten rows.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ComplexTypeDefaultValue.java:
##########
@@ -0,0 +1,175 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.plans.commands.info;
+
+import org.apache.doris.nereids.exceptions.AnalysisException;
+import org.apache.doris.nereids.parser.NereidsParser;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.literal.ArrayLiteral;
+import org.apache.doris.nereids.trees.expressions.literal.BooleanLiteral;
+import org.apache.doris.nereids.trees.expressions.literal.Literal;
+import org.apache.doris.nereids.trees.expressions.literal.MapLiteral;
+import org.apache.doris.nereids.trees.expressions.literal.NullLiteral;
+import org.apache.doris.nereids.trees.expressions.literal.StructLiteral;
+import org.apache.doris.nereids.types.ArrayType;
+import org.apache.doris.nereids.types.DataType;
+import org.apache.doris.nereids.types.MapType;
+import org.apache.doris.nereids.types.StructField;
+import org.apache.doris.nereids.types.StructType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.List;
+import java.util.Map;
+import java.util.StringJoiner;
+
+/**
+ * Validates a non-null ARRAY/MAP/STRUCT column default and rewrites it into a
canonical literal text.
+ *
+ * <p>The user supplied text is parsed as a SQL literal and every nested value
is cast to the declared
+ * nested type, so a type mismatch is rejected at DDL time instead of when old
rows are read. The
+ * canonical text only contains plain nested values (unquoted numbers, double
quoted strings, nested
+ * brackets and NULL). BE parses the stored text in two places that must
agree: the complex SerDe
+ * {@code from_fe_string} used by the default value iterator and schema
change, and the
+ * string-to-complex cast used when INSERT fills an unmentioned column.
Neither of them decodes
+ * escape sequences, and the DDL parser keeps the text of a default value
verbatim when SHOW CREATE
+ * TABLE output is replayed, so string values containing quotes or backslashes
are rejected instead
+ * of stored.
+ */
+public class ComplexTypeDefaultValue {
+ private ComplexTypeDefaultValue() {
+ }
+
+ /**
+ * Validate the default literal of a complex column and return its
canonical text.
+ */
+ public static String canonicalize(DataType type, String defaultValue)
throws AnalysisException {
+ Preconditions.checkArgument(type.isArrayType() || type.isMapType() ||
type.isStructType(),
+ "%s is not a complex type", type);
+ Expression expression;
+ try {
+ expression = new NereidsParser().parseExpression(defaultValue);
+ } catch (Exception e) {
+ throw literalShapeException(type);
+ }
+ if (!hasLiteralShape(expression, type)) {
+ throw literalShapeException(type);
+ }
+ try {
+ return render((Literal) expression, type);
+ } catch (AnalysisException e) {
+ throw new AnalysisException(String.format("Invalid default value
'%s' for %s column: %s",
+ defaultValue, type.toSql(), e.getMessage()), e);
+ }
+ }
+
+ private static boolean hasLiteralShape(Expression expression, DataType
type) {
+ if (type.isArrayType()) {
+ return expression instanceof ArrayLiteral;
+ }
+ if (type.isMapType()) {
+ return expression instanceof MapLiteral;
+ }
+ // `{}` parses as an empty map literal and means every struct field
takes its own default.
+ return expression instanceof StructLiteral
+ || (expression instanceof MapLiteral && ((MapLiteral)
expression).getValue().isEmpty());
+ }
+
+ private static AnalysisException literalShapeException(DataType type) {
+ String literalKind = type.isArrayType() ? "array" : type.isMapType() ?
"map" : "struct";
+ return new AnalysisException(String.format("%s type column default
value only supports %s literals"
+ + " or DEFAULT NULL", capitalize(literalKind), literalKind));
+ }
+
+ private static String capitalize(String value) {
+ return Character.toUpperCase(value.charAt(0)) + value.substring(1);
+ }
+
+ private static String render(Literal literal, DataType type) throws
AnalysisException {
+ if (literal instanceof NullLiteral) {
+ return "NULL";
+ }
+ if (type.isArrayType()) {
+ if (!(literal instanceof ArrayLiteral)) {
+ throw new AnalysisException(literal.toSql() + " is not an
array literal");
+ }
+ DataType itemType = ((ArrayType) type).getItemType();
+ StringJoiner joiner = new StringJoiner(", ", "[", "]");
+ for (Literal item : ((ArrayLiteral) literal).getValue()) {
+ joiner.add(render(item, itemType));
+ }
+ return joiner.toString();
+ }
+ if (type.isMapType()) {
+ if (!(literal instanceof MapLiteral)) {
+ throw new AnalysisException(literal.toSql() + " is not a map
literal");
+ }
+ MapType mapType = (MapType) type;
+ // The parser keeps the last value of a repeated key, so a
repeated key is stored once.
+ StringJoiner joiner = new StringJoiner(", ", "{", "}");
+ for (Map.Entry<Literal, Literal> entry : ((MapLiteral)
literal).getValue().entrySet()) {
+ joiner.add(render(entry.getKey(), mapType.getKeyType()) + ":"
Review Comment:
[P2] Deduplicate map keys after casting them to the declared key type.
`MAP<INT,INT> DEFAULT '{"01":1,"1":2}'` reaches this loop with two distinct
string keys, but both render as key `1`, so the stored default becomes `{1:1,
1:2}`. BE's default iterator materializes both entries for old rows, while a
replay of that text (including CREATE TABLE LIKE) and normal writes keep only
the last entry. Reject or normalize collisions after the target cast and cover
an old-row read and DDL replay.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
