YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-817988650
@godfreyhe Addressed your comments and rebased latest master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-816832991
@fsk119 All fixes are done except the one for removing
`PushFilterIntoTableSourceScanRule`, not sure I understood what you meant there.
--
This is an automated
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-816019831
@fsk119 A question about the predicate we added to return a
`LogicalTableScan` only instead of one wrapped with calc:
```java
if
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-816019831
@fsk119 A question about the predicate we added to return a
`LogicalTableScan` only instead of one wrapped with calc:
```
if
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-816019831
@fsk119 A question about the predicate we added to return a
`LogicalTableScan` only instead of one wrapped with calc:
```
if
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-816019831
@fsk119 A question about the predicate we added to return a
`LogicalTableScan` only instead of one wrapped with calc:
```
if
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-816019831
@fsk119 A question about the predicate we added to return a
`LogicalTableScan` only instead of one wrapped with calc:
```
if
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-815949448
@fsk119 After fixing the problem and rebasing master, it seems like there
are still some failing tests but they look non related to this change? I ran
the planner
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-815626304
@fsk119 I have a problem with `FilterableSourceTest`. If I move the new rule
back up again after the watermark pushdown, in
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-815626304
@fsk119 I have a problem with `FilterableSourceTest`. If I move the new rule
back up again after the watermark pushdown, in
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-815626304
@fsk119 I have a problem with `FilterableSourceTest`. If I move the new rule
back up again after the watermark pushdown. In
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-815143782
@fsk119 I reverted the rule to it's original place in the rule set. Now I
have two failing tests in `PushFilterInCalcIntoTableSourceRuleTest` that are
failing due
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-815143782
@fsk119 I reverted the rule to it's original place in the rule set. Now I
have two failing tests in `PushFilterInCalcIntoTableSourceRuleTest` that are
failing due
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-814257612
@fsk119 To be honest I don't understand why we need the added complexity of
code inheritance here, these tests should be ran individually anyway. If each
one of
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-814173776
After implementing the changes, the `PushFilterIntoTableSourceScanRuleTest`
now fail since I cant run the `setup` code for them (due to use of batch vs
stream):
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-814064673
@fsk119 OK, I figured it out. Since I abstracted away
`createTableScanAfterPushdown` to the base class, I always returned
`FlinkLogicalTableScan.create`. This
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-814042062
@fsk119 Pushing now.
--
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
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-814002061
@fsk119 A question about the implementation:
```
if (result.getRemainingFilters().isEmpty() && unconvertedPredicates.length
== 0) {
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-814002061
@fsk119 A question about the implementation:
```
if (result.getRemainingFilters().isEmpty() && unconvertedPredicates.length
== 0) {
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-813494028
@fsk119 When you say solve by merging, do you mean to add the
`FlinkCalcMergeRule` to the tests?
--
This is an automated message from the Apache Git Service.
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-813077657
Placing `PushFilterInCalcIntoTableSOurceScanRule` after
`FlinkLogicalCalcRemoveRule` in LOGICAL_REWRITES fixes the issue.
--
This is an automated message from
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-812995554
@fsk119 I've added a test to check computed column pushdown and it's failing.
Test:
```java
@Test
public void
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-812512544
@fsk119 I'm looking at
`org.apache.flink.table.planner.plan.stream.sql.CalcTest` and all I see is
basic tests there over table source, what do you think should be
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-812503994
> > @YuvalItzchakov . I think the added test is not enough to cover all.
> > I think you should also need to add end to end Tests in the
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-812499504
@fsk119 After the change, my partial predicate match no longer works:
```java
@Test
public void
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-812490603
> > @fsk119 I see you also added a test for partition pushdown, are those
related?
>
> Not related. I just try to find why the change breaks the test.
>
YuvalItzchakov edited a comment on pull request #15307:
URL: https://github.com/apache/flink/pull/15307#issuecomment-812471049
@fsk119 I'm confused with all the suggestions. Should I keep working off
this branch but have this as a logical rewrite instead? Are all the comments
you left
27 matches
Mail list logo
