mrhhsg commented on code in PR #63528:
URL: https://github.com/apache/doris/pull/63528#discussion_r4227164743
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinition.java:
##########
@@ -630,16 +629,68 @@ private void validateInternal(boolean isOlap, Set<String>
keysSet, Set<String> c
* Validate non-null defaults for complex types before connector-specific
validation.
*/
public void validateComplexTypeDefaultValue() throws AnalysisException {
- if (!defaultValue.isPresent() || defaultValue.get() ==
DefaultValue.NULL_DEFAULT_VALUE) {
- return;
- }
- if (type.isMapType()) {
- throw new AnalysisException("Map type column default value just
support null");
+ if (type.isArrayType()) {
+ validateArrayDefaultValue();
+ } else if (type.isMapType()) {
+ validateMapDefaultValue();
Review Comment:
Addressed in f44cc1901ac. The DDL parser keeps the text between the quotes
of a default value verbatim (`LogicalPlanBuilder.toStringValue`; unlike
comments it does not decode escapes), so `DEFAULT "{\"a\":10}"` would be
replayed as `{\"a\":10}`. `Column.toSql()` now wraps a default that contains
double quotes in single quotes (`DEFAULT '{"a":10}'`), which replays exactly;
plain defaults keep the existing `DEFAULT "..."` rendering. Canonical complex
defaults never contain single quotes (see the escaped-string thread). Covered
by `ColumnTest.testToSqlQuotesDefaultValueForReplay` and the SHOW CREATE TABLE
/ CREATE TABLE LIKE round trip in `test_complex_default_value` (`like_default`).
##########
be/src/core/data_type_serde/complex_type_deserialize_util.h:
##########
@@ -80,13 +81,37 @@ struct ComplexTypeDeserializeUtil {
static bool is_null_string(const StringRef& str) {
if (str.size == 4) {
- // null
- return str.data[0] == 'n' && str.data[1] == 'u' && str.data[2] ==
'l' &&
- str.data[3] == 'l';
+ // SQL NULL literal is case-insensitive.
+ return (str.data[0] == 'n' || str.data[0] == 'N') &&
+ (str.data[1] == 'u' || str.data[1] == 'U') &&
+ (str.data[2] == 'l' || str.data[2] == 'L') &&
+ (str.data[3] == 'l' || str.data[3] == 'L');
}
return false;
}
+ static Status process_field(const DataTypeSerDeSPtr& serde, StringRef str,
Field& field) {
+ str = str.trim_whitespace();
+ auto nullable_serde =
std::dynamic_pointer_cast<DataTypeNullableSerDe>(serde);
+ if (is_null_string(str)) {
+ if (nullable_serde == nullptr) {
+ return Status::InvalidArgument(
+ "NULL default is not allowed for non-nullable complex
field");
+ }
+ field = Field::create_field<TYPE_NULL>(Null {});
+ return Status::OK();
+ }
+ auto str_without_quote = str.trim_quote();
Review Comment:
Addressed in f44cc1901ac. FE now parses the default as a SQL literal and
stores a canonical text (`ComplexTypeDefaultValue.canonicalize`), so `"a""b"`
is decoded to `a"b` on the FE side. Because neither BE parser of that text
decodes escapes (the complex `from_fe_string` used here and the
string-to-complex cast used by INSERT) and the DDL parser keeps default text
verbatim when SHOW CREATE / LIKE output is replayed, a nested string value
containing a quote or backslash cannot be stored unambiguously; it is rejected
at DDL time with `must not contain quote or backslash` instead of being
materialized as `a""b`. Covered by `ColumnDefinitionTest` and the
`createTableRejects` cases in `test_complex_default_value`; accepted values
with delimiters and brackets inside are read back through the default iterator
in `alter_default` and pinned on the BE side by
`canonicalComplexDefaultFromFeString`.
##########
be/src/core/data_type_serde/data_type_map_serde.cpp:
##########
@@ -756,6 +756,68 @@ Status DataTypeMapSerDe::_from_string(StringRef& str,
IColumn& column,
return Status::OK();
}
+Status DataTypeMapSerDe::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() != '{') {
+ std::stringstream ss;
+ ss << slice.front() << '\'';
+ return Status::InvalidArgument("Map does not start with '{' character,
found '" + ss.str());
+ }
+ if (slice.back() != '}') {
+ std::stringstream ss;
+ ss << slice.back() << '\'';
+ return Status::InvalidArgument("Map does not end with '}' character,
found '" + ss.str());
+ }
+
+ Array keys;
+ Array values;
+ 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.map_key_delim || c ==
options.collection_delim;
+ });
+ if (split_result.size() % 2 != 0) {
+ return Status::InvalidArgument("Map does not have even number
of key-value pairs");
+ }
+ keys.reserve(split_result.size() / 2);
+ values.reserve(split_result.size() / 2);
+ for (int i = 0; i < split_result.size(); i += 2) {
+ if (split_result[i].delimiter != options.map_key_delim) {
+ return Status::InvalidArgument(
+ "Map key-value pair does not have map key
delimiter");
+ }
+ if (i != 0 && split_result[i - 1].delimiter !=
options.collection_delim) {
+ return Status::InvalidArgument(
+ "Map key-value pair does not have collection
delimiter");
+ }
+
+ Field key_field;
+ Field value_field;
+ RETURN_IF_ERROR(ComplexTypeDeserializeUtil::process_field(
+ key_serde, split_result[i].element, key_field));
+ RETURN_IF_ERROR(ComplexTypeDeserializeUtil::process_field(
+ value_serde, split_result[i + 1].element,
value_field));
+ keys.push_back(std::move(key_field));
Review Comment:
Addressed in f44cc1901ac. The canonical text is rendered from the parsed
`MapLiteral`, which keeps the last value of a repeated key, so `{"a":1,"a":2}`
is stored as `{"a":2}` and BE never receives repeated pairs. Covered by
`ColumnDefinitionTest` and by `map_repeated_key` in
`test_complex_default_value`, which reads the default through INSERT, the light
schema change default iterator (`alter_default`) and direct schema change
(`alter_direct_default`).
##########
be/src/core/data_type_serde/complex_type_deserialize_util.h:
##########
@@ -80,13 +81,37 @@ struct ComplexTypeDeserializeUtil {
static bool is_null_string(const StringRef& str) {
if (str.size == 4) {
- // null
- return str.data[0] == 'n' && str.data[1] == 'u' && str.data[2] ==
'l' &&
- str.data[3] == 'l';
+ // SQL NULL literal is case-insensitive.
+ return (str.data[0] == 'n' || str.data[0] == 'N') &&
+ (str.data[1] == 'u' || str.data[1] == 'U') &&
+ (str.data[2] == 'l' || str.data[2] == 'L') &&
+ (str.data[3] == 'l' || str.data[3] == 'L');
}
return false;
}
+ static Status process_field(const DataTypeSerDeSPtr& serde, StringRef str,
Field& field) {
+ str = str.trim_whitespace();
+ auto nullable_serde =
std::dynamic_pointer_cast<DataTypeNullableSerDe>(serde);
+ if (is_null_string(str)) {
+ if (nullable_serde == nullptr) {
+ return Status::InvalidArgument(
+ "NULL default is not allowed for non-nullable complex
field");
+ }
+ field = Field::create_field<TYPE_NULL>(Null {});
+ return Status::OK();
+ }
+ auto str_without_quote = str.trim_quote();
+ auto nested_serde = serde;
+ if (nullable_serde != nullptr) {
+ // Complex default values need strict nested conversion.
DataTypeNullableSerDe
+ // converts nested parse failures to NULL, which would silently
accept
+ // type-mismatched default literals.
+ nested_serde = nullable_serde->get_nested_serde();
+ }
+ return nested_serde->from_fe_string(str_without_quote.to_string(),
field);
Review Comment:
Addressed in f44cc1901ac. FE now casts every nested literal to the declared
nested type with strict checking and stores the rendered value instead of the
SQL token spelling: `[DATEV2 "2024-01-01"]` and `["2024-01-01"]` are both
stored as `["2024-01-01"]`, `[1e3, "7"]` for `ARRAY<INT>` as `[1000, 7]`,
booleans as `1`/`0`, decimals at the column scale, and a type mismatch such as
`["bad"]` is rejected at CREATE/ALTER time. Nested types whose text format is
not pinned down (TIMEV2, TIMESTAMP_NS, TIMESTAMP_TZ, UUID) are rejected
explicitly rather than forwarded. The swallow in the date serde's
`from_olap_string` is shared with the zonemap path and is left unchanged; it is
no longer reachable from a default because the stored text is always the
canonical `yyyy-MM-dd` form. Covered by `ColumnDefinitionTest`, BE UT
`canonicalComplexDefaultFromFeString` (what BE must accept from the canonical
text), and `arr_typed_date` / `arr_exponent` / `arr_bool` / `arr_decimal` in
`test_complex_defaul
t_value`, read through old rows in `alter_default` and `alter_direct_default`.
--
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]
