github-actions[bot] commented on code in PR #69070:
URL: https://github.com/apache/doris/pull/69070#discussion_r4272308303
##########
be/src/storage/index/zone_map/zone_map_index.h:
##########
@@ -95,7 +96,8 @@ class ZoneMapIndexWriter {
virtual ~ZoneMapIndexWriter() = default;
- virtual void add_values(const void* values, size_t count) = 0;
+ // Rows [row_pos, row_pos + count) of `column`.
+ virtual void add(const IColumn& column, size_t row_pos, size_t count) = 0;
Review Comment:
[P2] Migrate the active zone-map benchmark callers
The supported benchmark target (`./build.sh --benchmark` with its required
RELEASE configuration) includes `be/benchmark/benchmark_zone_map_index.hpp` via
`benchmark_main.cpp:39`, but `run()` and `run_column_writer_like()` still call
`w->add_values(...)` (lines 132 and 165) on `ZoneMapIndexWriter`. Removing this
member therefore makes `benchmark_test` fail to compile. Please migrate those
fixtures/calls to the column-range API as part of the interface change;
construct the columns outside the timed loops so the benchmark continues to
measure zone-map work.
##########
be/src/storage/index/zone_map/zone_map_index.cpp:
##########
@@ -200,49 +202,67 @@ void
TypedZoneMapIndexWriter<Type>::_update_page_zonemap(const ValType& min_valu
_page_zone_map.has_not_null = true;
}
-template <PrimitiveType Type>
-void TypedZoneMapIndexWriter<Type>::_materialize_page_minmax() {
+template <FieldType FT>
+void TypedZoneMapIndexWriter<FT>::_materialize_page_minmax() {
if (!_page_zone_map.has_not_null) {
return;
}
- _page_zone_map.min_value =
doris::Field::create_field_from_olap_value<Type>(_page_min);
- _page_zone_map.max_value =
doris::Field::create_field_from_olap_value<Type>(_page_max);
+ if constexpr (is_string_type(Type)) {
+ _page_zone_map.min_value =
doris::Field::create_field_from_olap_value<Type>(_page_min);
+ _page_zone_map.max_value =
doris::Field::create_field_from_olap_value<Type>(_page_max);
+ } else {
+ // Bounds are kept as compute-layer values here: for example, a DATE
is a
+ // VecDateTimeValue and a DECIMALV2 is a DecimalV2Value. Decode only
values
+ // read from storage; interpreting these as packed storage values
would turn
+ // 2020-01-01 into an invalid DATE and would read DECIMALV2 through
decimal12_t.
+ _page_zone_map.min_value = doris::Field::create_field<Type>(_page_min);
Review Comment:
[P1] Normalize default V1 date bounds before serialization
The deleted-key partial-update scenario added in this PR reaches
`ColumnDate`/`ColumnDateTime::insert_default()`. Their
`VecDateTimeValue::DEFAULT_VALUE` is initialized with primitive `TYPE_DATETIME`
(12) in a three-bit time tag, so its tag is 4, outside
`TIME_DATE`/`TIME_DATETIME`. Copying that compute value here preserves the tag:
`to_buffer()` returns zero and the zone-map bounds serialize as empty strings.
The reader's date serde turns those strings into `FIRST_DAY` and returns OK,
leaving usable segment bounds at year 1 although the stored value is
1970-01-01. Equality/greater-than predicates can then prune the committed row.
The removed packed-storage decode normalized this tag. Please normalize
DATE/DATETIME bound copies to their time kind (or correct the default constant)
before materializing them. This is a separate default-value failure from the
earlier thread's now-fixed ordinary DATE/DECIMAL representation mismatch.
--
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]
