Fix testApplyTableScanRedirectionWithFilter's TableScan#7228
Conversation
There was a problem hiding this comment.
This test was meant to simulate the following scenario:
There are 2 columns A and B, there is a predicate on A and projection on B
Constraint on column A is pushed into TS and enforced by connector
Column A assignment is pruned from TS by column pruning rule
Maybe instead of removing, the test case can be fixed by passing TupleDomain.all in enforcedConstraint (or whatever is left in enforcedConstraint after PPD when a constraint is enforced by connector and engine does not need to do any extra work). I think we are correctly sending constraint into MockConnectorTableHandle to mimc PPD.
There was a problem hiding this comment.
@raunaqmorarka is the described test scenario covered by some test in TestTableScanRedirectionWithPushdown?
What do we specifically test here? After @findepi changes:
Constraint on column A is pushed into TS and enforced by connector
Column A assignment is pruned from TS by column pruning rule
won't preseve predicate on A in enforced constraint
There was a problem hiding this comment.
TestTableScanRedirectionWithPushdown#testRedirectAfterColumnPruningOnPushedDownPredicate is also about this scenario. However, there we are testing the end result of applying multiple rules. We are not explicitly verifying the result of just running the redirection rule in this scenario elsewhere.
We are preserving predicate on A in the mock table handle. I'm assuming this is sufficient to mimic the PPD part of the above scenario and that the TupleDomain in enforced constraint doesn't need to have predicate on A.
There was a problem hiding this comment.
Suggested change:
Replace constraint with TupleDomain.all() in 4th parameter to p.tableScan
There was a problem hiding this comment.
From unit test perspective, this will be equivalent to unfiltered case, covered by io.trino.sql.planner.iterative.rule.TestApplyTableScanRedirection#testApplyTableScanRedirection.
There was a problem hiding this comment.
It wouldn't be equivalent to unfiltered case because the constraint would still be passed to MockConnectorTableHandle and the test will continue to match for a filter node.
findepi
force-pushed
the
findepi/remove-invalid-table-scan-redirection-test-case-b8868b
branch
from
f39bc4e to
2e5d23f
Compare
|
@sopel39 @raunaqmorarka |
findepi
force-pushed
the
findepi/remove-invalid-table-scan-redirection-test-case-b8868b
branch
from
2e5d23f to
ff47dad
Compare
findepi
changed the title
findepi
force-pushed
the
findepi/remove-invalid-table-scan-redirection-test-case-b8868b
branch
from
ff47dad to
7d12a48
Compare
|
rebased to resolve conflict. |
The test was constructing `TableScan` with a `Domain` on a non-projected column, but such columns should be removed from the plan together with associated constraints. The test input is fixed to adhere to this.
findepi
force-pushed
the
findepi/remove-invalid-table-scan-redirection-test-case-b8868b
branch
from
7d12a48 to
5e8f7b6
Compare
findepi
deleted the
findepi/remove-invalid-table-scan-redirection-test-case-b8868b
branch
3 participants
Extracted from #6963