Diveyam-Mishra commented on PR #5048:
URL: https://github.com/apache/calcite/pull/5048#issuecomment-4972686065
Hi,
@mihaibudiu Also tagging @julianhyde (Sorry not sure if i can tag
reviewers without their permission)
While working on this PR, I realized I should probably ask before taking the
implementation further.
My original assumption was that pushing filters into the scan would result
in a noticeable performance improvement. After implementing the changes, I ran
some simple benchmarks to validate that assumption. Surprisingly, I only
observed around a 5–6% improvement.
This leaves me wondering whether:
* my benchmarking methodology is flawed,
* the current implementation isn't actually pushing the filtering down
enough to provide a meaningful benefit, or
* the approach needs to be extended further (for example, pushing more of
the filtering logic into the enumerator).
Before I continue reworking the implementation, I'd appreciate your guidance
on a few design questions.
1. **Two-tier filtering**
Would you expect a hybrid approach where simple equality predicates are
evaluated in `CsvEnumerator` for early row skipping, while an `EnumerableCalc`
is still used for predicates that cannot be pushed down (ranges, OR, LIKE,
etc.)? Or would you prefer a different design?
2. **`sameValue()` vs string comparison**
The current enumerator performs string comparisons before type
conversion, allowing rows to be skipped cheaply. Using `sameValue()` would
require converting values before comparison, which seems to reduce the benefit
of early filtering. Do you have a preference here, or is the existing
string-based filtering acceptable?
3. **Scope of pushdown**
Should this PR only handle simple equality predicates (`column =
literal`), similar to the example CSV adapter, or should it also cover
predicates like ranges and `IS NULL`/`IS NOT NULL`?
4. **Predicate absorption**
Should the rule continue to absorb the entire filter into `CsvTableScan`
and let `implement()` decide what can be pushed down, or should only pushable
predicates be absorbed while leaving the rest as a `LogicalFilter`?
I'd rather align the design with what you're expecting before investing more
time in restructuring the implementation.
--
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]
