mrhhsg commented on code in PR #63528:
URL: https://github.com/apache/doris/pull/63528#discussion_r4258349225
##########
be/src/storage/segment/segment.cpp:
##########
@@ -894,29 +894,22 @@ Status Segment::get_default_value_field(const
TabletColumn& tablet_column, Field
return Status::OK();
}
- if (tablet_column.type() == FieldType::OLAP_FIELD_TYPE_ARRAY) {
- if (default_value != "[]") {
- return Status::NotSupported("Array default {} is unsupported",
default_value);
- }
+ // [] and {} need no nested SerDe, so an ARRAY/MAP TabletColumn built
without sub-columns can
+ // still materialize an empty default.
+ if (tablet_column.type() == FieldType::OLAP_FIELD_TYPE_ARRAY &&
default_value == "[]") {
*field = Field::create_field<TYPE_ARRAY>(Array {});
return Status::OK();
}
- if (tablet_column.type() == FieldType::OLAP_FIELD_TYPE_STRUCT) {
- return Status::NotSupported("STRUCT default type is unsupported");
- }
- if (tablet_column.type() == FieldType::OLAP_FIELD_TYPE_MAP) {
- if (default_value != "{}") {
- return Status::NotSupported("Map default {} is unsupported",
default_value);
- }
+ if (tablet_column.type() == FieldType::OLAP_FIELD_TYPE_MAP &&
default_value == "{}") {
*field = Field::create_field<TYPE_MAP>(Map
{Field::create_field<TYPE_ARRAY>(Array {}),
Field::create_field<TYPE_ARRAY>(Array {})});
return Status::OK();
}
- const auto serde = DataTypeFactory::instance()
- .create_data_type(tablet_column.type(),
tablet_column.precision(),
- tablet_column.frac(),
tablet_column.length())
- ->get_serde();
+ // Any other non-null default, including a non-empty ARRAY/MAP or any
STRUCT literal, needs the
+ // SerDe of the declared (possibly nested) type to parse recursively; a
bare FieldType cannot
+ // describe nested item types.
+ const auto serde =
remove_nullable(tablet_column.get_vec_type())->get_serde();
Review Comment:
Confirmed and fixed. Reproduced locally: after `ADD COLUMN s
STRUCT<a:INT,b:STRING> DEFAULT '{7, "x"}'` on a table with an old row, `SELECT
struct_element(s, 'b')` failed with `Cannot insert value of different size into
struct. field struct size 2, columns size 1`, and so did STRUCT items of an
ARRAY (`array_map(x -> struct_element(x, 'b'), arr)`) and STRUCT values of a
MAP (`struct_element(element_at(m, 'k'), 'b')`).
`Segment::new_column_iterator` now takes the materialized read type
(`ReadSchema::data_type`, i.e. the FE-pruned slot type the destination column
is created from). When access paths exist for the column, a
`ConstantColumnIterator` is projected onto that type with the new
`ConstantColumnIterator::project_to_read_type`: STRUCT fields are selected by
the read type's field names (declared order,
`DataTypeStruct::try_get_position_by_name`), ARRAY items and MAP values
recurse, MAP keys, scalars, NULL and the `{}` default are kept. File iterators
keep dropping their pruned sub-iterators as before.
Coverage:
`ConstantColumnIteratorTest.ComplexDefaultIsProjectedToPrunedReadType` (STRUCT,
ARRAY<STRUCT>, MAP<STRING, STRUCT>, projection to the declared type, `{}`), and
the new `alter_default_pruned` query in `test_complex_default_value`
(`struct_element` on the light-added STRUCT and on an empty `{}` STRUCT,
`array_map` over ARRAY<STRUCT>, `struct_element(element_at(map, ...))`,
`size()`, `IS NULL`) for the old row and a row inserted after the ALTER.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ComplexTypeDefaultValue.java:
##########
@@ -0,0 +1,238 @@
+// 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.DorisParser.ArrayLiteralContext;
+import org.apache.doris.nereids.DorisParser.ConstantContext;
+import org.apache.doris.nereids.DorisParser.MapLiteralContext;
+import org.apache.doris.nereids.exceptions.AnalysisException;
+import org.apache.doris.nereids.exceptions.ParseException;
+import org.apache.doris.nereids.parser.LogicalPlanBuilder;
+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 com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import org.antlr.v4.runtime.ParserRuleContext;
+
+import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+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 whose nested values
are kept exactly as written
+ * (see {@link LiteralBuilder}), and every nested value is then 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, new
LiteralBuilder());
+ } 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";
Review Comment:
Confirmed and fixed. Reproduced on a MoW table: a partial update of a new
key stored `["NULL"]`, `{"x":"NULL"}` and `{"a":null, "b":"NULL"}` for the
`[NULL]`, `{"x": NULL}` and `{NULL, NULL}` defaults.
The canonical text now renders a nested NULL as lower case `null`, the only
spelling every BE consumer of the text accepts:
`DataTypeNullableSerDe::deserialize_one_cell_from_json` (the
`default_from_string` path used by `BaseTablet::generate_default_value_block`
and `partial_update_info.cpp`) only recognizes `null` at nesting level >= 2,
`ComplexTypeDeserializeUtil::is_null_string` (default value iterator, schema
change, string-to-complex cast of INSERT) is case-insensitive, and the replayed
DDL parses `null` as the SQL NULL literal.
Coverage: `ColumnDefinitionTest` (`[null, null, 5]`, `{"a":1, "b":null}`,
`{null, "2"}`, `[[1], [], null]`), the SHOW CREATE TABLE assertions in the
regression, and the new MoW table `test_complex_default_value_partial_update`
with `enable_unique_key_partial_update=true` / `enable_insert_strict=false` and
an `INSERT INTO t(k)`; `array_contains(arr, NULL)` and `struct_element(s, 'f2')
IS NULL` distinguish a real NULL from the string `"NULL"` in the `.out`.
##########
be/src/core/data_type_serde/data_type_array_serde.cpp:
##########
@@ -593,6 +593,42 @@ Status DataTypeArraySerDe::_from_string(StringRef& str,
IColumn& column,
return Status::OK();
}
+Status DataTypeArraySerDe::from_fe_string(const std::string& str, Field&
field) const {
+ StringRef slice(str);
+ slice = slice.trim_whitespace();
+ if (slice.empty()) {
+ return Status::InvalidArgument("slice is empty!");
+ }
+ if (slice.front() != '[') {
+ return Status::InvalidArgument("Array does not start with '['
character, found '{}'",
+ slice.to_string());
+ }
+ if (slice.back() != ']') {
+ return Status::InvalidArgument("Array does not end with ']' character,
found '{}'",
+ slice.to_string());
+ }
+
+ Array array;
+ if (slice.size > 2) {
+ slice = slice.substring(1, slice.size - 2);
+ slice = slice.trim_whitespace();
+ if (!slice.empty()) {
+ FormatOptions options;
+ auto split_result = ComplexTypeDeserializeUtil::split_by_delimiter(
+ slice, [&](char c) { return c == options.collection_delim;
});
+ array.reserve(split_result.size());
Review Comment:
Not changing this in the PR. The parsed default uses the representation
every constant column already has: before this PR
`ConstantColumnReader::new_iterator` already kept one `Field` copy per
scan-owned `ConstantColumnIterator` for scalar and string defaults (a VARCHAR
default of up to 65533 bytes is retained the same way), and
`SchemaChangeJob::_init_column_mapping` has parsed
`ColumnMapping::default_value` through `from_fe_string` into an untracked
`Field` for years. `Array`/`Map`/`Struct` are defined as `std::vector<Field>`
in `core/field.h`, so an allocator-aware payload would change the `Field` type
itself, which is out of scope for complex defaults.
The retained size is bounded by the DDL default text (every element costs at
least one byte of text, i.e. roughly a `sizeof(Field)`-fold amplification per
iterator) and the same text is already kept untracked in every `TabletSchema`
and in the FE catalog. A 100000-element literal default is not a realistic
schema, and charging the thread tracker for these buffers would need a matching
release tied to one iterator kind's lifetime, more mechanism than the existing
default paths carry. If a tracked `Field` payload is wanted, it should be a
separate change covering all `Field` consumers.
--
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]
