morningman opened a new pull request, #68280:
URL: https://github.com/apache/doris/pull/68280
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
A `CASE` whose condition contains `JSON_EXTRACT_BOOL(...) OR <other
predicate>` can crash the BE with `SIGSEGV` in
`VCaseExpr::update_result_normal` →
`ColumnNullable::insert_from_with_type<ColumnVector<BIGINT>>`. It was hit in
production on 4.1.4 through a view over a Hive parquet table (the `CASE` was
pushed into the file scan as a conjunct, so the stack ends in
`parquet_scan.cpp:execute_batch_filters`), but the defect is in the expression
layer and reproduces on an internal table with a single query.
Three pieces of code combine into the crash:
1. `DataTypeNumberSerDe<T>::deserialize_column_from_jsonb_vector` (the
vectorized `CAST(jsonb AS BOOLEAN)` that `JSON_EXTRACT_BOOL` is rewritten into)
sizes the nested payload with `data.resize(size)` and then skips rows that cast
to NULL (missing key, JSON `null`, non-boolean value). Those rows keep whatever
bytes the recycled allocation held, typically ASCII from a previous string
column. `DataTypeDecimalSerDe` has the same pattern.
2. `VCompoundPred` evaluates `OR` as `res_data = lhs | rhs` with `res_null =
!lhs` when the right side is not NULL. For a row whose CAST is NULL but whose
other predicate is TRUE the result is a non-NULL boolean carrying `stale_byte |
1` (3, 5, 65, ...). `AND` normally masks it with `&`, but the `lhs_all_true`
short-circuit returns the CAST column untouched, which is exactly what happens
when every row of a batch holds valid JSON.
3. `VCaseExpr::_execute_impl` computes the branch index as `then_idx |=
(!then_idx) * cond_raw_data[row] * column_idx`, i.e. it multiplies the raw
byte. A payload like 65 produces an index far beyond `then_columns.size()`,
`raw_then_columns[idx]` reads a garbage `ColumnPtr`, and
`insert_from_with_type` dereferences it.
The bug depends on what the recycled buffer happens to contain, so the same
query can pass and crash on different runs, and the boolean can also silently
render as `3`, `5`, `7`, ... instead of `1` when it is selected instead of used
in a `CASE`.
Fix:
- `deserialize_column_from_jsonb_vector` (number and decimal serde) now uses
`data.resize_fill(size)` so NULL rows carry a well-defined default payload of 0.
- `VCaseExpr::_execute_impl` now uses `(cond_raw_data[row] != 0)` when
deriving the branch index, so a non-canonical TRUE byte can never become an
out-of-range branch index, whatever produced it.
Reproduction (before this PR, the last statement kills the BE; the first
`SELECT` shows values other than NULL / 1):
```sql
CREATE TABLE json_case_crash (id BIGINT, j STRING, a BIGINT, b BIGINT)
DUPLICATE KEY(id) DISTRIBUTED BY HASH(id) BUCKETS 1 PROPERTIES
("replication_num" = "1");
-- every row is valid JSON, only 1% of the rows carry the key "flag"
INSERT INTO json_case_crash
SELECT number,
CASE WHEN number % 100 = 0 THEN '{"flag": true}' ELSE CONCAT('{"n":
', number, '}') END,
number + CASE number % 3 WHEN 0 THEN 10 WHEN 1 THEN -100 ELSE -200000
END,
number
FROM numbers("number" = "100000");
-- expected: only NULL and 1; actual before the fix: 3, 5, 7, 15, 31, 65, ...
SELECT v, count(*) FROM (
SELECT JSON_EXTRACT_BOOL(JSON_PARSE_ERROR_TO_NULL(j), '$.flag') OR (a > b)
AS v
FROM json_case_crash) t GROUP BY v ORDER BY v;
-- SIGSEGV in VCaseExpr::update_result_normal before the fix; 100000 after it
SELECT count(*) FROM json_case_crash
WHERE (CASE WHEN JSON_EXTRACT_BOOL(JSON_PARSE_ERROR_TO_NULL(j), '$.flag') OR
a > b THEN b
WHEN a < b THEN a END) IS NOT NULL;
```
Whether the wrong branch index crashes or just corrupts the result depends
on what the heap holds next to the small `then_columns` vectors, so run the
statements on a freshly started BE in the order above (load, probe, `CASE`); on
a BE that has already served other queries the `CASE` may return wrong rows
silently instead of crashing.
Verification on this branch (macOS arm64 build of `9fb32d2bbf2`, BE launched
under lldb, same steps on a fresh BE each time):
- without the fix: the probe returns `3, 7, 9, 13, 15, ... 121` next to
`NULL`/`1`; the `CASE` query kills the BE with `EXC_BAD_ACCESS (address=0x11)`
in `ColumnNullable::insert_from_with_type<ColumnVector<BIGINT>>` ←
`VCaseExpr::update_result_normal<unsigned char, ColumnVector<BIGINT>, true>` ←
`VCaseExpr::_execute_impl` ← `VectorizedFnCall::_do_execute` ←
`VExpr::execute_filter` ← `SegmentIterator::_execute_common_expr`. The original
view over a parquet file read through `FileScannerV2` dies with the same frames
below `execute_batch_filters` ← `read_filter_columns` ←
`read_current_row_group_batch` ← `ParquetReader::get_block` ←
`TableReader::get_block` ← `FileScannerV2::_get_block_impl`, i.e. the
production stack. The new regression suite fails on `or_payload` (`true 32883`
plus a set of corrupted boolean rows instead of `true 34000`).
- with the fix: the probe returns exactly `NULL 66000` / `1 34000`, the
`CASE` query returns `100000`, the view over parquet returns the same values on
every run, and the regression suite passes.
The original production stack (4.1.4):
```
4#
doris::ColumnNullable::insert_from_with_type<doris::ColumnVector<(doris::PrimitiveType)6>>(...)
at column_nullable.h:162
5# doris::VCaseExpr::update_result_normal<unsigned char,
doris::ColumnVector<(doris::PrimitiveType)6>, true>(...) at vcase_expr.h:199
6# doris::VCaseExpr::_execute_update_result<unsigned char>(...) at
vcase_expr.h:129
7# doris::VCaseExpr::_execute_impl<unsigned char>(...) at vcase_expr.h:325
8# doris::VCaseExpr::execute_column(...)
9# doris::VectorizedFnCall::_do_execute(...) at vectorized_fn_call.cpp:412
11# doris::VExpr::execute_filter(...) at vexpr.cpp:1113
13# doris::format::parquet::execute_batch_filters(...) at
parquet_scan.cpp:1118
14# doris::format::parquet::ParquetScanScheduler::read_filter_columns(...)
at parquet_scan.cpp:2705
```
### Release note
Fix a BE crash (`SIGSEGV` in `VCaseExpr::update_result_normal`) and
corrupted boolean values when `JSON_EXTRACT_BOOL` / `CAST(json AS BOOLEAN)`
results are combined with `OR` and used in a `CASE`.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
-
`query_p0/sql_functions/json_functions/test_json_extract_bool_null_payload`:
the `OR` payload probe and the multi-branch `CASE` that crashed the BE.
- [x] Unit Test
- `DataTypeSerDeJsonbNullPayloadTest.*`: the target buffer is
pre-filled with stale bytes, then the jsonb deserializer must leave a 0 payload
in every NULL row (boolean, bigint, decimal).
- `VCaseConditionBytesTest.*`: `_execute_impl` with condition bytes
such as `0x41`, `0xFE`, `0x80` selects the THEN branch instead of running past
`then_columns`, with and without a null map.
- [x] Manual test (add detailed scripts or steps below)
- The SQL above, plus the original view over a parquet table read
through FileScannerV2, run against BEs built from this branch (`9fb32d2bbf2`)
and from the 4.1.4 tag, each with and without this patch: crash / corrupted
booleans before, correct results after (details above).
- [ ] 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:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [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]
