Doris-Breakwater commented on issue #70503: URL: https://github.com/apache/doris/issues/70503#issuecomment-6818184029
Breakwater-GitHub-Analysis-Slot: slot_823a96ea80de ### Initial assessment This warrants high-priority correctness triage for Iceberg predicate pushdown: the supplied result and EXPLAIN are consistent with a source-verified loss of CAST semantics before file pruning. Both `15` and `150` satisfy the requested string range, but the generated numeric range excludes `15`. Retaining the original Doris predicate cannot recover rows from files that never receive scan tasks. No labels were present when this issue was read; suggested classification is **bug / wrong results / Iceberg / FE predicate pushdown**. ### Verified evidence **4.1.0:** I inspected the local `4.1.0` tag at `5960d4cea0e6d2a08f62b45a07a662395d2bbcfc`, without modifying the checkout. - [`IcebergUtils.convertDorisExprToSlotRef`](https://github.com/apache/doris/blob/5960d4cea0e6d2a08f62b45a07a662395d2bbcfc/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java#L522-L531) accepts a `CastExpr` over a `SlotRef` and returns the underlying slot without checking whether the cast preserves comparison semantics. - [`convertToIcebergExpr`](https://github.com/apache/doris/blob/5960d4cea0e6d2a08f62b45a07a662395d2bbcfc/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java#L249-L295) uses that slot's Iceberg schema type to convert the literal. The [`StringLiteral` / INTEGER branch](https://github.com/apache/doris/blob/5960d4cea0e6d2a08f62b45a07a662395d2bbcfc/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java#L494-L507) calls `Integer.parseInt`, so `'100'` becomes numeric `100`, and the comparison operator is retained. - [`IcebergScanNode`](https://github.com/apache/doris/blob/5960d4cea0e6d2a08f62b45a07a662395d2bbcfc/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java#L539-L585) passes these predicates to `scan.filter` before `planFiles`. Its [manifest-cache planning path](https://github.com/apache/doris/blob/5960d4cea0e6d2a08f62b45a07a662395d2bbcfc/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java#L629-L651) independently builds the same filter for `InclusiveMetricsEvaluator`; disabling that cache does not remove this conversion. **Reported master commit `e125ea858678fd2aea5ebf179359650aa77fabb6`:** the implementation has moved to the connector framework, but the same semantic hazard remains. [Iceberg advertises CAST pushdown support](https://github.com/apache/doris/blob/e125ea858678fd2aea5ebf179359650aa77fabb6/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java#L2498-L2500), allowing CAST-containing conjuncts through [`buildRemainingFilter`](https://github.com/apache/doris/blob/e125ea858678fd2aea5ebf179359650aa77fabb6/fe/fe-core/src/main/java/org/apache/doris/datasource/scan/PluginDrivenScanNode.java#L2964-L2988). The [Expr bridge strips the CAST](https://github.com/apache/doris/blob/e125ea858678fd2aea5ebf179359650aa77fabb6/fe/fe-core/src/main/java/org/apache/doris/datasource/connector/converter/ExprToConnectorExpressionConverter.java#L104-L109), the [Iceberg converter parses the string as INTEGER](https://github.com/apache/doris/blob/e125ea8 58678fd2aea5ebf179359650aa77fabb6/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergPredicateConverter.java#L682-L695), and the [scan provider applies the resulting filter](https://github.com/apache/doris/blob/e125ea858678fd2aea5ebf179359650aa77fabb6/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java#L1444-L1455). A fix confined to the old `IcebergUtils` class would therefore not cover this master revision. **Executed check:** using Iceberg 1.10.1, which the inspected 4.1.0 FE declares, I evaluated synthetic unpartitioned one-row data-file metadata with exact integer lower/upper bounds: | File value | String range matches | Numeric pushdown may match | |---|---|---| | 15 | true | false | | 150 | true | true | This independently verifies the pruning mechanism. It does **not** constitute a Doris SQL reproduction, execution of the Doris converter, or a physical Parquet scan; the SQL result and EXPLAIN remain reporter-provided evidence. ### Suggested next steps 1. Reject this unsafe CAST pushdown before cast/type information is discarded, keeping the original predicate in Doris. A conservative first fix can retain each whole CAST-containing conjunct locally. On master, evaluate the existing CAST capability gate and audit other filter entry points; preserve its source-LIMIT suppression when a filter remains local. Only re-enable individual cast conversions with a proven semantics-preserving translation. 2. Preserve safe independent AND conjunct pushdown. Do not drop an unsupported OR branch or negate a partially widened expression: either can produce another overly restrictive source filter. 3. Add converter/bridge tests plus the supplied end-to-end two-file regression. Assert returned rows `15, 150` and absence of the unsafe numeric pushdown. Cover both SDK and manifest-cache planning, OR/NOT nesting, NULLs, and CAST predicates combined with LIMIT. Keep a plain numeric comparison as a control; it should still push down and return only `150` for this numeric range. 4. Assess release-branch backports after the regression passes. The inspected sources establish affected mechanisms at the two revisions above, not the status of every intervening or later release. ### Remaining information The report is sufficient to start a fix; a Profile or broad log collection is not needed to establish this conversion defect. To make the SQL regression independently reproducible, please attach the exact FE/BE build identifiers (`SELECT VERSION()` and build hashes), full EXPLAIN VERBOSE for that build, and the Iceberg snapshot/file metadata showing the two files and `id` lower/upper bounds. Separate INSERTs should be checked against actual file metadata rather than assumed to guarantee the intended layout. An unfiltered read of the same snapshot should show both rows. Include Iceberg format/file format and relevant catalog/session settings if the minimal case behaves differently. Public related history: [#55804](https://github.com/apache/doris/issues/55804) describes a different CAST direction with the same loss-of-semantics concern. [#55956](https://github.com/apache/doris/pull/55956) proposed rejecting CAST pushdown, but is closed **without merge**; it is a reference, not evidence of an available fix. -- 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]
