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

Separate data types from generic keywords #677

Merged
merged 7 commits into from
Dec 1, 2023
Merged

Separate data types from generic keywords #677

merged 7 commits into from
Dec 1, 2023

Conversation

karlhorky
Copy link
Contributor

@karlhorky karlhorky commented Nov 29, 2023

Part of preparations for PR #673


Copied from my comment:

I would want to then do a separate PR to separate out all of the data type information in src/languages/*/*.keywords.ts in these flat arrays in into a separate (optional) export in each file called export const dataTypes, which would temporarily merged together with keywords to keep compatibility before reservedDataTypes in the next step is implemented

@nene
Copy link
Collaborator

nene commented Dec 1, 2023

I notice that data types are missing from db2 and singlestoredb dialects.

I'm not sure whether keywords like DAY, MONTH, HOUR should be considered data types. Afaik one can use these in expressions like extract() or when writing an interval literal, but one can't use them when declaring a type, like CREATE TABLE foo (duration HOUR) would not be valid.

The ARRAY keyword is trickier. It can be used in both data type declaration, but also when writing an ARRAY[1, 2, 3] literal. I think we already have some special handling of arrays, but my memory can also be wrong.

Anyway... These are just some general thoughts. Not blockers for merging this.

@karlhorky
Copy link
Contributor Author

I notice that data types are missing from db2 and singlestoredb dialects.

  1. Yeah originally I was going to do only a few dialects, but I ended up doing more. Probably worth it to do all of them now.

I'm not sure whether keywords like DAY, MONTH, HOUR should be considered data types

  1. I was also unsure here. It was mentioned in multiple docs pages, so I decided to go for it. But I don't feel strongly about this, so I'll revert this.

The ARRAY keyword is trickier. It can be used in both data type declaration, but also when writing an ARRAY[1, 2, 3] literal. I think we already have some special handling of arrays, but my memory can also be wrong.

  1. Right, since ARRAY is used in those positions and is more commonly considered a data type (across docs and also programming languages generally), I would suggest keeping ARRAY as is, if possible. If it's important to revert it, I can do this too.

I'll get started on 1 and 2

@nene
Copy link
Collaborator

nene commented Dec 1, 2023

With ARRAY it's more like I'm not sure either. Will need to do some more research and thinking.

@karlhorky
Copy link
Contributor Author

  1. Data types for db2, singlestore and transactsql have also be split out of the keywords 971fcd3
  2. SECOND, MINUTE, HOUR, DAY, WEEK, MONTH, YEAR keywords are moved back to keywords 03f6f9a
  3. I'll wait for your availability + decision on ARRAY

@nene
Copy link
Collaborator

nene commented Dec 1, 2023

Looks good. Let's keep the ARRAY in your way for now.

@nene nene merged commit d9db4ca into sql-formatter-org:master Dec 1, 2023
1 check passed
@karlhorky karlhorky deleted the separate-data-types branch December 1, 2023 16:30
@karlhorky
Copy link
Contributor Author

Thanks for the review and merge! I'll rebase #673

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.

2 participants