starocean999 opened a new pull request, #67776:
URL: https://github.com/apache/doris/pull/67776
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Reproduction:
A normal user has a restrictive row policy `USING(k = 1)` on table `t1`.
Reading the table directly and joining it without a hint only returns the
allowed row, but adding a leading hint leaks the protected row:
-- returns only k = 1
SELECT t1.k, t1.v, t2.v FROM t1 JOIN t2 ON t1.k = t2.k ORDER BY t1.k;
-- returns k = 1 and k = 2, the row policy is bypassed
SELECT /*+ leading(t1 t2) */ t1.k, t1.v, t2.v
FROM t1 JOIN t2 ON t1.k = t2.k ORDER BY t1.k;
`EXPLAIN VERBOSE` shows that the scan of `t1` in the hinted plan has no
`k = 1` predicate, while the un-hinted plan has one. It is an access control
issue: the hint only changes the join order, so it must never change which
rows
a user is allowed to read.
Root cause:
`CheckPolicy` materializes a row policy as a `LogicalFilter` on the relation
(and a data mask as a `LogicalProject` above it). The analysis rule
`CollectJoinConstraint` only remembered the scan itself, or the
`Project(OlapScan)` directly above it, in `LeadingHint.relationIdToScanMap`.
`LeadingHint.generateLeadingJoinPlan` rebuilds the whole join from the plans
remembered in that map, so every node that was not remembered - the row
policy
filter, the data mask project, a binder filter, and the pre-aggregation of a
random distribution aggregate table - was silently dropped when the join was
rebuilt.
Fix:
`CollectJoinConstraint` now remembers the whole plan below each side of a
join
instead of only the relation or the project above the relation
(`collectLeafPlan()`), so rebuilding the join reuses exactly the original
leaves. `LeadingHint.getBitmap()` is generalized accordingly, so that a leaf
which is built on one relation (e.g. the `LogicalAggregate` generated for a
random distribution aggregate table) is still resolved to its table bitmap.
Before the fix, the hinted query returned `(1, 10, 100)` and `(2, 20, 200)`;
after the fix it returns only `(1, 10, 100)`, the same as the un-hinted
query.
As a side effect, `SELECT /*+ leading(...) */ ...` on a random distribution
aggregate table no longer loses its pre-aggregation, which previously
returned
un-merged rows or failed the `CheckAfterRewrite` slot validation.
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] 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:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] 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]
