This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 310a8843168 [fix](point query) Fix BE crash when a point query reads a
column added by light schema change (#68997)
310a8843168 is described below
commit 310a8843168df97705d870c09462417c8d589501
Author: Chenyang Sun <[email protected]>
AuthorDate: Wed Oct 14 14:38:16 2026 +0800
[fix](point query) Fix BE crash when a point query reads a column added by
light schema change (#68997)
### What problem does this PR solve?
Since #68198, a short-circuit point query that reads a MAP or STRUCT
column added by light schema change, before any new load, aborts the BE.
`tablet_fetch_data` now also catches exceptions, so a point query that
throws returns an error instead.
```sql
CREATE TABLE t (k INT NOT NULL, stored INT NOT NULL, wide INT NOT NULL)
UNIQUE KEY(k) DISTRIBUTED BY HASH(k) BUCKETS 1
PROPERTIES ("replication_num" = "1", "enable_unique_key_merge_on_write" =
"true",
"light_schema_change" = "true", "store_row_column" = "true",
"row_store_columns" = "k,stored");
INSERT INTO t VALUES (1, 10, 100);
ALTER TABLE t ADD COLUMN obj MAP<STRING, ARRAY<INT>> NULL;
SET enable_short_circuit_query = true;
SET enable_short_circuit_query_access_column_store = true;
SELECT obj, stored, wide FROM t WHERE k = 1;
-- before: BE aborts, client gets RpcException ... UNAVAILABLE: io exception
-- after: NULL 10 100
```
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### 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 -->
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
be/src/service/internal_service.cpp | 4 +-
be/src/service/point_query_executor.cpp | 9 +-
be/src/storage/schema.cpp | 95 +++++++++++++++
be/src/storage/schema.h | 5 +
be/test/storage/read_schema_test.cpp | 129 +++++++++++++++++++++
.../test_point_query_light_schema_change.out | 9 ++
.../test_point_query_light_schema_change.groovy | 19 +++
7 files changed, 261 insertions(+), 9 deletions(-)
diff --git a/be/src/service/internal_service.cpp
b/be/src/service/internal_service.cpp
index 8858f5eceb8..3ff14bdf8ce 100644
--- a/be/src/service/internal_service.cpp
+++ b/be/src/service/internal_service.cpp
@@ -1006,11 +1006,11 @@ void
PInternalService::fetch_arrow_flight_schema(google::protobuf::RpcController
Status PInternalService::_tablet_fetch_data(const PTabletKeyLookupRequest*
request,
PTabletKeyLookupResponse*
response) {
PointQueryExecutor executor;
- RETURN_IF_ERROR(executor.init(request, response));
+ RETURN_IF_ERROR_OR_CATCH_EXCEPTION(executor.init(request, response));
if (response->has_need_resend_query_context() &&
response->need_resend_query_context()) {
return Status::OK();
}
- RETURN_IF_ERROR(executor.lookup_up());
+ RETURN_IF_ERROR_OR_CATCH_EXCEPTION(executor.lookup_up());
executor.print_profile();
return Status::OK();
}
diff --git a/be/src/service/point_query_executor.cpp
b/be/src/service/point_query_executor.cpp
index 674ed7b71f8..560fb3ca1e6 100644
--- a/be/src/service/point_query_executor.cpp
+++ b/be/src/service/point_query_executor.cpp
@@ -37,7 +37,6 @@
#include "common/status.h"
#include "core/data_type/data_type_factory.hpp"
#include "core/data_type_serde/data_type_serde.h"
-#include "exec/common/variant_util.h"
#include "exec/sink/writer/vmysql_result_writer.h"
#include "exprs/vexpr.h"
#include "exprs/vexpr_context.h"
@@ -54,6 +53,7 @@
#include "storage/row_cursor.h"
#include "storage/rowset/beta_rowset.h"
#include "storage/rowset/rowset_fwd.h"
+#include "storage/schema.h"
#include "storage/segment/column_reader.h"
#include "storage/tablet/tablet_schema.h"
#include "storage/utils.h"
@@ -616,12 +616,7 @@ Status PointQueryExecutor::_lookup_row_data() {
// default -- the same answer it gives once a load does
bring the column in.
TabletColumn column_awaiting_a_load;
if (index < 0) {
- column_awaiting_a_load =
variant_util::get_column_by_type(
- slot->type(), slot->col_name(),
- variant_util::ExtraInfo {.unique_id =
slot->col_unique_id()});
- if (!slot->col_default_value().empty()) {
-
column_awaiting_a_load.set_default_value(slot->col_default_value());
- }
+ column_awaiting_a_load =
ReadSchema::tablet_column_from_slot(*slot);
}
const TabletColumn& read_column =
index >= 0 ? tablet_schema->column(index) :
column_awaiting_a_load;
diff --git a/be/src/storage/schema.cpp b/be/src/storage/schema.cpp
index 6cfee7a2523..5c4e0901ff6 100644
--- a/be/src/storage/schema.cpp
+++ b/be/src/storage/schema.cpp
@@ -22,10 +22,20 @@
#include "common/config.h"
#include "common/logging.h"
+#include "core/assert_cast.h"
#include "core/block/block.h"
#include "core/column/column_dictionary.h"
#include "core/column/column_nothing.h"
#include "core/column/column_nullable.h"
+#include "core/data_type/data_type_array.h"
+#include "core/data_type/data_type_decimal.h"
+#include "core/data_type/data_type_map.h"
+#include "core/data_type/data_type_nullable.h"
+#include "core/data_type/data_type_string.h"
+#include "core/data_type/data_type_struct.h"
+#include "core/data_type/data_type_variant_v2.h"
+#include "core/data_type/primitive_type.h"
+#include "runtime/descriptors.h"
#include "storage/binlog.h"
namespace doris {
@@ -47,6 +57,80 @@ bool row_binlog_value_columns_have_same_type(const
TabletColumn& lhs, const Tabl
return true;
}
+// Sets the type of `column` from `data_type`, sub columns included. Sub
columns get the names FE
+// gives them, and the length is computed the same way as for a column FE
sends.
+void set_type_from_data_type(const DataTypePtr& data_type, TabletColumn*
column) {
+ const DataTypePtr type = remove_nullable(data_type);
+ const PrimitiveType primitive_type = type->get_primitive_type();
+ column->set_type(type->get_storage_field_type());
+ column->set_is_nullable(data_type->is_nullable());
+ uint32_t string_length = 0;
+ switch (primitive_type) {
+ case TYPE_CHAR:
+ case TYPE_VARCHAR:
+ case TYPE_STRING:
+ string_length = cast_set<uint32_t>(assert_cast<const
DataTypeString&>(*type).len());
+ break;
+ case TYPE_DECIMALV2: {
+ // DECIMALV2 always computes as (27, 9). Keep the precision and scale
the column declares.
+ const auto& decimal_type = assert_cast<const
DataTypeDecimalV2&>(*type);
+ column->set_precision(decimal_type.get_original_precision());
+ column->set_frac(decimal_type.get_original_scale());
+ break;
+ }
+ case TYPE_DECIMAL32:
+ case TYPE_DECIMAL64:
+ case TYPE_DECIMAL128I:
+ case TYPE_DECIMAL256:
+ column->set_precision(type->get_precision());
+ column->set_frac(type->get_scale());
+ break;
+ case TYPE_DATETIMEV2:
+ case TYPE_TIMESTAMPTZ:
+ column->set_frac(type->get_scale());
+ break;
+ case TYPE_VARIANT: {
+ const auto& variant_type = assert_cast<const
DataTypeVariantV2&>(*type);
+
column->set_variant_max_subcolumns_count(variant_type.variant_max_subcolumns_count());
+ column->set_variant_enable_doc_mode(variant_type.enable_doc_mode());
+ break;
+ }
+ case TYPE_ARRAY: {
+ TabletColumn item;
+ item.set_name("item");
+ set_type_from_data_type(assert_cast<const
DataTypeArray&>(*type).get_nested_type(), &item);
+ column->add_sub_column(item);
+ break;
+ }
+ case TYPE_MAP: {
+ const auto& map_type = assert_cast<const DataTypeMap&>(*type);
+ TabletColumn key;
+ key.set_name("key");
+ set_type_from_data_type(map_type.get_key_type(), &key);
+ column->add_sub_column(key);
+ TabletColumn value;
+ value.set_name("value");
+ set_type_from_data_type(map_type.get_value_type(), &value);
+ column->add_sub_column(value);
+ break;
+ }
+ case TYPE_STRUCT: {
+ const auto& struct_type = assert_cast<const DataTypeStruct&>(*type);
+ for (size_t i = 0; i < struct_type.get_elements().size(); ++i) {
+ TabletColumn field;
+ field.set_name(struct_type.get_element_name(i));
+ set_type_from_data_type(struct_type.get_element(i), &field);
+ column->add_sub_column(field);
+ }
+ break;
+ }
+ default:
+ break;
+ }
+ column->set_length(cast_set<int32_t>(
+ TabletColumn::get_field_length_by_type(to_thrift(primitive_type),
string_length)));
+}
+
} // namespace
std::vector<TabletColumnPtr> project_columns_by_ordinal(
@@ -286,4 +370,15 @@ IColumn::MutablePtr
ReadSchema::get_predicate_column_ptr(const DataTypePtr& data
return data_type->create_column();
}
+TabletColumn ReadSchema::tablet_column_from_slot(const SlotDescriptor& slot) {
+ TabletColumn column;
+ column.set_name(slot.col_name());
+ column.set_unique_id(slot.col_unique_id());
+ set_type_from_data_type(slot.type(), &column);
+ if (!slot.col_default_value().empty()) {
+ column.set_default_value(slot.col_default_value());
+ }
+ return column;
+}
+
} // namespace doris
diff --git a/be/src/storage/schema.h b/be/src/storage/schema.h
index 4e27e9eeef9..e5f9397eb2f 100644
--- a/be/src/storage/schema.h
+++ b/be/src/storage/schema.h
@@ -42,6 +42,7 @@ namespace doris {
class ReadSchema;
class Block;
+class SlotDescriptor;
using ReadSchemaSPtr = std::shared_ptr<ReadSchema>;
// Select columns by their ordinal in `columns`, preserving the requested
order and duplicates.
@@ -70,6 +71,10 @@ public:
static IColumn::MutablePtr get_predicate_column_ptr(const DataTypePtr&
data_type,
const ReaderType
reader_type);
+ // Builds the column `slot` reads from the slot's name, unique id, type
and default value.
+ // For a column the tablet schema does not have yet, e.g. one a light
schema change just added.
+ static TabletColumn tablet_column_from_slot(const SlotDescriptor& slot);
+
const std::vector<TabletColumnPtr>& columns() const { return
_read_columns; }
DataTypePtr data_type(size_t ordinal) const {
diff --git a/be/test/storage/read_schema_test.cpp
b/be/test/storage/read_schema_test.cpp
index cd27e5c7533..6cde75ad573 100644
--- a/be/test/storage/read_schema_test.cpp
+++ b/be/test/storage/read_schema_test.cpp
@@ -15,10 +15,12 @@
// specific language governing permissions and limitations
// under the License.
+#include <gen_cpp/Descriptors_types.h>
#include <gen_cpp/olap_file.pb.h>
#include <gtest/gtest.h>
#include <cstdint>
+#include <limits>
#include <memory>
#include <string>
#include <tuple>
@@ -26,8 +28,20 @@
#include <vector>
#include "core/block/block.h"
+#include "core/data_type/data_type_array.h"
+#include "core/data_type/data_type_bitmap.h"
+#include "core/data_type/data_type_date_or_datetime_v2.h"
+#include "core/data_type/data_type_decimal.h"
+#include "core/data_type/data_type_factory.hpp"
+#include "core/data_type/data_type_ipv6.h"
+#include "core/data_type/data_type_jsonb.h"
+#include "core/data_type/data_type_map.h"
+#include "core/data_type/data_type_nullable.h"
#include "core/data_type/data_type_number.h"
+#include "core/data_type/data_type_string.h"
#include "core/data_type/data_type_struct.h"
+#include "core/data_type/data_type_variant_v2.h"
+#include "runtime/descriptors.h"
#include "storage/binlog.h"
#include "storage/schema.h"
#include "util/json/path_in_data.h"
@@ -333,4 +347,119 @@ TEST(ReadSchemaTest, HasExtractedVariantColumns) {
EXPECT_TRUE(read_schema.tablet_has_extracted_variant_columns());
}
+namespace {
+
+// FE sends this length for a STRING type.
+constexpr int FE_STRING_LENGTH = std::numeric_limits<int32_t>::max() - 4;
+
+TSlotDescriptor create_tslot(const DataTypePtr& type, std::string name,
int32_t unique_id) {
+ TSlotDescriptor tslot;
+ type->to_thrift(tslot.slotType);
+ tslot.__set_nullIndicatorBit(type->is_nullable() ? 0 : -1);
+ tslot.__set_colName(std::move(name));
+ tslot.__set_col_unique_id(unique_id);
+ return tslot;
+}
+
+} // namespace
+
+// MAP<STRING, ARRAY<INT>>: the sub columns are built and named the way FE
names them.
+TEST(ReadSchemaTest, TabletColumnFromSlotBuildsSubColumns) {
+ auto type = make_nullable(std::make_shared<DataTypeMap>(
+ make_nullable(std::make_shared<DataTypeString>(FE_STRING_LENGTH,
TYPE_STRING)),
+ make_nullable(std::make_shared<DataTypeArray>(
+ make_nullable(std::make_shared<DataTypeInt32>())))));
+ SlotDescriptor slot(create_tslot(type, "obj", 7));
+
+ TabletColumn column = ReadSchema::tablet_column_from_slot(slot);
+ EXPECT_EQ("obj", column.name());
+ EXPECT_EQ(7, column.unique_id());
+ EXPECT_EQ(FieldType::OLAP_FIELD_TYPE_MAP, column.type());
+ EXPECT_TRUE(column.is_nullable());
+ EXPECT_FALSE(column.has_default_value());
+ ASSERT_EQ(2, column.get_subtype_count());
+ const TabletColumn& key = column.get_sub_column(0);
+ EXPECT_EQ("key", key.name());
+ EXPECT_EQ(FieldType::OLAP_FIELD_TYPE_STRING, key.type());
+ EXPECT_TRUE(key.is_nullable());
+ const TabletColumn& value = column.get_sub_column(1);
+ EXPECT_EQ("value", value.name());
+ EXPECT_EQ(FieldType::OLAP_FIELD_TYPE_ARRAY, value.type());
+ ASSERT_EQ(1, value.get_subtype_count());
+ EXPECT_EQ("item", value.get_sub_column(0).name());
+ EXPECT_EQ(FieldType::OLAP_FIELD_TYPE_INT, value.get_sub_column(0).type());
+}
+
+// The column describes the slot's type: building the type back from it gives
the same type.
+TEST(ReadSchemaTest, TabletColumnFromSlotKeepsTheType) {
+ const auto nullable_int = make_nullable(std::make_shared<DataTypeInt32>());
+ const auto varchar = std::make_shared<DataTypeString>(10, TYPE_VARCHAR);
+ const DataTypes types = {
+ std::make_shared<DataTypeInt32>(),
+ make_nullable(std::make_shared<DataTypeDecimal32>(9, 2)),
+ make_nullable(std::make_shared<DataTypeDecimal128>(38, 10)),
+ make_nullable(std::make_shared<DataTypeDateTimeV2>(3)),
+ make_nullable(std::make_shared<DataTypeDateV2>()),
+ make_nullable(std::make_shared<DataTypeIPv6>()),
+ make_nullable(std::make_shared<DataTypeJsonb>()),
+ std::make_shared<DataTypeBitMap>(),
+
make_nullable(std::make_shared<DataTypeArray>(std::make_shared<DataTypeInt32>())),
+ make_nullable(std::make_shared<DataTypeArray>(
+
make_nullable(std::make_shared<DataTypeArray>(make_nullable(varchar))))),
+
make_nullable(std::make_shared<DataTypeMap>(make_nullable(varchar),
nullable_int)),
+ make_nullable(std::make_shared<DataTypeStruct>(
+ DataTypes {std::make_shared<DataTypeInt32>(),
make_nullable(varchar),
+
make_nullable(std::make_shared<DataTypeMap>(make_nullable(varchar),
+
nullable_int))},
+ Strings {"a", "b", "c"})),
+ };
+ for (const auto& type : types) {
+ SlotDescriptor slot(create_tslot(type, "c", 1));
+ TabletColumn column = ReadSchema::tablet_column_from_slot(slot);
+ EXPECT_EQ(type->get_name(),
+
DataTypeFactory::instance().create_data_type(column)->get_name());
+ }
+}
+
+// Length, precision, scale, variant settings and default value come out as
for a column FE sends.
+TEST(ReadSchemaTest, TabletColumnFromSlotKeepsTypeDetails) {
+ TSlotDescriptor char_tslot =
+ create_tslot(std::make_shared<DataTypeString>(5, TYPE_CHAR), "c",
1);
+ char_tslot.__set_col_default_value("ab");
+ TabletColumn char_column =
ReadSchema::tablet_column_from_slot(SlotDescriptor(char_tslot));
+ EXPECT_EQ(FieldType::OLAP_FIELD_TYPE_CHAR, char_column.type());
+ EXPECT_EQ(5, char_column.length());
+ EXPECT_TRUE(char_column.has_default_value());
+ EXPECT_EQ("ab", char_column.default_value());
+
+ TabletColumn varchar_column =
ReadSchema::tablet_column_from_slot(SlotDescriptor(
+ create_tslot(std::make_shared<DataTypeString>(10, TYPE_VARCHAR),
"c", 1)));
+ EXPECT_EQ(FieldType::OLAP_FIELD_TYPE_VARCHAR, varchar_column.type());
+ EXPECT_EQ(12, varchar_column.length());
+
+ TabletColumn string_column =
ReadSchema::tablet_column_from_slot(SlotDescriptor(
+ create_tslot(std::make_shared<DataTypeString>(FE_STRING_LENGTH,
TYPE_STRING), "c", 1)));
+ EXPECT_EQ(std::numeric_limits<int32_t>::max(), string_column.length());
+
+ // DECIMALV2 computes as (27, 9) but keeps the declared precision and
scale.
+ TSlotDescriptor decimalv2_tslot =
create_tslot(std::make_shared<DataTypeDecimalV2>(), "c", 1);
+ decimalv2_tslot.slotType.types[0].scalar_type.__set_precision(10);
+ decimalv2_tslot.slotType.types[0].scalar_type.__set_scale(2);
+ TabletColumn decimalv2_column =
+
ReadSchema::tablet_column_from_slot(SlotDescriptor(decimalv2_tslot));
+ EXPECT_EQ(FieldType::OLAP_FIELD_TYPE_DECIMAL, decimalv2_column.type());
+ EXPECT_EQ(10, decimalv2_column.precision());
+ EXPECT_EQ(2, decimalv2_column.frac());
+
+ TSlotDescriptor variant_tslot =
+ create_tslot(make_nullable(std::make_shared<DataTypeVariantV2>()),
"v", 1);
+
variant_tslot.slotType.types[0].scalar_type.__set_variant_max_subcolumns_count(3);
+
variant_tslot.slotType.types[0].scalar_type.__set_variant_enable_doc_mode(true);
+ TabletColumn variant_column =
+ ReadSchema::tablet_column_from_slot(SlotDescriptor(variant_tslot));
+ EXPECT_EQ(FieldType::OLAP_FIELD_TYPE_VARIANT, variant_column.type());
+ EXPECT_EQ(3, variant_column.variant_max_subcolumns_count());
+ EXPECT_TRUE(variant_column.variant_enable_doc_mode());
+}
+
} // namespace doris
diff --git
a/regression-test/data/point_query_p0/test_point_query_light_schema_change.out
b/regression-test/data/point_query_p0/test_point_query_light_schema_change.out
index 1544b3bac8c..da251c55fc8 100644
---
a/regression-test/data/point_query_p0/test_point_query_light_schema_change.out
+++
b/regression-test/data/point_query_p0/test_point_query_light_schema_change.out
@@ -17,3 +17,12 @@
-- !after_load_new_row --
3 3777 42
+-- !added_complex_columns --
+3 3000 \N \N \N
+
+-- !complex_after_load_old_row --
+1 \N \N \N
+
+-- !complex_after_load_new_row --
+4 {"x":[1, 2]} {"a":7, "b":"seven"} [8, 9]
+
diff --git
a/regression-test/suites/point_query_p0/test_point_query_light_schema_change.groovy
b/regression-test/suites/point_query_p0/test_point_query_light_schema_change.groovy
index 525690b6997..b9f9acb849a 100644
---
a/regression-test/suites/point_query_p0/test_point_query_light_schema_change.groovy
+++
b/regression-test/suites/point_query_p0/test_point_query_light_schema_change.groovy
@@ -77,4 +77,23 @@ suite("test_point_query_light_schema_change", "p0") {
sql """ INSERT INTO ${tableName} VALUES (3, 300, 3000, 3777, 42) """
qt_after_load_old_row """ SELECT k, v_default, v_null FROM ${tableName}
WHERE k = 1 """
qt_after_load_new_row """ SELECT k, v_default, v_null FROM ${tableName}
WHERE k = 3 """
+
+ // MAP, STRUCT and ARRAY columns added the same way. The column built from
the slot needs
+ // their sub columns; the rows written before the alter read NULL for them.
+ sql """
+ ALTER TABLE ${tableName} ADD COLUMN (
+ v_map MAP<STRING, ARRAY<INT>>,
+ v_struct STRUCT<a:INT, b:VARCHAR(10)>,
+ v_array ARRAY<INT>
+ )
+ """
+ waitForSchemaChangeDone({
+ sql schemaChangeStatus
+ time 600
+ })
+ qt_added_complex_columns """ SELECT k, v2, v_map, v_struct, v_array FROM
${tableName} WHERE k = 3 """
+
+ sql """ INSERT INTO ${tableName} VALUES (4, 400, 4000, 4777, 44, {'x': [1,
2]}, {7, 'seven'}, [8, 9]) """
+ qt_complex_after_load_old_row """ SELECT k, v_map, v_struct, v_array FROM
${tableName} WHERE k = 1 """
+ qt_complex_after_load_new_row """ SELECT k, v_map, v_struct, v_array FROM
${tableName} WHERE k = 4 """
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
