sql: improve error message for search_path with commas #53974
Merged
Conversation
This file contains 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
|
This change is |
solongordon
approved these changes
Sep 4, 2020
pkg/sql/vars.go
Outdated
| "schema name %q not supported in search_path", s) | ||
| return "", unimplemented.NewWithIssuef(53971, | ||
| `schema name %q has commas so is not supported in search_path. | ||
| Did you mean to omit quotes? SET search_path = %s`, s, s) |
There was a problem hiding this comment.
Might be nicer to make this a hint via errors.WithHint.
There was a problem hiding this comment.
I thought about that, but the unimplemented error type already uses a hint... I guess I could use two, or just not make it an unimplemented error, but this way we get telemetry on this hilarious situation.
There was a problem hiding this comment.
Hints are made to be chained / combined. We do this in other places too.
There was a problem hiding this comment.
Coming back to this several months later - thanks for the suggestion! Done.
It's easy to accidentally surround your search path with quotes when setting it, because you'd think that most things in `SET` syntax are quoted. But you are not supposed to quote things in set search_path, and it can lead to confusing scenarios. Now, if you try to set search_path to a string containing a comma, which we don't support anyway, the error message will be a bit friendlier. Release note (sql change): improve error message when people use set search_path incorrectly, or with a schema that legitimately has a comma in its name Release justification: error-message-only change
jordanlewis
force-pushed
the
fix-search-path-error
branch
from
159dc25 to
73d3246
Compare
|
bors r+ |
|
Build succeeded: |
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.
It's easy to accidentally surround your search path with quotes when
setting it, because you'd think that most things in
SETsyntax arequoted. But you are not supposed to quote things in set search_path, and
it can lead to confusing scenarios. Now, if you try to set search_path
to a string containing a comma, which we don't support anyway, the error
message will be a bit friendlier.
Release note (sql change): improve error message when people use set
search_path incorrectly, or with a schema that legitimately has a comma
in its name
Release justification: error-message-only change