Fix CREATE SCHEMA schema name double quoting issue. #5059
Conversation
| SchemaName::UnnamedAuthorization(auth) => ident_to_string(auth), | ||
| SchemaName::NamedAuthorization(schema_name, auth) => format!( | ||
| "{}.{}", | ||
| object_name_to_string(schema_name), |
There was a problem hiding this comment.
I wonder if this name needs to be normalized too
like CREATE SCHEMA Foo should create the schema foo (not Foo)
There was a problem hiding this comment.
Hey, thank you so much for the quick and positive response!
Honestly, I have no idea if Foo should become foo, but I'm happy to update the code to call utils::normalize_ident() on ident.value if necessary!
There was a problem hiding this comment.
Yes I think it should be lowercase, here is a demonstration of how that works in postgres:
postgres=# drop schema foo;
DROP SCHEMA
postgres=# create schema foo;
CREATE SCHEMA
postgres=# drop schema foo;
DROP SCHEMA
postgres=# create schema FoO;
CREATE SCHEMA
postgres=# create schema foo;
ERROR: schema "foo" already existsThere was a problem hiding this comment.
Thanks for the input! The code in the PR now calls normalize_ident and includes an integration test asserting this behavior.
|
Thanks @neumark |
|
Benchmark runs are scheduled for baseline = e24a7ec and contender = 8b716d3. 8b716d3 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #5058
Rationale for this change
The changes fix a bug resulting in double-quoted identifiers in SQL query plans.
What changes are included in this PR?
The changes affect logical plan creation in
datafusion/sql/src/statement.rsAre these changes tested?
The PR includes an integration test for the
CREATE SCHEMASQL statement.Are there any user-facing changes?
User queries with quoted identifiers will work as expected due to this change.