-
Notifications
You must be signed in to change notification settings - Fork 472
sql: make errors point to recorded view commands #13445
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
sql: make errors point to recorded view commands #13445
Conversation
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.
LGTM
Mind holding off on this one for a day, @teskje? I want to see if I can use this as motivation to plumb |
@benesch Of course! |
Most of the functions still produce only PlanError::Unstructured, but this will allow us to incrementally move each error to a structured error type. This allows the error structure to be propagated upwards to the coordinator when it exists, which will allow the structured SQL errors to return hints and long form descriptions. In particular, after this change, the error messages under construction in MaterializeInc#13445 will be able to properly emit hints.
Ok, done in #13455! |
Most of the functions still produce only PlanError::Unstructured, but this will allow us to incrementally move each error to a structured error type. This allows the error structure to be propagated upwards to the coordinator when it exists, which will allow the structured SQL errors to return hints and long form descriptions. In particular, after this change, the error messages under construction in MaterializeInc#13445 will be able to properly emit hints.
Most of the functions still produce only PlanError::Unstructured, but this will allow us to incrementally move each error to a structured error type. This allows the error structure to be propagated upwards to the coordinator when it exists, which will allow the structured SQL errors to return hints and long form descriptions. In particular, after this change, the error messages under construction in MaterializeInc#13445 will be able to properly emit hints.
Most of the functions still produce only PlanError::Unstructured, but this will allow us to incrementally move each error to a structured error type. This allows the error structure to be propagated upwards to the coordinator when it exists, which will allow the structured SQL errors to return hints and long form descriptions. In particular, after this change, the error messages under construction in #13445 will be able to properly emit hints.
When a user tries to use regular view commands (DROP VIEW, SHOW CREATE VIEW, ALTER VIEW) on recorded views, point them to the correct RECORDED VIEW commands in the error messages.
140be90
to
c946920
Compare
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.
LGTM
cc @philip-stoev: This solves item no. 2, 3, and 5 from #13346. |
When a user tries to use regular view commands (
DROP VIEW
,SHOW CREATE VIEW
,ALTER VIEW
) on recorded views, point them to the correct RECORDED VIEW commands in the error messages.Motivation
Part of MaterializeInc/database-issues#3692.
Tips for reviewer
The error messages are not optimal, because we lack the ability to returnAdapterError
s inside thesql
code (see my inline comment for more detail). If someone can think of a way to improve this without completely refactoring the crate's error handling, let me know!Testing
Release notes
This PR includes the following user-facing behavior changes: