Skip to content
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

fix(planner): corretly handle catalog in statements #5909

Merged
merged 3 commits into from
Jun 12, 2022

Conversation

andylokandy
Copy link
Collaborator

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

  • Parser: permit to specify catalog for DATABASE, TABLE and VIEW statements.
  • Planner: lowercase all catalog, database, and tables names
  • The other minor refactors.

Changelog

  • Bug Fix
  • Improvement

Related Issues

Part of #5907

@vercel
Copy link

vercel bot commented Jun 11, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
databend ⬜️ Ignored (Inspect) Jun 12, 2022 at 4:57AM (UTC)

@mergify
Copy link
Contributor

mergify bot commented Jun 11, 2022

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

@mergify mergify bot added pr-bugfix this PR patches a bug in codebase pr-improvement labels Jun 11, 2022
@BohuTANG
Copy link
Member

workspace/tests/suites/0_stateless/05_ddl/05_0003_ddl_alter_database_v2.stdout 2022-06-12 00:30:24.425825160 +0000
871
@@ -1,3 +1,5 @@
872
+ERROR 1105 (HY000) at line 44: Code: 1003, displayText = Unknown database 'b'.
873
+ERROR 1105 (HY000) at line 45: Code: 1003, displayText = Unknown database 'a'.
874
system
875
INFORMATION_SCHEMA
876
a

@sundy-li
Copy link
Member

sundy-li commented Jun 12, 2022

lowercase all catalog, database, and tables names

Is this behavior commonly used in other DBMS ? I thought it's always case-sensitive.

@leiysky
Copy link
Contributor

leiysky commented Jun 12, 2022

lowercase all catalog, database, and tables names

Is this behavior commonly used in other DBMS ? I thought it's always case-sensitive.

In most of the databases, the default behaviour is case-insensitive.

And the case-sensitivity of identifiers is controled in different ways, for example:

This is work about normalizing the convention of identifiers, which is correlated with #5717 .

This PR only makes minor refactor of AST and unifies the behaviour of identifier case handling in new planner. It doesn't aim to resolve the issue of identifier case.

I have made an offline discussion with @andylokandy , we decide to get rest work done later.

@BohuTANG BohuTANG merged commit 5de5b3f into databendlabs:main Jun 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need-review pr-bugfix this PR patches a bug in codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants