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

feat: Extend ANALYZE common syntax to cover multiple dialects #4591

Merged
merged 9 commits into from
Jan 14, 2025

Conversation

sqlglot/parser.py Outdated Show resolved Hide resolved
sqlglot/expressions.py Outdated Show resolved Hide resolved
sqlglot/parser.py Outdated Show resolved Hide resolved
sqlglot/parser.py Outdated Show resolved Hide resolved
@georgesittas
Copy link
Collaborator

@zashroof, no need to spend too much time on Drill and Doris, they're low priority dialects.

@geooo109
Copy link
Collaborator

geooo109 commented Jan 10, 2025

@zashroof first of all, great work!
After resolving the comments of @georgesittas .
You can check the following (if you have already a test for any of those and I missed it, you can skip it) :

  1. Add a test for databricks using ANALYZE TABLES IN .
  2. Add a test for mysql using DROP HISTOGRAM.
  3. Add a test for postgres using BUFFER_USAGE_LIMIT .
  4. Add tests for starrocks using FULL , WITH N BUCKETS (for histogram) , and WITH SYNC/ASYNC MODE .
  5. As I can understand for the oracle dialect, the syntax for things like here CLUSTER, validation_clauses, LIST_CHAINED_ROWS, DELETE isn't covered. You can use the exp.Command as a fallback of these and leave them for the future.

@zashroof
Copy link
Contributor Author

@zashroof first of all, great work! After resolving the comments of @georgesittas . You can check the following (if you have already a test for any of those and I missed it, you can skip it) :

  1. Add a test for databricks using ANALYZE TABLES IN .

Done

  1. Add a test for mysql using DROP HISTOGRAM.

Done

  1. Add a test for postgres using BUFFER_USAGE_LIMIT .

Done

  1. Add tests for starrocks using FULL , WITH N BUCKETS (for histogram) , and WITH SYNC/ASYNC MODE .

Done

  1. As I can understand for the oracle dialect, the syntax for things like here CLUSTER, validation_clauses, LIST_CHAINED_ROWS, DELETE isn't covered. You can use the exp.Command as a fallback of these and leave them for the future.

I tried to parse VALIDATE as a command to be the expression of the ANALYZE but couldn't get it to work. I think I don't understand how parse_string works...

sqlglot/generator.py Outdated Show resolved Hide resolved
sqlglot/generator.py Outdated Show resolved Hide resolved
sqlglot/parser.py Outdated Show resolved Hide resolved
sqlglot/parser.py Outdated Show resolved Hide resolved
sqlglot/parser.py Outdated Show resolved Hide resolved
@geooo109
Copy link
Collaborator

@zashroof Nice update.

Did you also check the rest of the oracle dialect? (e.g. LIST CHAINED ROWS, DELETE [ SYSTEM ] STATISTICS).

Check here:

  1. LIST CHAINED ROWS - DELETE
  2. VALIDATION_CLAUSES You working on that, but I mention it to have all the TODO things in one place.
  3. PARTITION_EXTENSION You cover the basic partition but there are more cases for the oracle dialect.

In the above cases, if the implementation is too complex, we mostly need the fallback mechanism ( treat the whole input as exp.Command ).

@zashroof
Copy link
Contributor Author

@geooo109 Added VALIDATE, LIST, DELETE clauses.
Not sure if I fallback correctly on parse_partition().

@zashroof
Copy link
Contributor Author

hehe github git operation is experiencing a major outage. Can't push my last commit.

@georgesittas
Copy link
Collaborator

georgesittas commented Jan 14, 2025

Thank you for the quick iterations @zashroof, we'll take another look soon :)

sqlglot/generator.py Outdated Show resolved Hide resolved
sqlglot/generator.py Outdated Show resolved Hide resolved
sqlglot/generator.py Outdated Show resolved Hide resolved
sqlglot/generator.py Outdated Show resolved Hide resolved
@georgesittas georgesittas merged commit e617d40 into tobymao:main Jan 14, 2025
7 checks passed
@georgesittas
Copy link
Collaborator

Appreciate the contribution @zashroof!

@zashroof
Copy link
Contributor Author

Thanks for bearing with me through the review!

@zashroof zashroof changed the title [WIP] Extend ANALYZE common syntax to cover multiple dialects feat: Extend ANALYZE common syntax to cover multiple dialects Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants