Uh oh!
There was an error while loading. Please reload this page.
[refactor](be) remove CHAR padding on read - #63291
Merged
yiguolei merged 1 commit intoJun 1, 2026
Merged
Conversation
hello-stephen
commented
May 15, 2026
Contributor
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
csun5285
commented
May 15, 2026
ContributorAuthor
run buildall |
csun5285
commented
May 15, 2026
ContributorAuthor
run buildall |
hello-stephen
commented
May 15, 2026
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
hello-stephen
commented
May 15, 2026
Contributor
TPC-H: Total hot run time: 31623 ms |
hello-stephen
commented
May 15, 2026
Contributor
TPC-DS: Total hot run time: 168893 ms |
hello-stephen
commented
May 15, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
May 15, 2026
Contributor
FE Regression Coverage ReportIncrement line coverage |
csun5285force-pushed
the
feature/shrink-char-padding-pushdown
branch
2 times, most recently
from
May 16, 2026 01:46
fb80b35 to
2fb1707Comparecsun5285
commented
May 16, 2026
ContributorAuthor
run buildall |
hello-stephen
commented
May 16, 2026
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
hello-stephen
commented
May 16, 2026
Contributor
TPC-H: Total hot run time: 31114 ms |
hello-stephen
commented
May 16, 2026
Contributor
TPC-DS: Total hot run time: 168927 ms |
hello-stephen
commented
May 16, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
May 16, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
csun5285force-pushed
the
feature/shrink-char-padding-pushdown
branch
3 times, most recently
from
May 21, 2026 09:53
ce3ec6e to
3b3a6e4CompareContributor
PR approved by anyone and no changes requested. |
hello-stephen
commented
May 29, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
There was a problem hiding this comment.
I found correctness issues that can cause false pruning or value truncation.
Critical checkpoint conclusions:
- Goal/test: the PR aims to make CHAR values unpadded at read/predicate time, but the current implementation does not fully preserve string/CHAR byte correctness and does not cover the inverted-index path.
- Scope/focus: the change is broad across readers, predicates, and page decoders; some paths now rely on predecode-only normalization, which exposes missed index/read paths.
- Concurrency/lifecycle/config/compatibility: no new concurrency, lifecycle, config, or wire/storage-format compatibility issue was identified beyond reading existing padded storage.
- Parallel paths: bloom filters were disabled for unpadded CHAR predicates, but string inverted-index predicates still query unpadded values against padded indexed terms.
- Tests: added unit tests cover several decoder/serde cases, but they do not cover the failing inverted-index scenario or embedded-NUL truncation.
- Observability/transactions/writes: not applicable for this PR.
User focus: no additional user-provided review focus was supplied.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
csun5285
commented
May 29, 2026
ContributorAuthor
|
bobhan1
approved these changes
May 29, 2026
gavinchou
approved these changes
Jun 1, 2026
Uh oh!
There was an error while loading. Please reload this page.
csun5285 added a commit
to csun5285/doris
that referenced
this pull request
Jun 2, 2026
PredicateColumnType<T> was a storage-layer wrapper that flattened every
predicate column into PaddedPODArray<value_type> so the SIMD-friendly
predicate-eval loops could use a uniform `data_array[i]` access regardless
of the underlying type. The cost was: a parallel column hierarchy with
restricted API, an extra 16 bytes/row (StringRef header) for strings on
top of the actual chars, and the need for callers to thread the column's
"role" (predicate vs output) down through the read path.
This change retires PredicateColumnType entirely:
- Schema::get_predicate_column_ptr now allocates the canonical
PrimitiveTypeTraits<T>::ColumnType (ColumnVector / ColumnDecimal /
ColumnString / ColumnIPv4 / ...). ColumnDictI32 is unchanged: it
still serves the low-cardinality string fast path for predicate eval.
- filter_by_selector is now implemented on ColumnVector<T> and
ColumnDecimal<T> with the same selector-gather semantics
PredicateColumnType used.
- ColumnString does not expose a contiguous StringRef[] array, so per-row
access in predicate evaluators flows through ColumnElementView<Type>
(already in core/column/column_execute_util.h for the compute layer)
rather than a pointer subscript:
ColumnElementView<Type> view {column};
_base_loop_vec<...>(size, flags, null_map, view, _value);
ColumnElementView numeric specialization yields T via pointer
arithmetic; the string specialization yields StringRef via
get_data_at — same `view[i]` call shape, no if-constexpr at the
call site. The compute-layer ColumnElementView gains size() /
operator[] aliases and its TYPE_STRING specialization is generalized
to all is_string_type(PType) via a defaulted bool template param so
TYPE_CHAR / TYPE_VARCHAR / TYPE_JSONB all resolve correctly.
- For HybridSet::find paths that need const T* per row, a small
ColumnPointerCursor<Type> lives next to ColumnElementView. Numeric
specialization holds a const T* (zero copy); string specialization
stages each row into a member StringRef and returns its address
(HybridSet::find consumes synchronously, so the staged-cell reuse
is safe).
- BloomFilter / BitmapFilter find_fixed_len_olap_engine API is
redesigned to take `const IColumn&` instead of a `const char*` that
was reinterpreted as `const T[]`. CommonFindOp / StringFindOp each
specialize per-row access:
CommonFindOp: reads column.get_raw_data() as `const T*`, passes
`[data](int i){return data[i];}` as the accessor.
StringFindOp: `assert_cast<const ColumnString&>(column)`, passes
`[&col](int i){return col.get_data_at(i);}`.
Storage-side BF/Bitmap callers collapse to one line and the previous
workaround (materializing a temporary `vector<StringRef>` to satisfy
the legacy char* API) goes away — saves one jemalloc + N×16-byte
store/load per evaluate on string columns.
- Schema-template CHAR predicate columns previously got trailing-zero
padding stripped on every PredicateColumnType<TYPE_CHAR>::get_data_at
call. ColumnString no longer does that; the strip is handled by
Block::shrink_char_type_column_suffix_zero on the output side (and
page-decoder-level for the read path per upstream PR apache#63291), so no
extra pass is needed here.
- ColumnDictI32::convert_to_predicate_column_if_dictionary now produces
ColumnString for mid-batch dict->plain fallback.
- predicate_column.h / predicate_column_test.cpp deleted; the
PredicateColumnHolderType<T> transition alias and all
core/column/predicate_column.h includes are removed.
On the _base_loop_vec signature: upstream used
`const TArray* __restrict data_array` which worked because
PredicateColumnType<TYPE_STRING> physically held a contiguous StringRef[].
After this change the string side is ColumnElementView<TYPE_STRING> (a
struct value), and `__restrict` is a pointer-only qualifier — so we pass
TArray by value without it. Verified on a Release build (objdump) that
this does NOT regress SIMD: vectorizable types (INT / BIGINT /
dict-encoded string) still emit fully-vectorized loops (vpcmpeqd /
vpcmpeqq, 4× unrolled, 16 elements/iter). The compiler's loop versioning
emits one runtime alias check at function entry (~5 cycles, <0.5% of a
1024-row batch's total cost); the main loop body is identical to the
__restrict version. Non-vectorizable types (LARGEINT / DOUBLE / DECIMAL /
ColumnString memcmp) were scalar regardless of __restrict. See the
comment above _base_loop_vec for full rationale.
Storage compiles clean in both Release and ASAN trees. Targeted UTs:
319 tests across 13 suites (Column* / Predicate* / Segment* /
BloomFilterFunc / BitmapFilterPredicate /
ColumnExecuteUtil): PASS, 0 FAIL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>csun5285 added a commit
to csun5285/doris
that referenced
this pull request
Jun 2, 2026
PredicateColumnType<T> was a storage-layer wrapper that flattened every
predicate column into PaddedPODArray<value_type> so the SIMD-friendly
predicate-eval loops could use a uniform `data_array[i]` access regardless
of the underlying type. The cost was: a parallel column hierarchy with
restricted API, an extra 16 bytes/row (StringRef header) for strings on
top of the actual chars, and the need for callers to thread the column's
"role" (predicate vs output) down through the read path.
This change retires PredicateColumnType entirely:
- Schema::get_predicate_column_ptr now allocates the canonical
PrimitiveTypeTraits<T>::ColumnType (ColumnVector / ColumnDecimal /
ColumnString / ColumnIPv4 / ...). ColumnDictI32 is unchanged: it
still serves the low-cardinality string fast path for predicate eval.
- filter_by_selector is now implemented on ColumnVector<T> and
ColumnDecimal<T> with the same selector-gather semantics
PredicateColumnType used.
- ColumnString does not expose a contiguous StringRef[] array, so per-row
access in predicate evaluators flows through ColumnElementView<Type>
(already in core/column/column_execute_util.h for the compute layer)
rather than a pointer subscript:
ColumnElementView<Type> view {column};
_base_loop_vec<...>(size, flags, null_map, view, _value);
ColumnElementView numeric specialization yields T via pointer
arithmetic; the string specialization yields StringRef via
get_data_at — same `view[i]` call shape, no if-constexpr at the
call site. The compute-layer ColumnElementView gains size() /
operator[] aliases and its TYPE_STRING specialization is generalized
to all is_string_type(PType) via a defaulted bool template param so
TYPE_CHAR / TYPE_VARCHAR / TYPE_JSONB all resolve correctly.
- For HybridSet::find paths that need const T* per row, a small
ColumnPointerCursor<Type> lives next to ColumnElementView. Numeric
specialization holds a const T* (zero copy); string specialization
stages each row into a member StringRef and returns its address
(HybridSet::find consumes synchronously, so the staged-cell reuse
is safe).
- BloomFilter / BitmapFilter find_fixed_len_olap_engine API is
redesigned to take `const IColumn&` instead of a `const char*` that
was reinterpreted as `const T[]`. CommonFindOp / StringFindOp each
specialize per-row access:
CommonFindOp: reads column.get_raw_data() as `const T*`, passes
`[data](int i){return data[i];}` as the accessor.
StringFindOp: `assert_cast<const ColumnString&>(column)`, passes
`[&col](int i){return col.get_data_at(i);}`.
Storage-side BF/Bitmap callers collapse to one line and the previous
workaround (materializing a temporary `vector<StringRef>` to satisfy
the legacy char* API) goes away — saves one jemalloc + N×16-byte
store/load per evaluate on string columns.
- Schema-template CHAR predicate columns previously got trailing-zero
padding stripped on every PredicateColumnType<TYPE_CHAR>::get_data_at
call. ColumnString no longer does that; the strip is handled by
Block::shrink_char_type_column_suffix_zero on the output side (and
page-decoder-level for the read path per upstream PR apache#63291), so no
extra pass is needed here.
- ColumnDictI32::convert_to_predicate_column_if_dictionary now produces
ColumnString for mid-batch dict->plain fallback.
- predicate_column.h / predicate_column_test.cpp deleted; the
PredicateColumnHolderType<T> transition alias and all
core/column/predicate_column.h includes are removed.
On the _base_loop_vec signature: upstream used
`const TArray* __restrict data_array` which worked because
PredicateColumnType<TYPE_STRING> physically held a contiguous StringRef[].
After this change the string side is ColumnElementView<TYPE_STRING> (a
struct value), and `__restrict` is a pointer-only qualifier — so we pass
TArray by value without it. Verified on a Release build (objdump) that
this does NOT regress SIMD: vectorizable types (INT / BIGINT /
dict-encoded string) still emit fully-vectorized loops (vpcmpeqd /
vpcmpeqq, 4× unrolled, 16 elements/iter). The compiler's loop versioning
emits one runtime alias check at function entry (~5 cycles, <0.5% of a
1024-row batch's total cost); the main loop body is identical to the
__restrict version. Non-vectorizable types (LARGEINT / DOUBLE / DECIMAL /
ColumnString memcmp) were scalar regardless of __restrict. See the
comment above _base_loop_vec for full rationale.
Storage compiles clean in both Release and ASAN trees. Targeted UTs:
319 tests across 13 suites (Column* / Predicate* / Segment* /
BloomFilterFunc / BitmapFilterPredicate /
ColumnExecuteUtil): PASS, 0 FAIL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>csun5285 added a commit
to csun5285/doris
that referenced
this pull request
Jun 2, 2026
PredicateColumnType<T> was a storage-layer wrapper that flattened every
predicate column into PaddedPODArray<value_type> so the SIMD-friendly
predicate-eval loops could use a uniform `data_array[i]` access regardless
of the underlying type. The cost was: a parallel column hierarchy with
restricted API, an extra 16 bytes/row (StringRef header) for strings on
top of the actual chars, and the need for callers to thread the column's
"role" (predicate vs output) down through the read path.
This change retires PredicateColumnType entirely:
- Schema::get_predicate_column_ptr now allocates the canonical
PrimitiveTypeTraits<T>::ColumnType (ColumnVector / ColumnDecimal /
ColumnString / ColumnIPv4 / ...). ColumnDictI32 is unchanged: it
still serves the low-cardinality string fast path for predicate eval.
- filter_by_selector is now implemented on ColumnVector<T> and
ColumnDecimal<T> with the same selector-gather semantics
PredicateColumnType used.
- ColumnString does not expose a contiguous StringRef[] array, so per-row
access in predicate evaluators flows through ColumnElementView<Type>
(already in core/column/column_execute_util.h for the compute layer)
rather than a pointer subscript:
ColumnElementView<Type> view {column};
_base_loop_vec<...>(size, flags, null_map, view, _value);
ColumnElementView numeric specialization yields T via pointer
arithmetic; the string specialization yields StringRef via
get_data_at — same `view[i]` call shape, no if-constexpr at the
call site. The compute-layer ColumnElementView gains size() /
operator[] aliases and its TYPE_STRING specialization is generalized
to all is_string_type(PType) via a defaulted bool template param so
TYPE_CHAR / TYPE_VARCHAR / TYPE_JSONB all resolve correctly.
- For HybridSet::find paths that need const T* per row, a small
ColumnPointerCursor<Type> lives next to ColumnElementView. Numeric
specialization holds a const T* (zero copy); string specialization
stages each row into a member StringRef and returns its address
(HybridSet::find consumes synchronously, so the staged-cell reuse
is safe).
- BloomFilter / BitmapFilter find_fixed_len_olap_engine API is
redesigned to take `const IColumn&` instead of a `const char*` that
was reinterpreted as `const T[]`. CommonFindOp / StringFindOp each
specialize per-row access:
CommonFindOp: reads column.get_raw_data() as `const T*`, passes
`[data](int i){return data[i];}` as the accessor.
StringFindOp: `assert_cast<const ColumnString&>(column)`, passes
`[&col](int i){return col.get_data_at(i);}`.
Storage-side BF/Bitmap callers collapse to one line and the previous
workaround (materializing a temporary `vector<StringRef>` to satisfy
the legacy char* API) goes away — saves one jemalloc + N×16-byte
store/load per evaluate on string columns.
- Schema-template CHAR predicate columns previously got trailing-zero
padding stripped on every PredicateColumnType<TYPE_CHAR>::get_data_at
call. ColumnString no longer does that; the strip is handled by
Block::shrink_char_type_column_suffix_zero on the output side (and
page-decoder-level for the read path per upstream PR apache#63291), so no
extra pass is needed here.
- ColumnDictI32::convert_to_predicate_column_if_dictionary now produces
ColumnString for mid-batch dict->plain fallback.
- predicate_column.h / predicate_column_test.cpp deleted; the
PredicateColumnHolderType<T> transition alias and all
core/column/predicate_column.h includes are removed.
On the _base_loop_vec signature: upstream used
`const TArray* __restrict data_array` which worked because
PredicateColumnType<TYPE_STRING> physically held a contiguous StringRef[].
After this change the string side is ColumnElementView<TYPE_STRING> (a
struct value), and `__restrict` is a pointer-only qualifier — so we pass
TArray by value without it. Verified on a Release build (objdump) that
this does NOT regress SIMD: vectorizable types (INT / BIGINT /
dict-encoded string) still emit fully-vectorized loops (vpcmpeqd /
vpcmpeqq, 4× unrolled, 16 elements/iter). The compiler's loop versioning
emits one runtime alias check at function entry (~5 cycles, <0.5% of a
1024-row batch's total cost); the main loop body is identical to the
__restrict version. Non-vectorizable types (LARGEINT / DOUBLE / DECIMAL /
ColumnString memcmp) were scalar regardless of __restrict. See the
comment above _base_loop_vec for full rationale.
Storage compiles clean in both Release and ASAN trees. Targeted UTs:
319 tests across 13 suites (Column* / Predicate* / Segment* /
BloomFilterFunc / BitmapFilterPredicate /
ColumnExecuteUtil): PASS, 0 FAIL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>csun5285 added a commit
to csun5285/doris
that referenced
this pull request
Jun 2, 2026
PredicateColumnType<T> was a storage-layer wrapper that flattened every
predicate column into PaddedPODArray<value_type> so the SIMD-friendly
predicate-eval loops could use a uniform `data_array[i]` access regardless
of the underlying type. The cost was: a parallel column hierarchy with
restricted API, an extra 16 bytes/row (StringRef header) for strings on
top of the actual chars, and the need for callers to thread the column's
"role" (predicate vs output) down through the read path.
This change retires PredicateColumnType entirely:
- Schema::get_predicate_column_ptr now allocates the canonical
PrimitiveTypeTraits<T>::ColumnType (ColumnVector / ColumnDecimal /
ColumnString / ColumnIPv4 / ...). ColumnDictI32 is unchanged: it
still serves the low-cardinality string fast path for predicate eval.
- filter_by_selector is now implemented on ColumnVector<T> and
ColumnDecimal<T> with the same selector-gather semantics
PredicateColumnType used.
- ColumnString does not expose a contiguous StringRef[] array, so per-row
access in predicate evaluators flows through ColumnElementView<Type>
(already in core/column/column_execute_util.h for the compute layer)
rather than a pointer subscript:
ColumnElementView<Type> view {column};
_base_loop_vec<...>(size, flags, null_map, view, _value);
ColumnElementView numeric specialization yields T via pointer
arithmetic; the string specialization yields StringRef via
get_data_at — same `view[i]` call shape, no if-constexpr at the
call site. The compute-layer ColumnElementView gains size() /
operator[] aliases and its TYPE_STRING specialization is generalized
to all is_string_type(PType) via a defaulted bool template param so
TYPE_CHAR / TYPE_VARCHAR / TYPE_JSONB all resolve correctly.
- For HybridSet::find paths that need const T* per row, a small
ColumnPointerCursor<Type> lives next to ColumnElementView. Numeric
specialization holds a const T* (zero copy); string specialization
stages each row into a member StringRef and returns its address
(HybridSet::find consumes synchronously, so the staged-cell reuse
is safe).
- BloomFilter / BitmapFilter find_fixed_len_olap_engine API is
redesigned to take `const IColumn&` instead of a `const char*` that
was reinterpreted as `const T[]`. CommonFindOp / StringFindOp each
specialize per-row access:
CommonFindOp: reads column.get_raw_data() as `const T*`, passes
`[data](int i){return data[i];}` as the accessor.
StringFindOp: `assert_cast<const ColumnString&>(column)`, passes
`[&col](int i){return col.get_data_at(i);}`.
Storage-side BF/Bitmap callers collapse to one line and the previous
workaround (materializing a temporary `vector<StringRef>` to satisfy
the legacy char* API) goes away — saves one jemalloc + N×16-byte
store/load per evaluate on string columns.
- Schema-template CHAR predicate columns previously got trailing-zero
padding stripped on every PredicateColumnType<TYPE_CHAR>::get_data_at
call. ColumnString no longer does that; the strip is handled by
Block::shrink_char_type_column_suffix_zero on the output side (and
page-decoder-level for the read path per upstream PR apache#63291), so no
extra pass is needed here.
- ColumnDictI32::convert_to_predicate_column_if_dictionary now produces
ColumnString for mid-batch dict->plain fallback.
- predicate_column.h / predicate_column_test.cpp deleted; the
PredicateColumnHolderType<T> transition alias and all
core/column/predicate_column.h includes are removed.
On the _base_loop_vec signature: upstream used
`const TArray* __restrict data_array` which worked because
PredicateColumnType<TYPE_STRING> physically held a contiguous StringRef[].
After this change the string side is ColumnElementView<TYPE_STRING> (a
struct value), and `__restrict` is a pointer-only qualifier — so we pass
TArray by value without it. Verified on a Release build (objdump) that
this does NOT regress SIMD: vectorizable types (INT / BIGINT /
dict-encoded string) still emit fully-vectorized loops (vpcmpeqd /
vpcmpeqq, 4× unrolled, 16 elements/iter). The compiler's loop versioning
emits one runtime alias check at function entry (~5 cycles, <0.5% of a
1024-row batch's total cost); the main loop body is identical to the
__restrict version. Non-vectorizable types (LARGEINT / DOUBLE / DECIMAL /
ColumnString memcmp) were scalar regardless of __restrict. See the
comment above _base_loop_vec for full rationale.
Storage compiles clean in both Release and ASAN trees. Targeted UTs:
319 tests across 13 suites (Column* / Predicate* / Segment* /
BloomFilterFunc / BitmapFilterPredicate /
ColumnExecuteUtil): PASS, 0 FAIL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>csun5285 added a commit
to csun5285/doris
that referenced
this pull request
Jun 2, 2026
PredicateColumnType<T> was a storage-layer wrapper that flattened every
predicate column into PaddedPODArray<value_type> so the SIMD-friendly
predicate-eval loops could use a uniform `data_array[i]` access regardless
of the underlying type. The cost was: a parallel column hierarchy with
restricted API, an extra 16 bytes/row (StringRef header) for strings on
top of the actual chars, and the need for callers to thread the column's
"role" (predicate vs output) down through the read path.
This change retires PredicateColumnType entirely:
- Schema::get_predicate_column_ptr now allocates the canonical
PrimitiveTypeTraits<T>::ColumnType (ColumnVector / ColumnDecimal /
ColumnString / ColumnIPv4 / ...). ColumnDictI32 is unchanged: it
still serves the low-cardinality string fast path for predicate eval.
- filter_by_selector is now implemented on ColumnVector<T> and
ColumnDecimal<T> with the same selector-gather semantics
PredicateColumnType used.
- ColumnString does not expose a contiguous StringRef[] array, so per-row
access in predicate evaluators flows through ColumnElementView<Type>
(already in core/column/column_execute_util.h for the compute layer)
rather than a pointer subscript:
ColumnElementView<Type> view {column};
_base_loop_vec<...>(size, flags, null_map, view, _value);
ColumnElementView numeric specialization yields T via pointer
arithmetic; the string specialization yields StringRef via
get_data_at — same `view[i]` call shape, no if-constexpr at the
call site. The compute-layer ColumnElementView gains size() /
operator[] aliases and its TYPE_STRING specialization is generalized
to all is_string_type(PType) via a defaulted bool template param so
TYPE_CHAR / TYPE_VARCHAR / TYPE_JSONB all resolve correctly.
- For HybridSet::find paths that need const T* per row, a small
ColumnPointerCursor<Type> lives next to ColumnElementView. Numeric
specialization holds a const T* (zero copy); string specialization
stages each row into a member StringRef and returns its address
(HybridSet::find consumes synchronously, so the staged-cell reuse
is safe).
- BloomFilter / BitmapFilter find_fixed_len_olap_engine API is
redesigned to take `const IColumn&` instead of a `const char*` that
was reinterpreted as `const T[]`. CommonFindOp / StringFindOp each
specialize per-row access:
CommonFindOp: reads column.get_raw_data() as `const T*`, passes
`[data](int i){return data[i];}` as the accessor.
StringFindOp: `assert_cast<const ColumnString&>(column)`, passes
`[&col](int i){return col.get_data_at(i);}`.
Storage-side BF/Bitmap callers collapse to one line and the previous
workaround (materializing a temporary `vector<StringRef>` to satisfy
the legacy char* API) goes away — saves one jemalloc + N×16-byte
store/load per evaluate on string columns.
- Schema-template CHAR predicate columns previously got trailing-zero
padding stripped on every PredicateColumnType<TYPE_CHAR>::get_data_at
call. ColumnString no longer does that; the strip is handled by
Block::shrink_char_type_column_suffix_zero on the output side (and
page-decoder-level for the read path per upstream PR apache#63291), so no
extra pass is needed here.
- ColumnDictI32::convert_to_predicate_column_if_dictionary now produces
ColumnString for mid-batch dict->plain fallback.
- predicate_column.h / predicate_column_test.cpp deleted; the
PredicateColumnHolderType<T> transition alias and all
core/column/predicate_column.h includes are removed.
On the _base_loop_vec signature: upstream used
`const TArray* __restrict data_array` which worked because
PredicateColumnType<TYPE_STRING> physically held a contiguous StringRef[].
After this change the string side is ColumnElementView<TYPE_STRING> (a
struct value), and `__restrict` is a pointer-only qualifier — so we pass
TArray by value without it. Verified on a Release build (objdump) that
this does NOT regress SIMD: vectorizable types (INT / BIGINT /
dict-encoded string) still emit fully-vectorized loops (vpcmpeqd /
vpcmpeqq, 4× unrolled, 16 elements/iter). The compiler's loop versioning
emits one runtime alias check at function entry (~5 cycles, <0.5% of a
1024-row batch's total cost); the main loop body is identical to the
__restrict version. Non-vectorizable types (LARGEINT / DOUBLE / DECIMAL /
ColumnString memcmp) were scalar regardless of __restrict. See the
comment above _base_loop_vec for full rationale.
Storage compiles clean in both Release and ASAN trees. Targeted UTs:
319 tests across 13 suites (Column* / Predicate* / Segment* /
BloomFilterFunc / BitmapFilterPredicate /
ColumnExecuteUtil): PASS, 0 FAIL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>zhaorongsheng pushed a commit
to zhaorongsheng/doris
that referenced
this pull request
Jun 4, 2026
- apache/doris-website#3759 - Problem: The CHAR padding contract leaked from the storage layer into the compute / predicate layers — every scan stripped padding at the Block level, while predicates re-padded values to match the on-disk shape. Logic was spread out and wasted work on every read. - Fix: On-disk format unchanged. The convertor still pads CHAR to the schema length on write, but the strip is pushed down to the page pre-decoder — the page cache holds unpadded data. All shrink_* / pad_* code above the page cache (SegmentIterator, Block, RowCursor, predicates) is removed. - BloomFilter: BF probing is skipped (return true, fall back to scan) for CHAR predicates — the BF hashes padded bytes but predicate values are unpadded, so the probe would never match. Other indexes (ZoneMap / inverted / bitmap) are unaffected. ### What problem does this PR solve? 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. --> - [x] Regression test - [x] 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: apache/doris-website#3759> ### 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 4.7 (1M context) <noreply@anthropic.com>
csun5285 added a commit
to csun5285/doris
that referenced
this pull request
Jun 4, 2026
PredicateColumnType<T> was a storage-layer wrapper that flattened every
predicate column into PaddedPODArray<value_type> so the SIMD-friendly
predicate-eval loops could use a uniform `data_array[i]` access regardless
of the underlying type. The cost was: a parallel column hierarchy with
restricted API, an extra 16 bytes/row (StringRef header) for strings on
top of the actual chars, and the need for callers to thread the column's
"role" (predicate vs output) down through the read path.
This change retires PredicateColumnType entirely:
- Schema::get_predicate_column_ptr now allocates the canonical
PrimitiveTypeTraits<T>::ColumnType (ColumnVector / ColumnDecimal /
ColumnString / ColumnIPv4 / ...). ColumnDictI32 is unchanged: it
still serves the low-cardinality string fast path for predicate eval.
- filter_by_selector is now implemented on ColumnVector<T> and
ColumnDecimal<T> with the same selector-gather semantics
PredicateColumnType used.
- ColumnString does not expose a contiguous StringRef[] array, so per-row
access in predicate evaluators flows through ColumnElementView<Type>
(already in core/column/column_execute_util.h for the compute layer)
rather than a pointer subscript:
ColumnElementView<Type> view {column};
_base_loop_vec<...>(size, flags, null_map, view, _value);
ColumnElementView numeric specialization yields T via pointer
arithmetic; the string specialization yields StringRef via
get_data_at — same `view[i]` call shape, no if-constexpr at the
call site. The compute-layer ColumnElementView gains size() /
operator[] aliases and its TYPE_STRING specialization is generalized
to all is_string_type(PType) via a defaulted bool template param so
TYPE_CHAR / TYPE_VARCHAR / TYPE_JSONB all resolve correctly.
- For HybridSet::find paths that need const T* per row, a small
ColumnPointerCursor<Type> lives next to ColumnElementView. Numeric
specialization holds a const T* (zero copy); string specialization
stages each row into a member StringRef and returns its address
(HybridSet::find consumes synchronously, so the staged-cell reuse
is safe).
- BloomFilter / BitmapFilter find_fixed_len_olap_engine API is
redesigned to take `const IColumn&` instead of a `const char*` that
was reinterpreted as `const T[]`. CommonFindOp / StringFindOp each
specialize per-row access:
CommonFindOp: reads column.get_raw_data() as `const T*`, passes
`[data](int i){return data[i];}` as the accessor.
StringFindOp: `assert_cast<const ColumnString&>(column)`, passes
`[&col](int i){return col.get_data_at(i);}`.
Storage-side BF/Bitmap callers collapse to one line and the previous
workaround (materializing a temporary `vector<StringRef>` to satisfy
the legacy char* API) goes away — saves one jemalloc + N×16-byte
store/load per evaluate on string columns.
- Schema-template CHAR predicate columns previously got trailing-zero
padding stripped on every PredicateColumnType<TYPE_CHAR>::get_data_at
call. ColumnString no longer does that; the strip is handled by
Block::shrink_char_type_column_suffix_zero on the output side (and
page-decoder-level for the read path per upstream PR apache#63291), so no
extra pass is needed here.
- ColumnDictI32::convert_to_predicate_column_if_dictionary now produces
ColumnString for mid-batch dict->plain fallback.
- predicate_column.h / predicate_column_test.cpp deleted; the
PredicateColumnHolderType<T> transition alias and all
core/column/predicate_column.h includes are removed.
On the _base_loop_vec signature: upstream used
`const TArray* __restrict data_array` which worked because
PredicateColumnType<TYPE_STRING> physically held a contiguous StringRef[].
After this change the string side is ColumnElementView<TYPE_STRING> (a
struct value), and `__restrict` is a pointer-only qualifier — so we pass
TArray by value without it. Verified on a Release build (objdump) that
this does NOT regress SIMD: vectorizable types (INT / BIGINT /
dict-encoded string) still emit fully-vectorized loops (vpcmpeqd /
vpcmpeqq, 4× unrolled, 16 elements/iter). The compiler's loop versioning
emits one runtime alias check at function entry (~5 cycles, <0.5% of a
1024-row batch's total cost); the main loop body is identical to the
__restrict version. Non-vectorizable types (LARGEINT / DOUBLE / DECIMAL /
ColumnString memcmp) were scalar regardless of __restrict. See the
comment above _base_loop_vec for full rationale.
Storage compiles clean in both Release and ASAN trees. Targeted UTs:
319 tests across 13 suites (Column* / Predicate* / Segment* /
BloomFilterFunc / BitmapFilterPredicate /
ColumnExecuteUtil): PASS, 0 FAIL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>csun5285 added a commit
to csun5285/doris
that referenced
this pull request
Jun 4, 2026
PredicateColumnType<T> was a storage-layer wrapper that flattened every
predicate column into PaddedPODArray<value_type> so the SIMD-friendly
predicate-eval loops could use a uniform `data_array[i]` access regardless
of the underlying type. The cost was: a parallel column hierarchy with
restricted API, an extra 16 bytes/row (StringRef header) for strings on
top of the actual chars, and the need for callers to thread the column's
"role" (predicate vs output) down through the read path.
This change retires PredicateColumnType entirely:
- Schema::get_predicate_column_ptr now allocates the canonical
PrimitiveTypeTraits<T>::ColumnType (ColumnVector / ColumnDecimal /
ColumnString / ColumnIPv4 / ...). ColumnDictI32 is unchanged: it
still serves the low-cardinality string fast path for predicate eval.
- filter_by_selector is now implemented on ColumnVector<T> and
ColumnDecimal<T> with the same selector-gather semantics
PredicateColumnType used.
- ColumnString does not expose a contiguous StringRef[] array, so per-row
access in predicate evaluators flows through ColumnElementView<Type>
(already in core/column/column_execute_util.h for the compute layer)
rather than a pointer subscript:
ColumnElementView<Type> view {column};
_base_loop_vec<...>(size, flags, null_map, view, _value);
ColumnElementView numeric specialization yields T via pointer
arithmetic; the string specialization yields StringRef via
get_data_at — same `view[i]` call shape, no if-constexpr at the
call site. The compute-layer ColumnElementView gains size() /
operator[] aliases and its TYPE_STRING specialization is generalized
to all is_string_type(PType) via a defaulted bool template param so
TYPE_CHAR / TYPE_VARCHAR / TYPE_JSONB all resolve correctly.
- For HybridSet::find paths that need const T* per row, a small
ColumnPointerCursor<Type> lives next to ColumnElementView. Numeric
specialization holds a const T* (zero copy); string specialization
stages each row into a member StringRef and returns its address
(HybridSet::find consumes synchronously, so the staged-cell reuse
is safe).
- BloomFilter / BitmapFilter find_fixed_len_olap_engine API is
redesigned to take `const IColumn&` instead of a `const char*` that
was reinterpreted as `const T[]`. CommonFindOp / StringFindOp each
specialize per-row access:
CommonFindOp: reads column.get_raw_data() as `const T*`, passes
`[data](int i){return data[i];}` as the accessor.
StringFindOp: `assert_cast<const ColumnString&>(column)`, passes
`[&col](int i){return col.get_data_at(i);}`.
Storage-side BF/Bitmap callers collapse to one line and the previous
workaround (materializing a temporary `vector<StringRef>` to satisfy
the legacy char* API) goes away — saves one jemalloc + N×16-byte
store/load per evaluate on string columns.
- Schema-template CHAR predicate columns previously got trailing-zero
padding stripped on every PredicateColumnType<TYPE_CHAR>::get_data_at
call. ColumnString no longer does that; the strip is handled by
Block::shrink_char_type_column_suffix_zero on the output side (and
page-decoder-level for the read path per upstream PR apache#63291), so no
extra pass is needed here.
- ColumnDictI32::convert_to_predicate_column_if_dictionary now produces
ColumnString for mid-batch dict->plain fallback.
- predicate_column.h / predicate_column_test.cpp deleted; the
PredicateColumnHolderType<T> transition alias and all
core/column/predicate_column.h includes are removed.
On the _base_loop_vec signature: upstream used
`const TArray* __restrict data_array` which worked because
PredicateColumnType<TYPE_STRING> physically held a contiguous StringRef[].
After this change the string side is ColumnElementView<TYPE_STRING> (a
struct value), and `__restrict` is a pointer-only qualifier — so we pass
TArray by value without it. Verified on a Release build (objdump) that
this does NOT regress SIMD: vectorizable types (INT / BIGINT /
dict-encoded string) still emit fully-vectorized loops (vpcmpeqd /
vpcmpeqq, 4× unrolled, 16 elements/iter). The compiler's loop versioning
emits one runtime alias check at function entry (~5 cycles, <0.5% of a
1024-row batch's total cost); the main loop body is identical to the
__restrict version. Non-vectorizable types (LARGEINT / DOUBLE / DECIMAL /
ColumnString memcmp) were scalar regardless of __restrict. See the
comment above _base_loop_vec for full rationale.
Storage compiles clean in both Release and ASAN trees. Targeted UTs:
319 tests across 13 suites (Column* / Predicate* / Segment* /
BloomFilterFunc / BitmapFilterPredicate /
ColumnExecuteUtil): PASS, 0 FAIL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>csun5285 added a commit
to csun5285/doris
that referenced
this pull request
Jun 4, 2026
PredicateColumnType<T> was a storage-layer wrapper that flattened every
predicate column into PaddedPODArray<value_type> so the SIMD-friendly
predicate-eval loops could use a uniform `data_array[i]` access regardless
of the underlying type. The cost was: a parallel column hierarchy with
restricted API, an extra 16 bytes/row (StringRef header) for strings on
top of the actual chars, and the need for callers to thread the column's
"role" (predicate vs output) down through the read path.
This change retires PredicateColumnType entirely:
- Schema::get_predicate_column_ptr now allocates the canonical
PrimitiveTypeTraits<T>::ColumnType (ColumnVector / ColumnDecimal /
ColumnString / ColumnIPv4 / ...). ColumnDictI32 is unchanged: it
still serves the low-cardinality string fast path for predicate eval.
- filter_by_selector is now implemented on ColumnVector<T> and
ColumnDecimal<T> with the same selector-gather semantics
PredicateColumnType used.
- ColumnString does not expose a contiguous StringRef[] array, so per-row
access in predicate evaluators flows through ColumnElementView<Type>
(already in core/column/column_execute_util.h for the compute layer)
rather than a pointer subscript:
ColumnElementView<Type> view {column};
_base_loop_vec<...>(size, flags, null_map, view, _value);
ColumnElementView numeric specialization yields T via pointer
arithmetic; the string specialization yields StringRef via
get_data_at — same `view[i]` call shape, no if-constexpr at the
call site. The compute-layer ColumnElementView gains size() /
operator[] aliases and its TYPE_STRING specialization is generalized
to all is_string_type(PType) via a defaulted bool template param so
TYPE_CHAR / TYPE_VARCHAR / TYPE_JSONB all resolve correctly.
- For HybridSet::find paths that need const T* per row, a small
ColumnPointerCursor<Type> lives next to ColumnElementView. Numeric
specialization holds a const T* (zero copy); string specialization
stages each row into a member StringRef and returns its address
(HybridSet::find consumes synchronously, so the staged-cell reuse
is safe).
- BloomFilter / BitmapFilter find_fixed_len_olap_engine API is
redesigned to take `const IColumn&` instead of a `const char*` that
was reinterpreted as `const T[]`. CommonFindOp / StringFindOp each
specialize per-row access:
CommonFindOp: reads column.get_raw_data() as `const T*`, passes
`[data](int i){return data[i];}` as the accessor.
StringFindOp: `assert_cast<const ColumnString&>(column)`, passes
`[&col](int i){return col.get_data_at(i);}`.
Storage-side BF/Bitmap callers collapse to one line and the previous
workaround (materializing a temporary `vector<StringRef>` to satisfy
the legacy char* API) goes away — saves one jemalloc + N×16-byte
store/load per evaluate on string columns.
- Schema-template CHAR predicate columns previously got trailing-zero
padding stripped on every PredicateColumnType<TYPE_CHAR>::get_data_at
call. ColumnString no longer does that; the strip is handled by
Block::shrink_char_type_column_suffix_zero on the output side (and
page-decoder-level for the read path per upstream PR apache#63291), so no
extra pass is needed here.
- ColumnDictI32::convert_to_predicate_column_if_dictionary now produces
ColumnString for mid-batch dict->plain fallback.
- predicate_column.h / predicate_column_test.cpp deleted; the
PredicateColumnHolderType<T> transition alias and all
core/column/predicate_column.h includes are removed.
On the _base_loop_vec signature: upstream used
`const TArray* __restrict data_array` which worked because
PredicateColumnType<TYPE_STRING> physically held a contiguous StringRef[].
After this change the string side is ColumnElementView<TYPE_STRING> (a
struct value), and `__restrict` is a pointer-only qualifier — so we pass
TArray by value without it. Verified on a Release build (objdump) that
this does NOT regress SIMD: vectorizable types (INT / BIGINT /
dict-encoded string) still emit fully-vectorized loops (vpcmpeqd /
vpcmpeqq, 4× unrolled, 16 elements/iter). The compiler's loop versioning
emits one runtime alias check at function entry (~5 cycles, <0.5% of a
1024-row batch's total cost); the main loop body is identical to the
__restrict version. Non-vectorizable types (LARGEINT / DOUBLE / DECIMAL /
ColumnString memcmp) were scalar regardless of __restrict. See the
comment above _base_loop_vec for full rationale.
Storage compiles clean in both Release and ASAN trees. Targeted UTs:
319 tests across 13 suites (Column* / Predicate* / Segment* /
BloomFilterFunc / BitmapFilterPredicate /
ColumnExecuteUtil): PASS, 0 FAIL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>csun5285 added a commit
to csun5285/doris
that referenced
this pull request
Jun 5, 2026
PredicateColumnType<T> was a storage-layer wrapper that flattened every
predicate column into PaddedPODArray<value_type> so the SIMD-friendly
predicate-eval loops could use a uniform `data_array[i]` access regardless
of the underlying type. The cost was: a parallel column hierarchy with
restricted API, an extra 16 bytes/row (StringRef header) for strings on
top of the actual chars, and the need for callers to thread the column's
"role" (predicate vs output) down through the read path.
This change retires PredicateColumnType entirely:
- Schema::get_predicate_column_ptr now allocates the canonical
PrimitiveTypeTraits<T>::ColumnType (ColumnVector / ColumnDecimal /
ColumnString / ColumnIPv4 / ...). ColumnDictI32 is unchanged: it
still serves the low-cardinality string fast path for predicate eval.
- filter_by_selector is now implemented on ColumnVector<T> and
ColumnDecimal<T> with the same selector-gather semantics
PredicateColumnType used.
- ColumnString does not expose a contiguous StringRef[] array, so per-row
access in predicate evaluators flows through ColumnElementView<Type>
(already in core/column/column_execute_util.h for the compute layer)
rather than a pointer subscript:
ColumnElementView<Type> view {column};
_base_loop_vec<...>(size, flags, null_map, view, _value);
ColumnElementView numeric specialization yields T via pointer
arithmetic; the string specialization yields StringRef via
get_data_at — same `view[i]` call shape, no if-constexpr at the
call site. The compute-layer ColumnElementView gains size() /
operator[] aliases and its TYPE_STRING specialization is generalized
to all is_string_type(PType) via a defaulted bool template param so
TYPE_CHAR / TYPE_VARCHAR / TYPE_JSONB all resolve correctly.
- For HybridSet::find paths that need const T* per row, a small
ColumnPointerCursor<Type> lives next to ColumnElementView. Numeric
specialization holds a const T* (zero copy); string specialization
stages each row into a member StringRef and returns its address
(HybridSet::find consumes synchronously, so the staged-cell reuse
is safe).
- BloomFilter / BitmapFilter find_fixed_len_olap_engine API is
redesigned to take `const IColumn&` instead of a `const char*` that
was reinterpreted as `const T[]`. CommonFindOp / StringFindOp each
specialize per-row access:
CommonFindOp: reads column.get_raw_data() as `const T*`, passes
`[data](int i){return data[i];}` as the accessor.
StringFindOp: `assert_cast<const ColumnString&>(column)`, passes
`[&col](int i){return col.get_data_at(i);}`.
Storage-side BF/Bitmap callers collapse to one line and the previous
workaround (materializing a temporary `vector<StringRef>` to satisfy
the legacy char* API) goes away — saves one jemalloc + N×16-byte
store/load per evaluate on string columns.
- Schema-template CHAR predicate columns previously got trailing-zero
padding stripped on every PredicateColumnType<TYPE_CHAR>::get_data_at
call. ColumnString no longer does that; the strip is handled by
Block::shrink_char_type_column_suffix_zero on the output side (and
page-decoder-level for the read path per upstream PR apache#63291), so no
extra pass is needed here.
- ColumnDictI32::convert_to_predicate_column_if_dictionary now produces
ColumnString for mid-batch dict->plain fallback.
- predicate_column.h / predicate_column_test.cpp deleted; the
PredicateColumnHolderType<T> transition alias and all
core/column/predicate_column.h includes are removed.
On the _base_loop_vec signature: upstream used
`const TArray* __restrict data_array` which worked because
PredicateColumnType<TYPE_STRING> physically held a contiguous StringRef[].
After this change the string side is ColumnElementView<TYPE_STRING> (a
struct value), and `__restrict` is a pointer-only qualifier — so we pass
TArray by value without it. Verified on a Release build (objdump) that
this does NOT regress SIMD: vectorizable types (INT / BIGINT /
dict-encoded string) still emit fully-vectorized loops (vpcmpeqd /
vpcmpeqq, 4× unrolled, 16 elements/iter). The compiler's loop versioning
emits one runtime alias check at function entry (~5 cycles, <0.5% of a
1024-row batch's total cost); the main loop body is identical to the
__restrict version. Non-vectorizable types (LARGEINT / DOUBLE / DECIMAL /
ColumnString memcmp) were scalar regardless of __restrict. See the
comment above _base_loop_vec for full rationale.
Storage compiles clean in both Release and ASAN trees. Targeted UTs:
319 tests across 13 suites (Column* / Predicate* / Segment* /
BloomFilterFunc / BitmapFilterPredicate /
ColumnExecuteUtil): PASS, 0 FAIL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>csun5285 added a commit
to csun5285/doris
that referenced
this pull request
Jun 5, 2026
PredicateColumnType<T> was a storage-layer wrapper that flattened every
predicate column into PaddedPODArray<value_type> so the SIMD-friendly
predicate-eval loops could use a uniform `data_array[i]` access regardless
of the underlying type. The cost was: a parallel column hierarchy with
restricted API, an extra 16 bytes/row (StringRef header) for strings on
top of the actual chars, and the need for callers to thread the column's
"role" (predicate vs output) down through the read path.
This change retires PredicateColumnType entirely:
- Schema::get_predicate_column_ptr now allocates the canonical
PrimitiveTypeTraits<T>::ColumnType (ColumnVector / ColumnDecimal /
ColumnString / ColumnIPv4 / ...). ColumnDictI32 is unchanged: it
still serves the low-cardinality string fast path for predicate eval.
- filter_by_selector is now implemented on ColumnVector<T> and
ColumnDecimal<T> with the same selector-gather semantics
PredicateColumnType used.
- ColumnString does not expose a contiguous StringRef[] array, so per-row
access in predicate evaluators flows through ColumnElementView<Type>
(already in core/column/column_execute_util.h for the compute layer)
rather than a pointer subscript:
ColumnElementView<Type> view {column};
_base_loop_vec<...>(size, flags, null_map, view, _value);
ColumnElementView numeric specialization yields T via pointer
arithmetic; the string specialization yields StringRef via
get_data_at — same `view[i]` call shape, no if-constexpr at the
call site. The compute-layer ColumnElementView gains size() /
operator[] aliases and its TYPE_STRING specialization is generalized
to all is_string_type(PType) via a defaulted bool template param so
TYPE_CHAR / TYPE_VARCHAR / TYPE_JSONB all resolve correctly.
- For HybridSet::find paths that need const T* per row, a small
ColumnPointerCursor<Type> lives next to ColumnElementView. Numeric
specialization holds a const T* (zero copy); string specialization
stages each row into a member StringRef and returns its address
(HybridSet::find consumes synchronously, so the staged-cell reuse
is safe).
- BloomFilter / BitmapFilter find_fixed_len_olap_engine API is
redesigned to take `const IColumn&` instead of a `const char*` that
was reinterpreted as `const T[]`. CommonFindOp / StringFindOp each
specialize per-row access:
CommonFindOp: reads column.get_raw_data() as `const T*`, passes
`[data](int i){return data[i];}` as the accessor.
StringFindOp: `assert_cast<const ColumnString&>(column)`, passes
`[&col](int i){return col.get_data_at(i);}`.
Storage-side BF/Bitmap callers collapse to one line and the previous
workaround (materializing a temporary `vector<StringRef>` to satisfy
the legacy char* API) goes away — saves one jemalloc + N×16-byte
store/load per evaluate on string columns.
- Schema-template CHAR predicate columns previously got trailing-zero
padding stripped on every PredicateColumnType<TYPE_CHAR>::get_data_at
call. ColumnString no longer does that; the strip is handled by
Block::shrink_char_type_column_suffix_zero on the output side (and
page-decoder-level for the read path per upstream PR apache#63291), so no
extra pass is needed here.
- ColumnDictI32::convert_to_predicate_column_if_dictionary now produces
ColumnString for mid-batch dict->plain fallback.
- predicate_column.h / predicate_column_test.cpp deleted; the
PredicateColumnHolderType<T> transition alias and all
core/column/predicate_column.h includes are removed.
On the _base_loop_vec signature: upstream used
`const TArray* __restrict data_array` which worked because
PredicateColumnType<TYPE_STRING> physically held a contiguous StringRef[].
After this change the string side is ColumnElementView<TYPE_STRING> (a
struct value), and `__restrict` is a pointer-only qualifier — so we pass
TArray by value without it. Verified on a Release build (objdump) that
this does NOT regress SIMD: vectorizable types (INT / BIGINT /
dict-encoded string) still emit fully-vectorized loops (vpcmpeqd /
vpcmpeqq, 4× unrolled, 16 elements/iter). The compiler's loop versioning
emits one runtime alias check at function entry (~5 cycles, <0.5% of a
1024-row batch's total cost); the main loop body is identical to the
__restrict version. Non-vectorizable types (LARGEINT / DOUBLE / DECIMAL /
ColumnString memcmp) were scalar regardless of __restrict. See the
comment above _base_loop_vec for full rationale.
Storage compiles clean in both Release and ASAN trees. Targeted UTs:
319 tests across 13 suites (Column* / Predicate* / Segment* /
BloomFilterFunc / BitmapFilterPredicate /
ColumnExecuteUtil): PASS, 0 FAIL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>csun5285 added a commit
to csun5285/doris
that referenced
this pull request
Jun 6, 2026
PredicateColumnType<T> was a storage-layer wrapper that flattened every
predicate column into PaddedPODArray<value_type> so the SIMD-friendly
predicate-eval loops could use a uniform `data_array[i]` access regardless
of the underlying type. The cost was: a parallel column hierarchy with
restricted API, an extra 16 bytes/row (StringRef header) for strings on
top of the actual chars, and the need for callers to thread the column's
"role" (predicate vs output) down through the read path.
This change retires PredicateColumnType entirely:
- Schema::get_predicate_column_ptr now allocates the canonical
PrimitiveTypeTraits<T>::ColumnType (ColumnVector / ColumnDecimal /
ColumnString / ColumnIPv4 / ...). ColumnDictI32 is unchanged: it
still serves the low-cardinality string fast path for predicate eval.
- filter_by_selector is now implemented on ColumnVector<T> and
ColumnDecimal<T> with the same selector-gather semantics
PredicateColumnType used.
- ColumnString does not expose a contiguous StringRef[] array, so per-row
access in predicate evaluators flows through ColumnElementView<Type>
(already in core/column/column_execute_util.h for the compute layer)
rather than a pointer subscript:
ColumnElementView<Type> view {column};
_base_loop_vec<...>(size, flags, null_map, view, _value);
ColumnElementView numeric specialization yields T via pointer
arithmetic; the string specialization yields StringRef via
get_data_at — same `view[i]` call shape, no if-constexpr at the
call site. The compute-layer ColumnElementView gains size() /
operator[] aliases and its TYPE_STRING specialization is generalized
to all is_string_type(PType) via a defaulted bool template param so
TYPE_CHAR / TYPE_VARCHAR / TYPE_JSONB all resolve correctly.
- For HybridSet::find paths that need const T* per row, a small
ColumnPointerCursor<Type> lives next to ColumnElementView. Numeric
specialization holds a const T* (zero copy); string specialization
stages each row into a member StringRef and returns its address
(HybridSet::find consumes synchronously, so the staged-cell reuse
is safe).
- BloomFilter / BitmapFilter find_fixed_len_olap_engine API is
redesigned to take `const IColumn&` instead of a `const char*` that
was reinterpreted as `const T[]`. CommonFindOp / StringFindOp each
specialize per-row access:
CommonFindOp: reads column.get_raw_data() as `const T*`, passes
`[data](int i){return data[i];}` as the accessor.
StringFindOp: `assert_cast<const ColumnString&>(column)`, passes
`[&col](int i){return col.get_data_at(i);}`.
Storage-side BF/Bitmap callers collapse to one line and the previous
workaround (materializing a temporary `vector<StringRef>` to satisfy
the legacy char* API) goes away — saves one jemalloc + N×16-byte
store/load per evaluate on string columns.
- Schema-template CHAR predicate columns previously got trailing-zero
padding stripped on every PredicateColumnType<TYPE_CHAR>::get_data_at
call. ColumnString no longer does that; the strip is handled by
Block::shrink_char_type_column_suffix_zero on the output side (and
page-decoder-level for the read path per upstream PR apache#63291), so no
extra pass is needed here.
- ColumnDictI32::convert_to_predicate_column_if_dictionary now produces
ColumnString for mid-batch dict->plain fallback.
- predicate_column.h / predicate_column_test.cpp deleted; the
PredicateColumnHolderType<T> transition alias and all
core/column/predicate_column.h includes are removed.
On the _base_loop_vec signature: upstream used
`const TArray* __restrict data_array` which worked because
PredicateColumnType<TYPE_STRING> physically held a contiguous StringRef[].
After this change the string side is ColumnElementView<TYPE_STRING> (a
struct value), and `__restrict` is a pointer-only qualifier — so we pass
TArray by value without it. Verified on a Release build (objdump) that
this does NOT regress SIMD: vectorizable types (INT / BIGINT /
dict-encoded string) still emit fully-vectorized loops (vpcmpeqd /
vpcmpeqq, 4× unrolled, 16 elements/iter). The compiler's loop versioning
emits one runtime alias check at function entry (~5 cycles, <0.5% of a
1024-row batch's total cost); the main loop body is identical to the
__restrict version. Non-vectorizable types (LARGEINT / DOUBLE / DECIMAL /
ColumnString memcmp) were scalar regardless of __restrict. See the
comment above _base_loop_vec for full rationale.
Storage compiles clean in both Release and ASAN trees. Targeted UTs:
319 tests across 13 suites (Column* / Predicate* / Segment* /
BloomFilterFunc / BitmapFilterPredicate /
ColumnExecuteUtil): PASS, 0 FAIL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>csun5285 added a commit
to csun5285/doris
that referenced
this pull request
Jun 12, 2026
PredicateColumnType<T> was a storage-layer wrapper that flattened every
predicate column into PaddedPODArray<value_type> so the SIMD-friendly
predicate-eval loops could use a uniform `data_array[i]` access regardless
of the underlying type. The cost was: a parallel column hierarchy with
restricted API, an extra 16 bytes/row (StringRef header) for strings on
top of the actual chars, and the need for callers to thread the column's
"role" (predicate vs output) down through the read path.
This change retires PredicateColumnType entirely:
- Schema::get_predicate_column_ptr now allocates the canonical
PrimitiveTypeTraits<T>::ColumnType (ColumnVector / ColumnDecimal /
ColumnString / ColumnIPv4 / ...). ColumnDictI32 is unchanged: it
still serves the low-cardinality string fast path for predicate eval.
- filter_by_selector is now implemented on ColumnVector<T> and
ColumnDecimal<T> with the same selector-gather semantics
PredicateColumnType used.
- ColumnString does not expose a contiguous StringRef[] array, so per-row
access in predicate evaluators flows through ColumnElementView<Type>
(already in core/column/column_execute_util.h for the compute layer)
rather than a pointer subscript:
ColumnElementView<Type> view {column};
_base_loop_vec<...>(size, flags, null_map, view, _value);
ColumnElementView numeric specialization yields T via pointer
arithmetic; the string specialization yields StringRef via
get_data_at — same `view[i]` call shape, no if-constexpr at the
call site. The compute-layer ColumnElementView gains size() /
operator[] aliases and its TYPE_STRING specialization is generalized
to all is_string_type(PType) via a defaulted bool template param so
TYPE_CHAR / TYPE_VARCHAR / TYPE_JSONB all resolve correctly.
- For HybridSet::find paths that need const T* per row, a small
ColumnPointerCursor<Type> lives next to ColumnElementView. Numeric
specialization holds a const T* (zero copy); string specialization
stages each row into a member StringRef and returns its address
(HybridSet::find consumes synchronously, so the staged-cell reuse
is safe).
- BloomFilter / BitmapFilter find_fixed_len_olap_engine API is
redesigned to take `const IColumn&` instead of a `const char*` that
was reinterpreted as `const T[]`. CommonFindOp / StringFindOp each
specialize per-row access:
CommonFindOp: reads column.get_raw_data() as `const T*`, passes
`[data](int i){return data[i];}` as the accessor.
StringFindOp: `assert_cast<const ColumnString&>(column)`, passes
`[&col](int i){return col.get_data_at(i);}`.
Storage-side BF/Bitmap callers collapse to one line and the previous
workaround (materializing a temporary `vector<StringRef>` to satisfy
the legacy char* API) goes away — saves one jemalloc + N×16-byte
store/load per evaluate on string columns.
- Schema-template CHAR predicate columns previously got trailing-zero
padding stripped on every PredicateColumnType<TYPE_CHAR>::get_data_at
call. ColumnString no longer does that; the strip is handled by
Block::shrink_char_type_column_suffix_zero on the output side (and
page-decoder-level for the read path per upstream PR apache#63291), so no
extra pass is needed here.
- ColumnDictI32::convert_to_predicate_column_if_dictionary now produces
ColumnString for mid-batch dict->plain fallback.
- predicate_column.h / predicate_column_test.cpp deleted; the
PredicateColumnHolderType<T> transition alias and all
core/column/predicate_column.h includes are removed.
On the _base_loop_vec signature: upstream used
`const TArray* __restrict data_array` which worked because
PredicateColumnType<TYPE_STRING> physically held a contiguous StringRef[].
After this change the string side is ColumnElementView<TYPE_STRING> (a
struct value), and `__restrict` is a pointer-only qualifier — so we pass
TArray by value without it. Verified on a Release build (objdump) that
this does NOT regress SIMD: vectorizable types (INT / BIGINT /
dict-encoded string) still emit fully-vectorized loops (vpcmpeqd /
vpcmpeqq, 4× unrolled, 16 elements/iter). The compiler's loop versioning
emits one runtime alias check at function entry (~5 cycles, <0.5% of a
1024-row batch's total cost); the main loop body is identical to the
__restrict version. Non-vectorizable types (LARGEINT / DOUBLE / DECIMAL /
ColumnString memcmp) were scalar regardless of __restrict. See the
comment above _base_loop_vec for full rationale.
Storage compiles clean in both Release and ASAN trees. Targeted UTs:
319 tests across 13 suites (Column* / Predicate* / Segment* /
BloomFilterFunc / BitmapFilterPredicate /
ColumnExecuteUtil): PASS, 0 FAIL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>csun5285 added a commit
to csun5285/doris
that referenced
this pull request
Jun 15, 2026
PredicateColumnType<T> was a storage-layer wrapper that flattened every
predicate column into PaddedPODArray<value_type> so the SIMD-friendly
predicate-eval loops could use a uniform `data_array[i]` access regardless
of the underlying type. The cost was: a parallel column hierarchy with
restricted API, an extra 16 bytes/row (StringRef header) for strings on
top of the actual chars, and the need for callers to thread the column's
"role" (predicate vs output) down through the read path.
This change retires PredicateColumnType entirely:
- Schema::get_predicate_column_ptr now allocates the canonical
PrimitiveTypeTraits<T>::ColumnType (ColumnVector / ColumnDecimal /
ColumnString / ColumnIPv4 / ...). ColumnDictI32 is unchanged: it
still serves the low-cardinality string fast path for predicate eval.
- filter_by_selector is now implemented on ColumnVector<T> and
ColumnDecimal<T> with the same selector-gather semantics
PredicateColumnType used.
- ColumnString does not expose a contiguous StringRef[] array, so per-row
access in predicate evaluators flows through ColumnElementView<Type>
(already in core/column/column_execute_util.h for the compute layer)
rather than a pointer subscript:
ColumnElementView<Type> view {column};
_base_loop_vec<...>(size, flags, null_map, view, _value);
ColumnElementView numeric specialization yields T via pointer
arithmetic; the string specialization yields StringRef via
get_data_at — same `view[i]` call shape, no if-constexpr at the
call site. The compute-layer ColumnElementView gains size() /
operator[] aliases and its TYPE_STRING specialization is generalized
to all is_string_type(PType) via a defaulted bool template param so
TYPE_CHAR / TYPE_VARCHAR / TYPE_JSONB all resolve correctly.
- For HybridSet::find paths that need const T* per row, a small
ColumnPointerCursor<Type> lives next to ColumnElementView. Numeric
specialization holds a const T* (zero copy); string specialization
stages each row into a member StringRef and returns its address
(HybridSet::find consumes synchronously, so the staged-cell reuse
is safe).
- BloomFilter / BitmapFilter find_fixed_len_olap_engine API is
redesigned to take `const IColumn&` instead of a `const char*` that
was reinterpreted as `const T[]`. CommonFindOp / StringFindOp each
specialize per-row access:
CommonFindOp: reads column.get_raw_data() as `const T*`, passes
`[data](int i){return data[i];}` as the accessor.
StringFindOp: `assert_cast<const ColumnString&>(column)`, passes
`[&col](int i){return col.get_data_at(i);}`.
Storage-side BF/Bitmap callers collapse to one line and the previous
workaround (materializing a temporary `vector<StringRef>` to satisfy
the legacy char* API) goes away — saves one jemalloc + N×16-byte
store/load per evaluate on string columns.
- Schema-template CHAR predicate columns previously got trailing-zero
padding stripped on every PredicateColumnType<TYPE_CHAR>::get_data_at
call. ColumnString no longer does that; the strip is handled by
Block::shrink_char_type_column_suffix_zero on the output side (and
page-decoder-level for the read path per upstream PR apache#63291), so no
extra pass is needed here.
- ColumnDictI32::convert_to_predicate_column_if_dictionary now produces
ColumnString for mid-batch dict->plain fallback.
- predicate_column.h / predicate_column_test.cpp deleted; the
PredicateColumnHolderType<T> transition alias and all
core/column/predicate_column.h includes are removed.
On the _base_loop_vec signature: upstream used
`const TArray* __restrict data_array` which worked because
PredicateColumnType<TYPE_STRING> physically held a contiguous StringRef[].
After this change the string side is ColumnElementView<TYPE_STRING> (a
struct value), and `__restrict` is a pointer-only qualifier — so we pass
TArray by value without it. Verified on a Release build (objdump) that
this does NOT regress SIMD: vectorizable types (INT / BIGINT /
dict-encoded string) still emit fully-vectorized loops (vpcmpeqd /
vpcmpeqq, 4× unrolled, 16 elements/iter). The compiler's loop versioning
emits one runtime alias check at function entry (~5 cycles, <0.5% of a
1024-row batch's total cost); the main loop body is identical to the
__restrict version. Non-vectorizable types (LARGEINT / DOUBLE / DECIMAL /
ColumnString memcmp) were scalar regardless of __restrict. See the
comment above _base_loop_vec for full rationale.
Storage compiles clean in both Release and ASAN trees. Targeted UTs:
319 tests across 13 suites (Column* / Predicate* / Segment* /
BloomFilterFunc / BitmapFilterPredicate /
ColumnExecuteUtil): PASS, 0 FAIL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>csun5285 added a commit
to csun5285/doris
that referenced
this pull request
Jun 16, 2026
- apache/doris-website#3759 - Problem: The CHAR padding contract leaked from the storage layer into the compute / predicate layers — every scan stripped padding at the Block level, while predicates re-padded values to match the on-disk shape. Logic was spread out and wasted work on every read. - Fix: On-disk format unchanged. The convertor still pads CHAR to the schema length on write, but the strip is pushed down to the page pre-decoder — the page cache holds unpadded data. All shrink_* / pad_* code above the page cache (SegmentIterator, Block, RowCursor, predicates) is removed. - BloomFilter: BF probing is skipped (return true, fall back to scan) for CHAR predicates — the BF hashes padded bytes but predicate values are unpadded, so the probe would never match. Other indexes (ZoneMap / inverted / bitmap) are unaffected. Issue Number: close #xxx Related PR: #xxx Problem Summary: None - Test <!-- At least one of them must be included. --> - [x] Regression test - [x] 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: apache/doris-website#3759> - [ ] 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 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit e072997)
yiguolei pushed a commit
that referenced
this pull request
Jun 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[doc] note CHAR BloomFilter no longer takes effect since 4.1.2 doris-website#3759
Problem: The CHAR padding contract leaked from the storage layer into the
compute / predicate layers — every scan stripped padding at the Block level,
while predicates re-padded values to match the on-disk shape. Logic was spread
out and wasted work on every read.
length on write, but the strip is pushed down to the page pre-decoder — the
page cache holds unpadded data. All shrink_* / pad_* code above the page cache
(SegmentIterator, Block, RowCursor, predicates) is removed.
predicates — the BF hashes padded bytes but predicate values are unpadded, so
the probe would never match. Other indexes (ZoneMap / inverted / bitmap) are
unaffected.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)