englefly opened a new pull request, #66801:
URL: https://github.com/apache/doris/pull/66801
### What problem does this PR solve?
Problem Summary:
`LogicalCatalogRelation.computeUnique()` registered a **partial** unique key
when the scan output did not contain every constrained column:
`findSlotsByColumn()` returned `outputSet ∩ columns`, so a non-base index
covering only `(a, c)` of a table-level `UNIQUE(a, b)` constraint advertised
`{a}` as unique. The FD `a -> c` derived from it then let `EliminateGroupByKey`
drop `c` from `GROUP BY` (wrapping it with `any_value`), merging distinct
groups such as `(1,'x')` and `(1,'y')`.
`LogicalOlapScan.computeUnique()` imported table-level constraints via
`super.computeUnique()` **before** its raw-version guards ran. For MOR
unique-key tables read as DUP (`read_mor_as_dup_tables`, or
`skipDeleteBitmap`), the read exposes every version, e.g.
`(1,10),(1,20),(1,30)`, so the unique key `k` is not unique in the data; the
early `return` still left the superclass constraint registered, and the `k ->
v` FD could collapse those three groups into one.
Fix:
- `findSlotsByColumn()` now requires **every** constrained column to be
present in the scan output; when any is missing it returns an empty set, so a
partial constraint is never registered (both `LogicalCatalogRelation` and
`PhysicalCatalogRelation`).
- `LogicalOlapScan.computeUnique()` checks the raw-version read conditions
(`skipDeleteBitmap` / `read_mor_as_dup_tables`) **before**
`super.computeUnique()`, so the table constraint is not imported for data whose
uniqueness does not hold; the redundant inner guards were removed.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Unit Test
- `FdTest.testScanOutputMissingConstraintColumns`: scan output
missing a constrained column must not register the partial key as unique.
- `FdTest.testMorReadAsDupSuppressesUniqueConstraint`: MOR table
read as DUP must not keep the `k` unique constraint.
- Both fail on the old code and pass with the fix; the full `FdTest`
class is green.
- [ ] Regression test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
Fixes wrong query results (merged groups) for the two scan shapes
above; no intended plan-shape or performance change otherwise.
- Does this need documentation?
- [x] 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]
