Skip to content

Conversation

@davies
Copy link
Contributor

@davies davies commented Sep 14, 2016

What changes were proposed in this pull request?

In optimizer, we try to evaluate the condition to see whether it's nullable or not, but some expressions are not evaluable, we should check that before evaluate it.

How was this patch tested?

Added regression tests.

@SparkQA
Copy link

SparkQA commented Sep 14, 2016

Test build #65402 has finished for PR 15103 at commit 1a076b7.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 16, 2016

Test build #65502 has finished for PR 15103 at commit 4b6ec42.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

val emptyRow = new GenericInternalRow(attributes.length)
val v = BindReferences.bindReference(e, attributes).eval(emptyRow)
val boundE = BindReferences.bindReference(e, attributes)
if (boundE.find(_.isInstanceOf[Unevaluable]).isDefined) return false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there's a similar hasUnevaluableExpression check in the ConvertToLocalRelation optimizer rule, except that rule also carves out a special-case exception for AttributeReference (see #13418 (comment) for discussion).

I guess we don't want to do that exemption here, though, since we're actually having to evaluate the rule and the attribute reference won't be replaced before evaluation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I guess that the AttributeReferences should have been replaced following binding in the previous line, so this looks good to me.

@JoshRosen
Copy link
Contributor

LGTM

@davies
Copy link
Contributor Author

davies commented Sep 19, 2016

Merging into master and 2.0

@asfgit asfgit closed this in d810415 Sep 19, 2016
asfgit pushed a commit that referenced this pull request Sep 19, 2016
## What changes were proposed in this pull request?

In optimizer, we try to evaluate the condition to see whether it's nullable or not, but some expressions are not evaluable, we should check that before evaluate it.

## How was this patch tested?

Added regression tests.

Author: Davies Liu <[email protected]>

Closes #15103 from davies/udf_join.

(cherry picked from commit d810415)
Signed-off-by: Davies Liu <[email protected]>
wgtmac pushed a commit to wgtmac/spark that referenced this pull request Sep 19, 2016
## What changes were proposed in this pull request?

In optimizer, we try to evaluate the condition to see whether it's nullable or not, but some expressions are not evaluable, we should check that before evaluate it.

## How was this patch tested?

Added regression tests.

Author: Davies Liu <[email protected]>

Closes apache#15103 from davies/udf_join.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants