Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/15840
Finally the purpose of this pr is to fix nullabilities of `MapObjects` and
`ExternalMapToCatalyst`, I'll update pr title and description.
---
If your project is set up for it, you can reply to this
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/15840
@hvanhovell Ah, good question.
I checked the Janino code around `IfStatement` and I found Janino seems to
optimize if/else block, too:
https://github.com/janino-compiler/janino/blob/22a7
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/15840
LGTM - pending jenkins.
Small question: is it really easier for Janino to optimize ternary
operators than an if/else block?
---
If your project is set up for it, you can reply to this e
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/15840
@hvanhovell Could you review this pr again, please?
I reverted some commits which replace null checking with
`ctx.nullSafeExec()` and then modified some null checkings to `${isNull} ? null
: ${va
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15840
**[Test build #68931 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68931/consoleFull)**
for PR 15840 at commit
[`78d9e80`](https://github.com/apache/spark/commit/7
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/15840
@hvanhovell I'd like to revert some of commits in this pr which replace
null checking with `ctx.nullSafeExec()` to focus on the original purpose of
this pr, and send another pr to discuss null checki
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15840
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/68629/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15840
**[Test build #68629 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68629/consoleFull)**
for PR 15840 at commit
[`ec0c55c`](https://github.com/apache/spark/commit/
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15840
**[Test build #68627 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68627/consoleFull)**
for PR 15840 at commit
[`658c5e3`](https://github.com/apache/spark/commit/
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15840
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
e
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15840
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/68627/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15840
**[Test build #68629 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68629/consoleFull)**
for PR 15840 at commit
[`ec0c55c`](https://github.com/apache/spark/commit/e
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15840
**[Test build #68627 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68627/consoleFull)**
for PR 15840 at commit
[`658c5e3`](https://github.com/apache/spark/commit/6
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15840
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/68607/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15840
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
e
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15840
**[Test build #68607 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68607/consoleFull)**
for PR 15840 at commit
[`0fc36a8`](https://github.com/apache/spark/commit/
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/15840
@hvanhovell The pattern basically I used to replace nullability checking
here is like:
```scala
val eval = child.genCode(ctx)
ev.copy(code = s"""
${eval.code}
boolean ${ev
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15840
**[Test build #68607 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68607/consoleFull)**
for PR 15840 at commit
[`0fc36a8`](https://github.com/apache/spark/commit/0
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/15840
I replaced nullability checking where we can use
`CodegenContext.nullSafeExec()`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If y
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/15840
I agree that this case is fine.
---
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 w
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/15840
@kiszk I have not checked all the case yet but I think the case that we
need to generate else-clause doesn't match the case we discuss here.
Of course we can add the method you suggested when we f
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/15840
Does this approach avoid to generate code `convertedArray[loopIndex] =
null;` if `lambdaFunction.nullable` is `true`? Is it a design? I know that it
works correctly in this case since an array allocat
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15840
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
e
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15840
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/68509/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15840
**[Test build #68509 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68509/consoleFull)**
for PR 15840 at commit
[`deffccd`](https://github.com/apache/spark/commit/
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/15840
@hvanhovell @kiszk I tried to use `CodegenContext.nullSafeExec()` in
`MapObjects` as an example.
If you can bear with this for now, I'll apply it to other places to
generate nullability checking
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15840
**[Test build #68509 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68509/consoleFull)**
for PR 15840 at commit
[`deffccd`](https://github.com/apache/spark/commit/d
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/15840
I found that we could use `CodegenContext.nullSafeExec()` for most case,
which is not so generic but we can easily replace nullability checking for now.
---
If your project is set up for it, you can
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/15840
@hvanhovell Thank for pointing out `NullIntolerant`. I did not know it. I
also provide it as a starting point based on two instances that I saw today.
I agree that it would be good to add `NullInto
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/15840
@kiszk that could be a start. However I would like to take a step back, and
see which nullability checking patterns are common, and provide generic
utilities for them. I also think that the prope
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/15840
Do we prepare a utility method like this in `CodeGenerator`?
```java
def genNullableAssignment(genIsNull: boolean, isNull: boolean, lhs: String,
rhs: String): String = {
if (genIsNull) {
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15840
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
e
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15840
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/68466/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15840
**[Test build #68466 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68466/consoleFull)**
for PR 15840 at commit
[`b287ded`](https://github.com/apache/spark/commit/
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/15840
On a whole this PR looks good. On a more general level, I would like to
suggest that we should have a more generic way of preventing unneeded
null-checks.
---
If your project is set up for it,
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15840
**[Test build #68466 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68466/consoleFull)**
for PR 15840 at commit
[`b287ded`](https://github.com/apache/spark/commit/b
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/15840
Jenkins, retest this please.
---
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 wis
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15840
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
e
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15840
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/68462/
Test FAILed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15840
**[Test build #68462 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68462/consoleFull)**
for PR 15840 at commit
[`b287ded`](https://github.com/apache/spark/commit/
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15840
**[Test build #68462 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68462/consoleFull)**
for PR 15840 at commit
[`b287ded`](https://github.com/apache/spark/commit/b
41 matches
Mail list logo
