github-actions[bot] commented on code in PR #66538: URL: https://github.com/apache/doris/pull/66538#discussion_r3876194808
########## fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RewriteDefaultExpression.java: ########## @@ -0,0 +1,113 @@ +// 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.rules.expression.rules; + +import org.apache.doris.catalog.Column; +import org.apache.doris.catalog.TableIf; +import org.apache.doris.datasource.iceberg.IcebergExternalTable; +import org.apache.doris.datasource.iceberg.IcebergWriteSchemaContext; +import org.apache.doris.nereids.CascadesContext; +import org.apache.doris.nereids.analyzer.UnboundFunction; +import org.apache.doris.nereids.exceptions.AnalysisException; +import org.apache.doris.nereids.parser.NereidsParser; +import org.apache.doris.nereids.rules.analysis.ExpressionAnalyzer; +import org.apache.doris.nereids.rules.expression.ExpressionMatchingContext; +import org.apache.doris.nereids.rules.expression.ExpressionPatternMatcher; +import org.apache.doris.nereids.rules.expression.ExpressionPatternRuleFactory; +import org.apache.doris.nereids.rules.expression.ExpressionRuleType; +import org.apache.doris.nereids.trees.expressions.Default; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.SlotReference; +import org.apache.doris.nereids.trees.expressions.literal.NullLiteral; +import org.apache.doris.nereids.trees.plans.logical.LogicalPlan; +import org.apache.doris.nereids.types.DataType; +import org.apache.doris.nereids.util.TypeCoercionUtils; + +import com.google.common.collect.ImmutableList; + +import java.util.List; +import java.util.Optional; + +/** Rewrite DEFAULT(column) to the column's resolved default value. */ +public class RewriteDefaultExpression implements ExpressionPatternRuleFactory { + + public static final RewriteDefaultExpression INSTANCE = new RewriteDefaultExpression(); + + @Override + public List<ExpressionPatternMatcher<? extends Expression>> buildRules() { + return ImmutableList.of( + matchesType(Default.class) + .thenApply(RewriteDefaultExpression::rewrite) + .toRule(ExpressionRuleType.REWRITE_DEFAULT_EXPRESSION) + ); + } + + private static Expression rewrite(ExpressionMatchingContext<Default> context) { + Default defaultExpr = context.expr; + Expression child = defaultExpr.child(); + + if (!(child instanceof SlotReference)) { + throw new AnalysisException("DEFAULT requires a column reference, but got: " + child.toSql()); + } + + SlotReference slotRef = (SlotReference) child; + Optional<Column> columnOpt = slotRef.getOriginalColumn(); + if (!columnOpt.isPresent()) { + throw new AnalysisException("Cannot find column information for DEFAULT(" + + slotRef.getName() + ")"); + } + + Column column = columnOpt.get(); + DataType targetType = DataType.fromCatalogType(column.getType()); + if (column.isGeneratedColumn()) { + throw new AnalysisException("DEFAULT cannot be used on generated column '" + + column.getName() + "'"); + } + + Optional<IcebergWriteSchemaContext> icebergContext = context.cascadesContext + .getStatementContext().getIcebergWriteSchemaContext(); + Optional<TableIf> originalTable = slotRef.getOriginalTable(); Review Comment: [P1] Bind INSERT-SELECT DEFAULT against the write target At this point the child is already bound in the SELECT input scope. For `INSERT INTO iceberg_t(id, v) SELECT id, DEFAULT(v) FROM staging_t`, `v` either fails binding when absent or binds `staging_t.v`; in the latter case this falls through to the staging column's Doris default/NULL instead of the pinned Iceberg write default. The positive test only self-scans `iceberg_t`, which makes source and target identity coincide. Pre-resolve DEFAULT nodes for non-inline INSERT SELECT against the write-schema context, as VALUES/UPDATE/MERGE do, and cover staging-table and no-FROM SELECTs. ########## be/src/format_v2/table/iceberg_reader.cpp: ########## @@ -336,10 +999,92 @@ std::string IcebergTableReader::debug_string() const { return out.str(); } +Status IcebergTableReader::_validate_required_mapping_column( + const format::ColumnMapping& mapping, const ColumnPtr& column, + const NullMap* nullable_parent_null_map) { + DORIS_CHECK(column.get() != nullptr); + DORIS_CHECK(mapping.table_type != nullptr); + const auto full_column = column->convert_to_full_column_if_const(); + const IColumn* nested_column = full_column.get(); + const NullMap* own_null_map = nullptr; + if (const auto* nullable = check_and_get_column<ColumnNullable>(*nested_column)) { + own_null_map = &nullable->get_null_map_data(); + nested_column = &nullable->get_nested_column(); + if (mapping.reject_null_value && nullable->has_null()) { + DORIS_CHECK(nullable_parent_null_map == nullptr || + nullable_parent_null_map->size() == own_null_map->size()); + for (size_t row = 0; row < own_null_map->size(); ++row) { + if ((*own_null_map)[row] != 0 && (nullable_parent_null_map == nullptr || + (*nullable_parent_null_map)[row] == 0)) { + return Status::InvalidArgument("Required Iceberg field '{}' contains NULL", + mapping.table_column_name); + } + } + } + } + if (mapping.child_mappings.empty()) { + return Status::OK(); + } + + NullMap combined_parent_null_map; + const NullMap* descendant_parent_null_map = nullable_parent_null_map; + if (own_null_map != nullptr) { + descendant_parent_null_map = own_null_map; + if (nullable_parent_null_map != nullptr) { + DORIS_CHECK(nullable_parent_null_map->size() == own_null_map->size()); + combined_parent_null_map.resize(own_null_map->size()); + for (size_t row = 0; row < own_null_map->size(); ++row) { + combined_parent_null_map[row] = + (*own_null_map)[row] || (*nullable_parent_null_map)[row]; + } + descendant_parent_null_map = &combined_parent_null_map; + } + } + + const auto table_type = remove_nullable(mapping.table_type); + switch (table_type->get_primitive_type()) { + case TYPE_STRUCT: { + const auto& struct_column = assert_cast<const ColumnStruct&>(*nested_column); + DORIS_CHECK(mapping.child_mappings.size() == struct_column.tuple_size()); + for (size_t child = 0; child < mapping.child_mappings.size(); ++child) { + RETURN_IF_ERROR(_validate_required_mapping_column(mapping.child_mappings[child], Review Comment: [P1] Match struct validation by table child identity `_materialize_struct_mapping_column()` reorders child mappings into `DataTypeStruct` order before building the `ColumnStruct`, but this loop zips that reordered tuple with raw `mapping.child_mappings`. Partial access paths are stored lexically, so a schema-order `STRUCT<z optional, a required>` can produce mappings `[a,z]` and columns `[z,a]`; requiredness for `a` is then checked on `z`, allowing NULL `a` through or rejecting a valid NULL `z`. Reorder mappings here with the same table-order helper and add an opposite-requiredness partial-projection regression, including a struct nested under ARRAY/MAP. -- 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]
