-
Notifications
You must be signed in to change notification settings - Fork 59
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
feat(pg): create columns as array and matrices #185
Conversation
grammar.js
Outdated
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))), | ||
']' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
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()
?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Closes #183