xiaokang commented on code in PR #40573: URL: https://github.com/apache/doris/pull/40573#discussion_r1798626995
########## be/src/olap/tablet_reader.cpp: ########## @@ -276,6 +276,8 @@ TabletColumn TabletReader::materialize_column(const TabletColumn& orig) { cast_type.type); } column_with_cast_type.set_type(filed_type); + column_with_cast_type.set_precision_frac(cast_type.precision, cast_type.scale); + column_with_cast_type.set_is_decimal(cast_type.precision > 0); Review Comment: It's not rigorous to use `cast_type.precision > 0` to judge is_decimal. You can use cast_type.is_decimal_v2_type() and cast_type.is_decimal_v3_type() or add a new is_decimal_type for cast_type. ########## be/src/vec/data_types/data_type_time_v2.h: ########## @@ -159,12 +160,22 @@ class DataTypeDateTimeV2 final : public DataTypeNumberBase<UInt64> { node.date_literal.value); } } + MutableColumnPtr create_column() const override; UInt32 get_scale() const override { return _scale; } void to_pb_column_meta(PColumnMeta* col_meta) const override; + Field get_type_field(const IColumn& column, int row) const override { + const auto& column_data = + assert_cast<const ColumnUInt64&, TypeCheckOnRelease::DISABLE>(column); + Field field; + column_data.get(row, field); + field.set_type_info(get_type_id(), 0, static_cast<int>(get_scale())); Review Comment: Is precision == 0 right? ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinition.java: ########## @@ -640,6 +640,19 @@ private void validateScalarType(ScalarType scalarType) { } break; } + case VARIANT: + ArrayList<org.apache.doris.catalog.StructField> predefinedFields = + ((org.apache.doris.catalog.VariantType) scalarType).getPredefinedFields(); + Set<String> fieldNames = new HashSet<>(); + for (org.apache.doris.catalog.StructField field : predefinedFields) { + Type fieldType = field.getType(); + validateNestedType(scalarType, fieldType); + if (!fieldNames.add(field.getName())) { + throw new AnalysisException("Duplicate field name " + field.getName() + + " in struct " + scalarType.toSql()); Review Comment: in variant ########## be/src/vec/data_types/data_type_time.h: ########## @@ -79,6 +79,13 @@ class DataTypeTimeV2 final : public DataTypeNumberBase<Float64> { const char* get_family_name() const override { return "timev2"; } UInt32 get_scale() const override { return _scale; } + Field get_type_field(const IColumn& column, int row) const override { + Field field; + column.get(row, field); + field.set_type_info(get_type_id(), 0, static_cast<int>(get_scale())); Review Comment: Is precision == 0 right? ########## be/src/olap/rowset/segment_v2/vertical_segment_writer.cpp: ########## @@ -992,6 +1001,12 @@ Status VerticalSegmentWriter::_append_block_with_variant_subcolumns(RowsInBlock& auto full_path = full_path_builder.append(parent_column->name_lower_case(), false) .append(entry->path.get_parts(), false) .build(); + if (typed_columns.contains(entry->path.get_path())) { Review Comment: What's the purpose of typed_columns? -- 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: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org