Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/2810
Thanks for the update @tonycox.
Merging this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2810
@fhueske I update this PR according to last changes in master
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/2810
Thanks for asking @tonycox! FLINK-5188 is more urgent. We want to do the
feature freeze by end of the week. Would be great if you could open a PR at
Thursday the latest. If this is too soon, please
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2810
Hi @fhueske , what should I finish first, this PR or FLINK-5188 ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2810
#2926
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2810
Hi @fhueske and @KurtYoung I totally agree with the plan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user KurtYoung commented on the issue:
https://github.com/apache/flink/pull/2810
Hi @fhueske, I think this is a fair enough approach, thanks for your
effort. We will change our pr ASAP.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/2810
Hi @tonycox, @beyond1920, and @KurtYoung,
I think I found a good way to split the issue. Let me first summarize what
both of your PRs implement:
@tonycox (PR #2810)
- good test
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2810
@fhueske Basically yes, but I think we need finish projection rule for
Stream, after FLINK-5251 merging
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/2810
@tonycox, great. Thanks for the notice! Is the PR ready to review from your
point of view or are you still working on it?
@KurtYoung Thanks for reaching out to this PR. I'd like to propose
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2810
Hi @fhueske, your PR do solve my problem. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this
Github user KurtYoung commented on the issue:
https://github.com/apache/flink/pull/2810
Hi @tonycox , i want to appologize first for opening another "project
pushdown" issue when this one is still in active. We actually come up with some
whole new idea and designs compare to the
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/2810
Hi @tonycox, the plan choice issues might be related to FLINK-5226. I have
a PR pending to fix this issue: #2926. You could try to built your PR on top of
my PR (or for the sake of simplicity simply
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2810
I see. @fhueske. Set an order of scaning is much better way.
I have problem when plan is choosing. Best expression is not correct. Rules
work, but planner decides to choose `MapFunction` anyway,
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/2810
This is a good question @tonycox. First of all, we need to agree on the
semantics of the `ProjectableTableSource.setProjection()` method. IMO, the
table source must return a `DataSet` (or
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2810
@wuchong, @fhueske, question about field shuffling, should I shuffle it in
RowCsvInputFormat by setting an order to scan, or let LogicalCal do it?
---
If your project is set up for it, you can
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/2810
Please rebase. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2810
Hi @fhueske , I see conflicts PR check, should I rebase new commits, or
merge it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/2810
Hi @tonycox, can you please drop a comment when the PR is ready for review
again.
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2810
Hi @wuchong , thank you for the review.
Do you mean only for `Stream`?
And why we shouldn't to override computeSelfCost for DataStreamRel ?
---
If your project is set up for it, you can
Github user wuchong commented on the issue:
https://github.com/apache/flink/pull/2810
Hi @tonycox , thanks for the update. I'm thinking of an idea that
`StreamProjectableTableSourceScan` seems duplicate with
`StreamTableSourceScan`, why not reuse the `StreamTableSourceScan` with an
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/2810
Hi @tonycox, I'll have a look over the weekend or early next week as well.
Thanks, Fabian
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user wuchong commented on the issue:
https://github.com/apache/flink/pull/2810
Hi @tonycox , the overall change looks good to me. I will do more thorough
review in this weekend.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2810
@wuchong could you review my changes?
cc @fhueske @StephanEwen
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2810
@wuchong I added noticed items
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and
Github user wuchong commented on the issue:
https://github.com/apache/flink/pull/2810
Hi @tonycox , the
[`RowCsvInputFormat`](https://github.com/apache/flink/blob/master/flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/runtime/io/RowCsvInputFormat.scala#L100)
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2810
> In addition, the TableSource should know project information (including
order) not just fieldIncluded. So maybe we should also adapt RowCsvInputFormat.
Do you mean we need to shuffle field
27 matches
Mail list logo
