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(pg): create columns as array and matrices #185

Merged
merged 3 commits into from
Jul 29, 2023

Conversation

matthias-Q
Copy link
Collaborator

Closes #183

grammar.js Outdated
Comment on lines 448 to 457
choice(
seq(optional($.keyword_array), $._array_size_definition),
seq( $._array_size_definition, $._array_size_definition),
$.keyword_array,
),
),
_array_size_definition: $ => seq(
'[',
optional(field("size", alias($._integer, $.literal))),
']'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
choice(
seq(optional($.keyword_array), $._array_size_definition),
seq( $._array_size_definition, $._array_size_definition),
$.keyword_array,
),
),
_array_size_definition: $ => seq(
'[',
optional(field("size", alias($._integer, $.literal))),
']'
seq(
optional($.keyword_array),
repeat(
seq(
'[',
optional(field("size", alias($._integer, $.literal))),
']'
)
)
),

Postgres allows n-dimensional arrays > 2; repeat is 0 or more times so this should cover all possibilities

Copy link
Collaborator Author

@matthias-Q matthias-Q Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the ARRAY keyword works only for 1D arrays
shouldn't it be repeat1() instead of repeat()?

https://www.postgresql.org/docs/current/arrays.html

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have pushed a version, which allows for more dimensions. Unfortunately, it requires a conflict rule.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repeat would allow array, array[n], array[n][n].... and cover all three at once (including the zero-repeats case of just the keyword), but it is more permissive than necessary if the keyword is only used with 1d arrays. I'm fine either way though.

if you replace the top and bottom options in your latest with seq($.keyword_array, optional($._array_size_definition)) do you still need the conflict rule?

Copy link
Collaborator Author

@matthias-Q matthias-Q Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, this resolves the conflict.

I have pushed it to my branch, but for some reason it does not show up in the PR 😕

Now it is there.

@matthias-Q matthias-Q merged commit 530d18e into DerekStride:main Jul 29, 2023
4 checks passed
@matthias-Q matthias-Q deleted the pg_create_array_matrix branch July 29, 2023 14:43
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.

PG: Create Table with Array types
2 participants