Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SEDONA-348] Implemented null-tolerant variants of inferred functions #952

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

Kontinuation
Copy link
Member

Did you read the Contributor Guide?

Is this PR related to a JIRA ticket?

What changes were proposed in this PR?

Functions such as ST_MakePoint need to accept null as ordinates. For example, we can use ST_MakePoint(x, y, null, m) to construct a point object with M ordinate. This requires the InferredExpression to support null-tolerant functions.

This patch introduces a family of nullTolerantInferredFunction functions for wrapping null-tolerant functions as spark catalyst expressions. For example, makePoint can be wrapped in this way:

case class ST_MakePoint(inputExpressions: Seq[Expression])
  extends InferredExpression(nullTolerantInferrableFunction4(Constructors.makePoint)) {
  // ...
}

We won't check null values for function arguments when evaluating ST_MakePoint and it is up to the wrapped function (makePoint in this case) to handle null values correctly.

How was this patch tested?

We've cherry-picked this patch on top of #950 and verified that it works as expected.
It will be covered by more null-tolerant functions we'll implement in the future.

Did this PR include necessary documentation updates?

  • No, this PR does not affect any public API so no need to change the docs.

@jiayuasu jiayuasu merged commit e94a886 into apache:master Aug 8, 2023
39 checks passed
@Kontinuation Kontinuation deleted the inferred-expr-null-tolerance branch August 23, 2023 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants