Skip to content

Conversation

@viirya
Copy link
Member

@viirya viirya commented Oct 31, 2017

What changes were proposed in this pull request?

Under the current execution mode of Python UDFs, we don't well support Python UDFs as branch values or else value in CaseWhen expression.

Since to fix it might need the change not small (e.g., #19592) and this issue has simpler workaround. We should just notice users in the document about this.

How was this patch tested?

Only document change.

@viirya
Copy link
Member Author

viirya commented Oct 31, 2017

cc @HyukjinKwon @BryanCutler

@viirya
Copy link
Member Author

viirya commented Oct 31, 2017

retest this please.

@SparkQA
Copy link

SparkQA commented Oct 31, 2017

Test build #83242 has finished for PR 19617 at commit a43430b.

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

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

LGTM otherwise.

it is present in the query.
.. note:: The user-defined functions do not support conditional execution by using them with
SQL conditional expressions such `when` or `if`. The functions still apply on all rows no
Copy link
Member

Choose a reason for hiding this comment

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

Looks a tiny typo expressions such `when` or `if`. -> expressions such as `when` or `if`..

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops! Thanks.

duplicate invocations may be eliminated or the function may even be invoked more times than
it is present in the query.
.. note:: The user-defined functions do not support conditional execution by using them with
Copy link
Member

Choose a reason for hiding this comment

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

Hm, should we maybe clarify the output itself is correct if it does not cause the runtime failure by the condition? Maybe I am too much worried but think it might mislead the output is incorrect at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I think this is a valid worry. Thanks.

@SparkQA
Copy link

SparkQA commented Oct 31, 2017

Test build #83261 has finished for PR 19617 at commit 47bb26e.

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

@viirya
Copy link
Member Author

viirya commented Nov 1, 2017

cc @cloud-fan for checking the document too.

@cloud-fan
Copy link
Contributor

thanks, merging to master!

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.

4 participants