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

Removes the dynamic function definition #1272

Merged
merged 2 commits into from
Nov 29, 2023

Conversation

johnedquinn
Copy link
Member

Relevant Issues

Description

  • I realize I left an unused plan node in my PR of dynamic dispatch of functions. In the PR above, there is both a scalar function call (of static, unresolved, and dynamic) and a scalar function definition (resolved, unresolved, dynamic). There need not be a "definition" of a dynamic function call -- as this wouldn't make much sense. Hence, it was unused.
  • This PR removes the node. Since it was unused, no other code needs to be modified.

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.

Copy link

github-actions bot commented Nov 29, 2023

Conformance comparison report

Base (c23ca8c) e750694 +/-
% Passing 92.33% 92.33% 0.00%
✅ Passing 5372 5372 0
❌ Failing 446 446 0
🔶 Ignored 0 0 0
Total Tests 5818 5818 0

Number passing in both: 5372

Number failing in both: 446

Number passing in Base (c23ca8c) but now fail: 0

Number failing in Base (c23ca8c) but now pass: 0

@rchowell
Copy link
Contributor

Do you mind rebasing on partiql-planner branch now?

@johnedquinn johnedquinn marked this pull request as ready for review November 29, 2023 23:29
@johnedquinn johnedquinn requested a review from rchowell November 29, 2023 23:42
@johnedquinn johnedquinn merged commit 8662f3b into partiql-planner Nov 29, 2023
10 checks passed
@rchowell rchowell deleted the simplify-dynamic-fns branch November 30, 2023 00:02
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