github-actions[bot] commented on code in PR #65602:
URL: https://github.com/apache/doris/pull/65602#discussion_r3579112287
##########
be/test/format_v2/table_reader_test.cpp:
##########
@@ -2186,6 +2187,50 @@ TEST(TableReaderTest,
ConditionCacheSkipsRuntimeFilterConjunct) {
ASSERT_TRUE(reader.close().ok());
}
+// Scenario: FileScannerV2 supplies a non-zero digest computed from the exact
Ready RF payload in
+// this split. The RF wrapper is no longer a reason to disable condition
cache; a MISS context is
+// created and can be published under that payload-specific key after EOF.
+TEST(TableReaderTest, ConditionCacheAllowsRuntimeFilterCoveredBySplitDigest) {
+ ScopedConditionCacheForTest cache;
+ std::vector<ColumnDefinition> file_schema;
+ file_schema.push_back(make_file_column(0, "id",
std::make_shared<DataTypeInt32>()));
+
+ std::vector<ColumnDefinition> projected_columns;
+ projected_columns.push_back(make_table_column(0, "id",
std::make_shared<DataTypeInt32>()));
+ set_name_identifiers(&projected_columns);
+
+ RuntimeState state {TQueryOptions(), TQueryGlobals()};
+ auto fake_state = std::make_shared<FakeFileReaderState>();
+ fake_state->total_rows = ConditionCacheContext::GRANULE_SIZE;
+ FakeTableReader reader(file_schema, fake_state);
+ ASSERT_TRUE(
+ reader.init({
+ .projected_columns = projected_columns,
+ .conjuncts = {prepared_conjunct(
+ &state, runtime_filter_wrapper_expr(
+
table_int32_greater_than_expr(0, 0, 0)))},
+ .format = FileFormat::PARQUET,
+ .scan_params = nullptr,
+ .io_ctx = nullptr,
+ .runtime_state = &state,
+ .scanner_profile = nullptr,
+ .condition_cache_digest = 7,
+ })
+ .ok());
+
+ SplitReadOptions split_options;
+ split_options.current_range.__set_path("fake-table-reader-input");
+ split_options.condition_cache_digest = 11;
Review Comment:
This test enables condition cache for a split digest, but it does not prove
the split digest is the value used in the cache key. The reader is initialized
with digest `7`, the split sets digest `11`, and the only assertion is that a
miss context exists. If a regression kept `_condition_cache_digest` at `7`
while only setting `_condition_cache_digest_covers_current_split = true`, this
test would still pass even though `P AND RF` would be stored under the stale
key. Please make the split digest observable here, for example by
publishing/looking up the same fake file under digest `11` and proving digest
`7` does not hit.
##########
be/src/format_v2/parquet/parquet_scan.cpp:
##########
@@ -1395,6 +1414,18 @@ Status
ParquetScanScheduler::read_current_row_group_batch(
.column->filter(output_filter,
selected_rows)));
}
}
+ if (selected_rows == 0) {
Review Comment:
The new pending-skip path avoids calling `SkipRecords()` for lazy output
columns when `selected_rows == 0`, but it does not prevent merge-range from
fetching those output chunks earlier. `prepare_current_row_group_reader()`
still builds ranges from `request_scan_columns(request)`, which includes both
predicate and non-predicate columns, and installs them before predicates run.
When merge-range is active, `prefetch_current_row_group_columns()` is skipped,
so the survivor gate below does not control the active reader path; a
predicate-column `ReadAt()` can merge forward into adjacent output-column
ranges that this branch later discards without materializing. For filtered
scans, please install the merge ranges for predicate columns first and only add
the non-predicate ranges once a survivor exists, or disable merge-range for
lazy output chunks until then.
--
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]
