[core] Execute complete filters with unprojected fields - #9872
JingsongLi merged 3 commits into
Conversation
leaves12138
left a comment
There was a problem hiding this comment.
Reviewed 5e17eb2ecda1679335b0d4ccdbc6866b977b0a82. Requesting changes for one authorization-boundary issue: the new filter-only columns are read without being included in the column authorization request. See the inline finding and reproduction.
Validation:
- 258 focused tests passed on JDK 8 with normal Maven checks enabled: predicate remapping, read transforms, FormatTable, append/PK filtering, Data Evolution indexes, authorization helpers, and all 204
MockRESTCatalogTestcases. Checkstyle and Spotless passed. The codegen loader was packaged before running the integration tests. - Added a REST integration regression granting access only to
public_id. Directly selectingsecret_scoreis rejected, but the PR allows filtering on it and returns the matching public ID. The fail-closed assertion fails on the reviewed head. - As a targeted old-reader control, replacing only
AbstractDataTableReadwith the base implementation returns all three public IDs rather than evaluating the unprojected secret predicate; the new complete-filter execution exposes the exact restricted-value match. - A local candidate that adds user-filter operands to the authorization request makes the new regression and the existing basic column-authorization test pass. This was validation in a temporary checkout only; no changes were pushed.
Separate CI follow-up: both Spark 3 Scala 2.12/2.13 jobs fail two PermissionProcedureTest assertions because the new remapper reports Cannot resolve field 'unknown' / Cannot resolve field 'region', while the tests still expect column unknown / column region (lines 545 and 667). These are stale message assertions, distinct from the authorization issue.
leaves12138
left a comment
There was a problem hiding this comment.
Re-reviewed f1e1fc93ab7ab34707d8f58958228b86dd7b03c2, including the changes since 5e17eb2.
The previous P1 is fixed. authQuery() now authorizes the union of output columns, user-filter operands, and explicit partition-filter operands before planning/pruning. The REST regressions reject access to the unprojected restricted column in both batch and streaming scans; granting access allows the same query while preserving the requested output projection.
The separation from trusted policy dependencies is also preserved: row-filter and cross-column masking inputs do not become caller-selected columns. Combined partition predicates collect their actual dependencies, while constant partition predicates introduce none.
Validation on JDK 8, with normal Maven checks enabled:
- 263 focused tests passed on the exact PR sources: predicate remapping, read transforms, FormatTable, append/PK filtering, Data Evolution indexes, authorization helpers, and all 209
MockRESTCatalogTestcases. Checkstyle and Spotless passed. - 8 additional temporary REST regressions passed, covering batch/streaming, ordinary/empty output projections, and AND/OR user predicates. Each case rejects an unauthorized predicate operand, then succeeds after granting it without exposing extra output columns. No source changes were pushed.
- The two stale Spark
PermissionProcedureTestmessage assertions have been updated to the new remapper messages. I verified those edits, but did not run the Spark suites locally; the latest Spark and several other CI jobs are still running at review time.
No further blocking findings. Approving the code changes; this does not claim that the full CI matrix has finished.
Purpose
TableRead.executeFilter()currently drops predicates that reference fields outside the requested read type. For example, reading onlyawitha = 1 AND b = 2evaluates onlya = 1, whilea = 1 OR b = 2can skip row filtering entirely.Read all filter operands internally, evaluate the complete predicate after query authorization and column masking, and restore the requested output projection, including nested fields and field order. Apply the same behavior to FormatTable, retain Data Evolution files needed by filter operands during planning, and document the
executeFilter()contract.A per-reader
ReadTransformshares dependency calculation with scan planning and applies authorization, masking, query filtering, and output projection in order. Query predicates execute once: the complete filter when requested, otherwise only the conjuncts involving masked columns. Strict field-name remapping for predicates and mask transforms lives inpaimon-common; authorization-specific validation remains separate.Authorize the union of output columns and user-query filter operands before planning, including predicates passed through
withPartitionFilter. Compound partition predicates collect only their referenced columns. Catalog-provided row-filter and masking dependencies retain their existing authorization semantics.Tests
240 tests passed on JDK 8 in a clean, isolated worktree with normal Maven checks enabled:
PredicateRemapperTestcases.AbstractDataTableReadTestcases.TableQueryAuthResultTestcases.MockRESTCatalogTestcases, including new batch/stream regressions for unauthorized ordinary and partition-filter operands, successful filtering after granting column access, combined partition predicates, and trusted masking/row-filter dependencies.PermissionProcedureTestcases on Spark 3 / Scala 2.12, including the two assertions that failed in all three Spark CI matrices after the remapper's exception text changed.Checkstyle, Spotless, and
git diff --checkpassed. Both ordinary and partition-filter authorization regressions failed before their respective fixes because the unauthorized query did not throw.The earlier focused regression run also covered FormatTable, append-only and primary-key reads, Data Evolution pruning, unprojected leaf/AND/OR operands, empty and nested projections, single evaluation after masking, independent readers under changing authorization, and overwritten Data Evolution columns.
Verification command: