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

Fixes function invocation for missing arguments #1523

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

johnedquinn
Copy link
Member

@johnedquinn johnedquinn commented Jul 24, 2024

Description

  • Fixes function invocation for missing arguments and fixes evaluation of Rex.Op.Missing.

Other Information

License Information

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Fixes evaluation of Rex.Op.Missing
@johnedquinn johnedquinn force-pushed the v1-functions-missing branch from 97a6fe6 to 923c8a0 Compare July 25, 2024 17:08
@johnedquinn johnedquinn marked this pull request as ready for review July 25, 2024 17:22
Copy link
Contributor

@yliuuuu yliuuuu left a comment

Choose a reason for hiding this comment

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

I understand this PR is for evaluator only.

Curious about your thoughts on whether we should also fix the planer. i.e, The Planner is operating under signal mode, will turn 1 + MISSING in to an rexOpErr.

@johnedquinn johnedquinn merged commit b5c596c into partiql:v1 Jul 25, 2024
7 checks passed
@johnedquinn johnedquinn deleted the v1-functions-missing branch July 25, 2024 23:43
@johnedquinn
Copy link
Member Author

I understand this PR is for evaluator only.

Curious about your thoughts on whether we should also fix the planer. i.e, The Planner is operating under signal mode, will turn 1 + MISSING in to an rexOpErr.

Just checked to be sure, always missing arguments currently returns a Rex.Op.Missing.

@yliuuuu
Copy link
Contributor

yliuuuu commented Jul 25, 2024

The conversion happens when we convert the internal plan to public plan.

i.e.,

override fun visitRexOpMissing(node: Rex.Op.Missing, ctx: Unit): PlanNode {
// gather problem from subtree.
val trace = node.causes.map { visitRexOp(it, ctx) }
return when (signalMode) {
true -> {
onProblem.invoke(ProblemGenerator.asError(node.problem))
rexOpErr(node.problem.toString(), trace)
}
false -> {
onProblem.invoke(ProblemGenerator.asWarning(node.problem))
org.partiql.plan.rexOpMissing(node.problem.toString(), trace)
}
}
}

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.

2 participants