github-actions[bot] commented on code in PR #65548:
URL: https://github.com/apache/doris/pull/65548#discussion_r3588662469
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/FileScanNode.java:
##########
@@ -103,6 +105,21 @@ protected void setPushDownCount(long count) {
tableLevelRowCount = count;
}
+ /**
+ * Return whether FE may replace real table-format splits with metadata
COUNT splits.
+ *
+ * <p>The aggregate opcode alone is insufficient because both {@code
COUNT(*)} and
+ * {@code COUNT(col)} use {@link TPushAggOp#COUNT}. The semantic argument
list distinguishes
+ * them: it is empty only for {@code COUNT(*)}/{@code COUNT(1)}. For
example, if an Iceberg
+ * table has 100 data files, retaining one representative split is correct
for a snapshot
+ * {@code COUNT(*)}. Doing that for {@code COUNT(required_col)} is unsafe:
BE deliberately
+ * falls back to reading the column, but it would then see only the
representative file and
+ * undercount the table.
+ */
+ protected boolean isTableLevelCountStarPushdown() {
+ return pushDownAggNoGroupingOp == TPushAggOp.COUNT &&
pushDownCountSlotIds.isEmpty();
+ }
Review Comment:
This semantic COUNT(*) helper also needs to cover the other FE count-split
shortcuts. Hive and TVF still check only `getPushDownAggNoGroupingOp() ==
COUNT` before calling `FileSplitter.needSplitForCountPushdown()`. When that
returns false, they pass `splittable=false` to `splitFile()`, so a large
splittable file becomes one full-file range. That was fine when the scan really
used metadata only, but this PR makes non-empty COUNT arguments fall back to
real row scans for unsafe mappings, filters/deletes, or old/unknown semantics.
A Hive/TVF `COUNT(col)` fallback can therefore lose normal scan parallelism
even though the result is still correct. Please gate those no-split shortcuts
on the explicit COUNT(*)/COUNT(1) state, or keep normal splitting whenever
`pushDownCountSlotIds` is non-empty.
##########
be/src/format_v2/column_mapper.cpp:
##########
@@ -843,6 +872,192 @@ static bool rewrite_struct_element_path_to_file_expr(
return true;
}
+static VExprSPtr cast_file_expr_to_table_type(const VExprSPtr& file_expr,
+ const DataTypePtr& table_type,
+ RewriteContext* rewrite_context)
{
+ DORIS_CHECK(file_expr != nullptr);
+ DORIS_CHECK(table_type != nullptr);
+ DORIS_CHECK(rewrite_context != nullptr);
+ auto cast_expr = Cast::create_shared(table_type);
+ cast_expr->add_child(file_expr);
+ rewrite_context->add_created_expr(cast_expr);
+ return cast_expr;
+}
+
+// Prefer comparing in the physical file leaf type when a table predicate uses
a promoted struct
+// child. For example, with table STRUCT<a: BIGINT>, old-file STRUCT<a: INT>,
and `s.a = 10`, the
+// localized predicate should be `file_s.a::INT = 10::INT`, not
+// `CAST(file_s.a::INT AS BIGINT) = 10::BIGINT`. Converting one literal avoids
a cast for every row.
+//
+// This rewrite is valid only when every possible file value survives
file-to-table conversion and
+// the particular literal survives a table-to-file-to-table round trip. A
value such as BIGINT
+// 2147483648 cannot be represented by an INT file leaf, so that case
deliberately falls back to
+// `CAST(file_s.a AS BIGINT) = 2147483648`, which preserves the original
table-level semantics.
+static bool rewrite_binary_struct_literal_predicate(
+ const VExprSPtr& expr, const std::vector<ColumnMapping>&
filter_mappings,
+ const std::map<GlobalIndex, FileSlotRewriteInfo>& global_to_file_slot,
+ RewriteContext* rewrite_context) {
+ if (!is_binary_comparison_predicate(expr)) {
+ return false;
+ }
+ auto children = expr->children();
+ int struct_child_idx = -1;
+ int literal_child_idx = -1;
+ if (is_struct_element_expr(children[0])) {
+ struct_child_idx = 0;
+ literal_child_idx = 1;
+ } else if (is_struct_element_expr(children[1])) {
+ struct_child_idx = 1;
+ literal_child_idx = 0;
+ } else {
+ return false;
+ }
+
+ const auto table_leaf_type = children[struct_child_idx]->data_type();
+ DORIS_CHECK(table_leaf_type != nullptr);
+ auto table_literal =
unwrap_literal_for_file_cast(children[literal_child_idx], table_leaf_type);
+ if (table_literal == nullptr ||
+ !rewrite_struct_element_path_to_file_expr(children[struct_child_idx],
filter_mappings,
+ global_to_file_slot,
rewrite_context)) {
+ return false;
+ }
+
+ const auto file_leaf_type = children[struct_child_idx]->data_type();
+ DORIS_CHECK(file_leaf_type != nullptr);
+ const FileSlotRewriteInfo leaf_rewrite_info {
+ .block_position = 0,
+ .file_type = file_leaf_type,
+ .table_type = table_leaf_type,
+ .file_column_name = {},
+ };
+ auto file_literal =
+ rewrite_literal_to_file_type(table_literal, leaf_rewrite_info,
rewrite_context);
+ if (file_literal != nullptr) {
+ children[literal_child_idx] = std::move(file_literal);
+ } else {
+ children[struct_child_idx] =
cast_file_expr_to_table_type(children[struct_child_idx],
Review Comment:
This fallback still runs a non-lossless file-to-table cast below
`TableReader`, so it can hide rows that normal materialization would reject.
For example, take an old file `STRUCT<a: BIGINT NOT NULL>` mapped to a required
table `STRUCT<a: INT NOT NULL>` and filter `s.a = 1`. Because BIGINT->INT is
not lossless, `rewrite_literal_to_file_type()` returns null, but this branch
still localizes the predicate as `CAST(file_s.a AS INT) = 1`. In non-strict
mode an overflow value such as `2147483648` becomes NULL, the predicate drops
that row, and `_align_column_nullability()` never sees the cast-produced NULL
that the normal table materialization path would reject for the required INT
child. Please keep this predicate at table level whenever the file-to-table
leaf cast is not lossless, or otherwise prove the fallback cast cannot change
the NULL/error behavior before filtering.
--
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]
