-
-
Notifications
You must be signed in to change notification settings - Fork 216
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
refactor create procedure and call procedure #2833
Conversation
… into james/proc
… into james/proc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shape of the changes looks good, if all tests pass it seems like a good directional improvement even if it's still missing correctness edge cases.
|
||
// limit validation | ||
case *ast.Select: | ||
if s.Limit != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the limit/offset in analyzer seems like it does a lot, was this for a specific test that regular analyzer loop doesn't catch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, create procedure
needs to check that the limit/offset value is actually a numeric.
This PR refactors a ton of the stored procedure behavior to more closely match MySQL.
Changes:
plan.Call
nodesPartially addresses: dolthub/dolt#8053