You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "Mryange (via GitHub)" <gi...@apache.org> on 2026/04/17 07:34:18 UTC
[PR] [refine](column) remove IDataType get_default [doris]
Mryange opened a new pull request, #62582:
URL: https://github.com/apache/doris/pull/62582
### What problem does this PR solve?
Problem Summary:
`IDataType::get_default()` duplicated the default-value behavior already owned by `IColumn::insert_default()`, but the two paths did not always produce the same result. This made default materialization inconsistent for complex and nested types, and forced several code paths to round-trip through `Field` even when the column layer already had the correct defaulting semantics.
This change removes the redundant type-level API and makes default generation consistently follow the column implementation.
### Changes
- Remove `IDataType::get_default()` from the BE data type interface and delete all corresponding overrides.
- Rewrite `create_column_const_with_default_value()` to create a column and materialize its default value through `insert_default()`.
- Replace storage-side callers that used `type->get_default()` with direct column-level `insert_default()` calls.
- Update unit tests to stop asserting the removed `Field`-based API and to validate defaults through column behavior instead.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4266353083
run buildall
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4266222646
/review
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4386411545
/review
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4387153336
/review
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4267763229
# BE Regression && UT Coverage Report
Increment line coverage `75.00% (6/8)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/62582_21ac3cfa81015c9fbddf1e6383a45ac60ded5439_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/62582_21ac3cfa81015c9fbddf1e6383a45ac60ded5439_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.76% (27508/37292) |
| Line Coverage | 57.48% (297924/518277) |
| Region Coverage | 54.67% (247908/453432) |
| Branch Coverage | 56.28% (107319/190673) |
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4266216803
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR).
Please clearly describe your PR:
1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4386257291
/review
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4387275657
/review
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4387378310
run buildall
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4277562801
run buildall
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4278166141
run nonConcurrent
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "zclllyybb (via GitHub)" <gi...@apache.org>.
zclllyybb commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4281228049
/review
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4387152611
> Automated review conclusion: request changes.
>
> I did not add new inline comments because the blocking issues I verified are already covered by existing inline review threads and should not be duplicated. The unresolved blockers are: `create_column_const_with_default_value()` now materializes column defaults instead of the removed type-level defaults, which changes Array/Map/JSONB default values in shared non-storage call sites; fixed partial update and flexible partial update also persist the new empty/NULL complex defaults for insert-after-delete rows instead of the previous synthetic defaults; and the new Variant default unit-test expectation was already noted as inconsistent with `ColumnVariant::insert_default()`/`ColumnVariant::get()`. The current PR also reports BE UT (macOS) failed.
>
> Critical checkpoint conclusions:
>
> * Goal and proof: the PR aims to remove `IDataType::get_default()` and use column default materialization consistently, but the implementation is not behavior-preserving while the PR states no behavior change, and test/check status does not prove correctness.
> * Scope/minimality: the API removal is broad but mechanically focused; however, replacing shared helper/storage semantics without compatibility or behavior acknowledgement is not safe.
> * Concurrency/lifecycle/config: no new concurrency, lifecycle, or configuration concerns were found in the changed code.
> * Compatibility/data correctness: there is user-visible/default-materialization behavior change for complex types and MoW partial-update reinsert paths; this remains the main blocker.
> * Parallel paths: fixed partial update, flexible partial update, and shared const-default helper paths were reviewed; the same semantic change exists across them and has already been commented.
> * Testing: a new BE unit test was added and a regression `.out` changed, but the PR checklist says no tests are needed, the current checks show BE UT failure, and the regression diff itself demonstrates changed behavior.
> * Observability/performance/memory: no additional observability, performance, or memory-tracking issues were identified for this refactor.
>
> User focus points: no additional user-provided review focus was supplied.
create_column_const_with_default_value 没有问题。
Array/Map/JSONB 的默认值变更是合理的。
请重新review。
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4387046562
/review
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #62582:
URL: https://github.com/apache/doris/pull/62582#discussion_r3194029688
##########
be/test/core/data_type/data_type_insert_default_test.cpp:
##########
@@ -0,0 +1,275 @@
+// 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.
+
+#include <gtest/gtest.h>
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "agent/be_exec_version_manager.h"
+#include "core/data_type/data_type_agg_state.h"
+#include "core/data_type/data_type_array.h"
+#include "core/data_type/data_type_factory.hpp"
+#include "core/data_type/data_type_fixed_length_object.h"
+#include "core/data_type/data_type_map.h"
+#include "core/data_type/data_type_nothing.h"
+#include "core/data_type/data_type_nullable.h"
+#include "core/data_type/data_type_struct.h"
+#include "core/data_type/data_type_varbinary.h"
+#include "core/data_type/data_type_variant.h"
+#include "core/field.h"
+#include "core/types.h"
+#include "core/value/timestamptz_value.h"
+#include "core/value/vdatetime_value.h"
+
+namespace doris {
+namespace {
+
+struct DefaultCase {
+ std::string name;
+ DataTypePtr data_type;
+ Field expected;
+};
+
+void assert_field_equals(const Field& actual, const Field& expected);
+
+void assert_string_field_equals(const Field& actual, const Field& expected) {
+ EXPECT_EQ(actual.get<TYPE_STRING>(), expected.get<TYPE_STRING>());
+}
+
+void assert_array_field_equals(const Field& actual, const Field& expected) {
+ const auto& actual_array = actual.get<TYPE_ARRAY>();
+ const auto& expected_array = expected.get<TYPE_ARRAY>();
+ ASSERT_EQ(actual_array.size(), expected_array.size());
+ for (size_t i = 0; i < actual_array.size(); ++i) {
+ assert_field_equals(actual_array[i], expected_array[i]);
+ }
+}
+
+void assert_map_field_equals(const Field& actual, const Field& expected) {
+ const auto& actual_fields = actual.get<TYPE_MAP>();
+ const auto& expected_fields = expected.get<TYPE_MAP>();
+ ASSERT_EQ(actual_fields.size(), expected_fields.size());
+ for (size_t i = 0; i < actual_fields.size(); ++i) {
+ assert_field_equals(actual_fields[i], expected_fields[i]);
+ }
+}
+
+void assert_struct_field_equals(const Field& actual, const Field& expected) {
+ const auto& actual_fields = actual.get<TYPE_STRUCT>();
+ const auto& expected_fields = expected.get<TYPE_STRUCT>();
+ ASSERT_EQ(actual_fields.size(), expected_fields.size());
+ for (size_t i = 0; i < actual_fields.size(); ++i) {
+ assert_field_equals(actual_fields[i], expected_fields[i]);
+ }
+}
+
+void assert_empty_jsonb_field(const Field& actual) {
+ EXPECT_EQ(actual.get<TYPE_JSONB>().get_size(), 0);
+}
+
+void assert_empty_complex_state(const Field& actual) {
+ switch (actual.get_type()) {
+ case TYPE_BITMAP:
+ EXPECT_EQ(actual.get<TYPE_BITMAP>().cardinality(), 0);
+ return;
+ case TYPE_HLL:
+ EXPECT_EQ(actual.get<TYPE_HLL>().estimate_cardinality(), 0);
+ return;
+ case TYPE_QUANTILE_STATE: {
+ QuantileState empty_state;
+ EXPECT_EQ(actual.get<TYPE_QUANTILE_STATE>().get_serialized_size(),
+ empty_state.get_serialized_size());
+ return;
+ }
+ default:
+ FAIL() << "unexpected complex type: " << actual.get_type_name();
+ }
+}
+
+void assert_field_equals(const Field& actual, const Field& expected) {
+ if (expected.get_type() == TYPE_JSONB) {
+ ASSERT_TRUE(actual.get_type() == TYPE_JSONB || is_string_type(actual.get_type()));
+ if (actual.get_type() == TYPE_JSONB) {
+ assert_empty_jsonb_field(actual);
+ } else {
+ EXPECT_TRUE(actual.get<TYPE_STRING>().empty());
+ }
+ return;
+ }
+
+ if (is_string_type(actual.get_type()) && is_string_type(expected.get_type())) {
+ assert_string_field_equals(actual, expected);
+ return;
+ }
+
+ ASSERT_EQ(actual.get_type(), expected.get_type());
+
+ if (actual.get_type() == TYPE_NULL) {
+ EXPECT_TRUE(actual.is_null());
+ return;
+ }
+
+ if (actual.get_type() == TYPE_ARRAY) {
+ assert_array_field_equals(actual, expected);
+ return;
+ }
+
+ if (actual.get_type() == TYPE_MAP) {
+ assert_map_field_equals(actual, expected);
+ return;
+ }
+
+ if (actual.get_type() == TYPE_STRUCT) {
+ assert_struct_field_equals(actual, expected);
+ return;
+ }
+
+ if (actual.get_type() == TYPE_VARIANT) {
+ EXPECT_TRUE(actual.get<TYPE_VARIANT>().empty());
+ return;
+ }
+
+ if (actual.get_type() == TYPE_BITMAP || actual.get_type() == TYPE_HLL ||
+ actual.get_type() == TYPE_QUANTILE_STATE) {
+ assert_empty_complex_state(actual);
+ return;
+ }
+
+ EXPECT_EQ(actual, expected);
+}
+
+void assert_insert_default(const DefaultCase& test_case) {
+ auto column = test_case.data_type->create_column();
+ ASSERT_EQ(column->size(), 0);
+
+ column->insert_default();
+ ASSERT_EQ(column->size(), 1);
+
+ Field actual;
+ column->get(0, actual);
+ assert_field_equals(actual, test_case.expected);
+}
+
+TEST(DataTypeInsertDefaultTest, AllTypeFamilies) {
+ auto make_simple_type = [](PrimitiveType type) {
+ return DataTypeFactory::instance().create_data_type(type, false);
+ };
+
+ std::vector<DefaultCase> cases;
+ cases.push_back(
+ {"bool", make_simple_type(TYPE_BOOLEAN), Field::create_field<TYPE_BOOLEAN>(UInt8(0))});
+ cases.push_back({"tinyint", make_simple_type(TYPE_TINYINT),
+ Field::create_field<TYPE_TINYINT>(Int8(0))});
+ cases.push_back({"smallint", make_simple_type(TYPE_SMALLINT),
+ Field::create_field<TYPE_SMALLINT>(Int16(0))});
+ cases.push_back({"int", make_simple_type(TYPE_INT), Field::create_field<TYPE_INT>(Int32(0))});
+ cases.push_back(
+ {"bigint", make_simple_type(TYPE_BIGINT), Field::create_field<TYPE_BIGINT>(Int64(0))});
+ cases.push_back({"largeint", make_simple_type(TYPE_LARGEINT),
+ Field::create_field<TYPE_LARGEINT>(Int128(0))});
+ cases.push_back(
+ {"float", make_simple_type(TYPE_FLOAT), Field::create_field<TYPE_FLOAT>(Float32(0))});
+ cases.push_back({"double", make_simple_type(TYPE_DOUBLE),
+ Field::create_field<TYPE_DOUBLE>(Float64(0))});
+ cases.push_back({"decimalv2",
+ DataTypeFactory::instance().create_data_type(TYPE_DECIMALV2, false, 27, 9),
+ Field::create_field<TYPE_DECIMALV2>(DecimalV2Value())});
+ cases.push_back({"decimal32",
+ DataTypeFactory::instance().create_data_type(TYPE_DECIMAL32, false, 9, 2),
+ Field::create_field<TYPE_DECIMAL32>(Decimal32(0))});
+ cases.push_back({"decimal64",
+ DataTypeFactory::instance().create_data_type(TYPE_DECIMAL64, false, 18, 9),
+ Field::create_field<TYPE_DECIMAL64>(Decimal64(0))});
+ cases.push_back({"decimal128",
+ DataTypeFactory::instance().create_data_type(TYPE_DECIMAL128I, false, 38, 18),
+ Field::create_field<TYPE_DECIMAL128I>(Decimal128V3(0))});
+ cases.push_back({"decimal256",
+ DataTypeFactory::instance().create_data_type(TYPE_DECIMAL256, false, 76, 18),
+ Field::create_field<TYPE_DECIMAL256>(Decimal256(0))});
+ cases.push_back({"date", make_simple_type(TYPE_DATE),
+ Field::create_field<TYPE_DATE>(VecDateTimeValue::DEFAULT_VALUE)});
+ cases.push_back({"datetime", make_simple_type(TYPE_DATETIME),
+ Field::create_field<TYPE_DATETIME>(VecDateTimeValue::DEFAULT_VALUE)});
+ cases.push_back(
+ {"datev2", make_simple_type(TYPE_DATEV2),
+ Field::create_field<TYPE_DATEV2>(DateV2Value<DateV2ValueType>::DEFAULT_VALUE)});
+ cases.push_back({"datetimev2",
+ DataTypeFactory::instance().create_data_type(TYPE_DATETIMEV2, false, 0, 6),
+ Field::create_field<TYPE_DATETIMEV2>(
+ DateV2Value<DateTimeV2ValueType>::DEFAULT_VALUE)});
+ cases.push_back({"timev2",
+ DataTypeFactory::instance().create_data_type(TYPE_TIMEV2, false, 0, 6),
+ Field::create_field<TYPE_TIMEV2>(Float64(0))});
+ cases.push_back({"timestamptz",
+ DataTypeFactory::instance().create_data_type(TYPE_TIMESTAMPTZ, false, 0, 6),
+ Field::create_field<TYPE_TIMESTAMPTZ>(TimestampTzValue::DEFAULT_VALUE)});
+ cases.push_back({"ipv4", make_simple_type(TYPE_IPV4), Field::create_field<TYPE_IPV4>(IPv4(0))});
+ cases.push_back({"ipv6", make_simple_type(TYPE_IPV6), Field::create_field<TYPE_IPV6>(IPv6(0))});
+ cases.push_back({"char", DataTypeFactory::instance().create_data_type(TYPE_CHAR, false, 8, 0),
+ Field::create_field<TYPE_STRING>(String())});
+ cases.push_back({"varchar",
+ DataTypeFactory::instance().create_data_type(TYPE_VARCHAR, false, 32, 0),
+ Field::create_field<TYPE_STRING>(String())});
+ cases.push_back(
+ {"string", make_simple_type(TYPE_STRING), Field::create_field<TYPE_STRING>(String())});
+ cases.push_back({"varbinary", std::make_shared<DataTypeVarbinary>(),
+ Field::create_field<TYPE_VARBINARY>(StringView())});
+ cases.push_back(
+ {"jsonb", make_simple_type(TYPE_JSONB), Field::create_field<TYPE_JSONB>(JsonbField())});
+ cases.push_back({"bitmap", make_simple_type(TYPE_BITMAP),
+ Field::create_field<TYPE_BITMAP>(BitmapValue::empty_bitmap())});
+ cases.push_back({"hll", make_simple_type(TYPE_HLL),
+ Field::create_field<TYPE_HLL>(HyperLogLog::empty())});
+ cases.push_back({"quantile_state", make_simple_type(TYPE_QUANTILE_STATE),
+ Field::create_field<TYPE_QUANTILE_STATE>(QuantileState())});
+ cases.push_back({"variant", std::make_shared<DataTypeVariant>(),
Review Comment:
This case does not match `ColumnVariant`'s actual default materialization. `ColumnVariant::insert_default()` inserts an empty variant row, and `ColumnVariant::get()` initializes the result as `Field::create_field<TYPE_VARIANT>(VariantMap())`, so `assert_field_equals()` will fail at the earlier `ASSERT_EQ(actual.get_type(), expected.get_type())` because the expected field here is `TYPE_NULL`. Please expect an empty `TYPE_VARIANT` value (or adjust the assertion logic) so the new unit test actually passes and validates the column default behavior.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on code in PR #62582:
URL: https://github.com/apache/doris/pull/62582#discussion_r3194855838
##########
be/src/storage/partial_update_info.cpp:
##########
@@ -724,7 +724,7 @@ static void fill_non_primary_key_cell_for_row_store(
// store the generated auto-increment value in fixed partial update
new_col->insert_from(cur_col, block_pos);
} else {
- new_col->insert(tablet_column.get_vec_type()->get_default());
+ new_col->insert_default();
Review Comment:
只有当这个列被删除的时候才会插入到默认值，可以看修改的回归，delete 列为1
##########
be/src/storage/tablet/base_tablet.cpp:
##########
@@ -1125,7 +1125,7 @@ static void fill_cell_for_flexible_partial_update(
// keep consistency between replicas
new_col->insert_from(cur_col, read_index_update[cast_set<uint32_t>(idx)]);
} else {
- new_col->insert(tablet_column.get_vec_type()->get_default());
+ new_col->insert_default();
Review Comment:
只有当这个列被删除的时候才会插入到默认值，可以看修改的回归，delete 列为1
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4389774934
run buildall
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #62582:
URL: https://github.com/apache/doris/pull/62582#discussion_r3194819571
##########
be/src/storage/partial_update_info.cpp:
##########
@@ -724,7 +724,7 @@ static void fill_non_primary_key_cell_for_row_store(
// store the generated auto-increment value in fixed partial update
new_col->insert_from(cur_col, block_pos);
} else {
- new_col->insert(tablet_column.get_vec_type()->get_default());
+ new_col->insert_default();
Review Comment:
This is the row-store flexible partial-update branch, which is distinct from the existing comment on the column-store branch above. When `_probe_key_for_mow()` classifies an insert-after-delete row as `use_default` and the skipped column is read through row-store, this value is written into the rebuilt row. Switching from `get_default()` to `insert_default()` therefore also changes committed non-null complex values on this path (`[null]`/`{null:null}`/JSON `null` become `[]`/`{}`/SQL `NULL`). Please preserve the previous materialization here as well, otherwise row-store partial updates still regress even if the column-store path is fixed.
##########
be/src/storage/tablet/base_tablet.cpp:
##########
@@ -1125,7 +1125,7 @@ static void fill_cell_for_flexible_partial_update(
// keep consistency between replicas
new_col->insert_from(cur_col, read_index_update[cast_set<uint32_t>(idx)]);
} else {
- new_col->insert(tablet_column.get_vec_type()->get_default());
+ new_col->insert_default();
Review Comment:
This flexible publish path is separate from the fixed partial-update branch already discussed earlier in this file. For skipped non-null complex columns on an insert-after-delete row, `generate_new_block_for_flexible_partial_update()` reaches this helper with `use_default` and writes the value into the output rowset. Using `insert_default()` changes the committed contents from the old synthetic defaults to empty array/map or SQL NULL, so the flexible publish path needs the same compatibility fix rather than this mechanical replacement.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62582:
URL: https://github.com/apache/doris/pull/62582#issuecomment-4278732605
# BE Regression && UT Coverage Report
Increment line coverage `75.00% (6/8)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/62582_dda1bc241365714653f9839263f7cabc0dc241b5_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/62582_dda1bc241365714653f9839263f7cabc0dc241b5_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.78% (27590/37393) |
| Line Coverage | 57.52% (298369/518715) |
| Region Coverage | 54.73% (248383/453803) |
| Branch Coverage | 56.36% (107553/190817) |
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei merged PR #62582:
URL: https://github.com/apache/doris/pull/62582
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refine](column) remove IDataType get_default [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #62582:
URL: https://github.com/apache/doris/pull/62582#discussion_r3111504778
##########
be/src/core/data_type/data_type.cpp:
##########
@@ -60,7 +60,9 @@ ColumnPtr IDataType::create_column_const(size_t size, const Field& field) const
}
ColumnPtr IDataType::create_column_const_with_default_value(size_t size) const {
- return create_column_const(size, get_default());
+ auto column = create_column();
+ column->insert_default();
Review Comment:
This helper is no longer equivalent to the removed `get_default()` for complex types. For Array, Map, and JSONB, `insert_default()` yields `[]`, `{}`, and SQL `NULL` instead of the previous synthetic defaults (`[null]`, `{null:null}`, JSON `null`), and the regression diff in `test_partial_update_complex_type.out` already shows that behavior change. Because this helper is shared by many non-storage call sites, the PR is not behavior-preserving anymore.
##########
be/src/storage/tablet/base_tablet.cpp:
##########
@@ -1057,7 +1057,7 @@ Status BaseTablet::generate_new_block_for_partial_update(
mutable_column.get())
->insert_default();
} else {
- mutable_column->insert(rs_column.get_vec_type()->get_default());
+ mutable_column->insert_default();
Review Comment:
This branch is not just filling dead payload. When a fixed partial update hits a previously delete-marked row, `use_default` becomes true and the new row is rebuilt from these values. Replacing `get_default()` with `insert_default()` therefore changes committed results for non-null complex columns from the previous synthetic defaults to `[]`, `{}`, and SQL `NULL`.
##########
be/src/storage/partial_update_info.cpp:
##########
@@ -618,7 +618,7 @@ static void fill_non_primary_key_cell_for_column_store(
// store the generated auto-increment value in fixed partial update
new_col->insert_from(cur_col, block_pos);
} else {
- new_col->insert(tablet_column.get_vec_type()->get_default());
+ new_col->insert_default();
Review Comment:
The same issue exists in flexible partial update. `_probe_key_for_mow()` sends insert-after-delete rows down the `use_default` path without calling `handle_new_key()`, so these values become the visible contents of the re-inserted row. With this change, skipped non-null complex columns now persist `[]`, `{}`, and SQL `NULL` instead of the previous synthetic defaults.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org