-
Notifications
You must be signed in to change notification settings - Fork 669
Complete PostgreSQL CREATE TYPE Support
#2094
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
Conversation
iffyio
left a comment
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.
Thanks @LucaCappelletti94! Changes look good to me overall, left some comments
| /// Range type: `CREATE TYPE name AS RANGE (options)` | ||
| Range { | ||
| options: Vec<UserDefinedTypeRangeOption>, | ||
| }, | ||
| /// Base type (SQL definition): `CREATE TYPE name (options)` | ||
| /// | ||
| /// Note the lack of `AS` keyword | ||
| SqlDefinition { | ||
| options: Vec<UserDefinedTypeSqlDefinitionOption>, | ||
| }, |
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.
Could we include a link to the postgres docs for the new variants?
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 wanted to, but they do not have a linkable subsection so the link would be to the same CREATE TYPE page. The sub-headers do not have the anchor link unfortunately.
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 I think in that case linking to the postgres create type page for each variant would suffice, most important that its clear where each variant comes from especially if the overlying statement/enum is shared between multiple dialects. Basically similar to what the Enum { ... } variant above documents?
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.
Ok, doing that.
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.
Added
| fn parse_range_option(&mut self) -> Result<UserDefinedTypeRangeOption, ParserError> { | ||
| let keyword = self.parse_one_of_keywords(&[ | ||
| Keyword::SUBTYPE, | ||
| Keyword::SUBTYPE_OPCLASS, |
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.
can we add tests for SUBTYPE_OPCLASS,SUBTYPE_DIFF and any other variants that are missing coverage?
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.
Oke
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.
Tentatively done, I think all are covered now.
Co-authored-by: Ifeanyi Ubah <ify1992@yahoo.com>
Co-authored-by: Ifeanyi Ubah <ify1992@yahoo.com>
Co-authored-by: Ifeanyi Ubah <ify1992@yahoo.com>
iffyio
left a comment
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.
LGTM! Thanks @LucaCappelletti94!
Adds full parsing for all PostgreSQL
CREATE TYPEstatement variants. Previously only composite and enum types were supported.What's New
Range Types - Parse range type definitions with 6 configuration options
Base Types - Parse low-level type definitions with 19 options (INPUT/OUTPUT functions, ALIGNMENT, STORAGE, etc.)
Simple Declarations - Parse type shells without representation
The AST extends
UserDefinedTypeRepresentationwith three new variants (Range,SqlDefinition,None) and adds five supporting enums (UserDefinedTypeRangeOption,UserDefinedTypeSqlDefinitionOption,Alignment,UserDefinedTypeStorage,UserDefinedTypeInternalLength) with comprehensive rustdoc. The parser refactorsparse_create_type()into modular functions (parse_create_type_composite(),parse_create_type_range(), and base type option parsers) that handle all PostgreSQL type configuration syntax. Tests provide comprehensive coverage with explicit AST verification for all variants and edge cases. Added 17 PostgreSQL-specific keywords, necessary to parse the keywords used in the type options (ALIGNMENT, CANONICAL, INTERNALLENGTH, PASSEDBYVALUE, RECEIVE, SEND, STORAGE, SUBTYPE, TYPMOD_IN, etc.).Closes issue #2092