Github user viirya commented on the issue:
https://github.com/apache/spark/pull/18346
@cloud-fan Thanks for update. Considering the complexity of codegen-only
expressions, I guess we need significant community effort and review bandwidth
in order to make all expressions support
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/18346
Sorry I'm taking this off, `CodegenFallback` may be not needed, but we do
need the interpreted evaluation of expressions, see
https://issues.apache.org/jira/browse/SPARK-21320 .
So to
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/18346
Ah, that's right. Catalyst is not public API.
---
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 cloud-fan commented on the issue:
https://github.com/apache/spark/pull/18346
`CodegenFallback` is never exposed to users, Spark doesn't have an official
API for customized expressions, it's all internal. You can develop customized
logical plan, physical plan, expression,
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/18346
ping @cloud-fan any more feedback on this?
---
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
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/18346
Another concern I might have is that `CodegenFallback` is already exposed
to users. So is it ok to remove it now?
---
If your project is set up for it, you can reply to this email and have your
Github user dbtsai commented on the issue:
https://github.com/apache/spark/pull/18346
@cloud-fan +1 on this. This can clear up the testability issue we have when
implementing `Expression` since it's challenging to test properly for both
`eval` and `codegen` path. If we can
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/18346
Interesting. I'd close this first if we don't have `CodegenFallback`
anymore.
---
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
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/18346
@dbtsai that's my proposal, and I'm going to open a JIRA to make all
expression support codegen and remove `CodegenFallback`.
---
If your project is set up for it, you can reply to this email
Github user dbtsai commented on the issue:
https://github.com/apache/spark/pull/18346
Does it mean all the expressions **have to** implement codegen, and no
`eval` any more?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/18346
I mean, there should be no non-codegen expressions in the future. All
expressions should be able to do codegen by passing some references to the
generated code, instead of passing itself to the
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/18346
Not sure if I understand correctly. Currently `CodegenFallback` already
does codegen for non-codegen expressions via `CodegenContext.references`. So I
am not sure if we don't have an adapter like
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/18346
I think eventually we should remove `CodegenFallback`. With
`CodegenContext.addReferenceObj`, we can codegen everything, and we can even
remove `Expression.eval` at some point.
---
If your
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/18346
Btw, even we can evaluate all children expressions of `CodegenFallback`
with codegen path, we still can't do wholestage codegen with the plans
including `CodegenFallback` expressions. We just can do
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/18346
Thanks @dbtsai for the comment.
Yeah, I've also tried to let `CodegenFallback` evaluate all its children
under codegen path in parallel with this PR. It works.
Of course the
Github user dbtsai commented on the issue:
https://github.com/apache/spark/pull/18346
Thanks, @viirya for this PR.
We hit this issue, and @viirya was kindly helping us to find the root
cause. This approach LGTM. One alternative approach we took in the end to
unblock our
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/18346
cc @cloud-fan @gatorsmile
---
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 AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18346
Merged build finished. Test PASSed.
---
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
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18346
Merged build finished. Test PASSed.
---
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
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18346
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78238/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18346
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78237/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18346
**[Test build #78238 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78238/testReport)**
for PR 18346 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18346
**[Test build #78237 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78237/testReport)**
for PR 18346 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18346
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78236/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18346
Merged build finished. Test PASSed.
---
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
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18346
**[Test build #78236 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78236/testReport)**
for PR 18346 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18346
Merged build finished. Test FAILed.
---
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
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18346
**[Test build #78234 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78234/testReport)**
for PR 18346 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18346
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78234/
Test FAILed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18346
**[Test build #78238 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78238/testReport)**
for PR 18346 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18346
**[Test build #78237 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78237/testReport)**
for PR 18346 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18346
**[Test build #78236 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78236/testReport)**
for PR 18346 at commit
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/18346
cc @dbtsai
---
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
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18346
**[Test build #78234 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78234/testReport)**
for PR 18346 at commit
34 matches
Mail list logo
