refactor: Pass the context to the executor compiler #5222
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
A bit tedious to drag the context through everywhere here since this compiler should be removed eventually, but I'd expect the context to be wanted in whatever replaces it anyway so. |
onelson
approved these changes
Sep 26, 2022
Contributor
onelson
left a comment
There was a problem hiding this comment.
On a superficial level, this looks to me as just what is needed.
I'm not sure what we can do to verify the change other than to rely on the existing suite, and maybe merge it after the next release so we can have a week to run it ourselves with the hope that anything lacking will surface during that time.
9771c10 to
f3d84df
Compare
This was referenced Sep 26, 2022
onelson
pushed a commit
that referenced
this pull request
Sep 28, 2022
A new version of our logical evaluator was introduced in an effort to re-align the lang with the spec regarding nulls. Ref: <#5192> The "strict null" flavor of the logical evaluator was bypassed as part of a perf regression investigation. Ref: <#5221> The thought was checking the feature flag with each eval was adding overhead and raising query latency, but with a susequent refactor (<#5222>) we are now able to check the flag earlier, reducing the overhead.
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Should make it possible to enable the strict null logical ops when compiling the executor instead of branching inside the executor itself.
Checklist
Dear Author 👋, the following checks should be completed (or explicitly dismissed) before merging.
experimental/docs/Spec.mdhas been updatedDear Reviewer(s) 👋, you are responsible (among others) for ensuring the completeness and quality of the above before approval.