feat(parser): match Python SQLGlot data type parsing - #28
Draft
fc-anjos wants to merge 1 commit into
Draft
Conversation
Contributor
Author
|
Hello @yigalrozenberg! Looking for your input on this one, do you think this feature and API in general and the testing strategy in particular makes sense given the long term goals of the project, or should I adapt it to something else before marking it as ready for review? Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
parse_data_typeinterprets standalone type strings, including types supplied to schema-aware qualification and lineage. Real database schemas contain dialect-specific spellings that are not always recognized.For example,
character varyingis a standard PostgreSQL spelling forVARCHAR. On currentmaster, this call returns a parse error:A single type-parsing failure also prevents an FFI mapping schema from being constructed, so qualification and lineage cannot use an otherwise valid schema.
Currently,
sql-glot-rusthas no dedicated parser for standalone data types.parse_data_typedelegates to the type grammar inside the general SQL parser. That grammar handles a fixed set of common types, but it does not apply comprehensive dialect-specific type mappings. It also has no explicit fallback for qualified user-defined type names.Python SQLGlot already defines both behaviors through
DataType.from_str:The
dialectargument controls how SQL type names are interpreted. Theudt(user-defined type) argument controls what happens when the input is not a recognized built-in type. It defaults toFalse, which produces an error, whileTruepreserves a valid type name as a user-defined type.This PR introduces a dedicated standalone type parser that follows these semantics across the dialects shared with Python SQLGlot. Both Rust entry points take an explicit dialect.
parse_data_typeperforms strict parsing, whileparse_data_type_with_udtadditionally preserves an otherwise unrecognized valid type name as a user-defined type.Summary
DataType.from_str(..., udt:)DTypevariants with their canonical serialized namesUNKNOWNReference behavior
The parity fixture is generated from Python SQLGlot at commit
6b12cb8444816377fe7eb8b27d492fc117e3409b.It contains 3,681 cases across ANSI and the 29 named dialects shared by the Python and Rust implementations. Dialect support for user-defined types follows Python SQLGlot's definitions at that commit.
The fixture uses normalized JSON Lines with one metadata record followed by one case per line. Normal Rust tests remain self-contained and do not require Python.
The parity tool provides explicit commands for writing and verifying the fixture:
The
verifycommand regenerates the corpus from the supplied Python SQLGlot checkout and fails if it differs from the committed fixture. A separate Rust test records the user-defined type policy for every shared dialect.The fixture asserts the resulting canonical
DataTypeKindname, or that parsing fails. It does not claim complete AST parity for parameters, nested arguments, preserved spelling, or generated SQL.Public API
Python exposes user-defined type handling as an optional
udt=Falseargument onDataType.from_str. Rust does not support optional function arguments, and changing the existing two-argumentparse_data_typefunction would break its callers. The existing function therefore remains the strict form, equivalent to Python's default. Thewith_udtvariant always enables the fallback. Both entry points use the same parser and require an explicit dialect.For semantic schema input, the internal caller chooses whether to enable the fallback from the dialect's user-defined type policy.
Test strategy: open for discussion
This draft uses a committed fixture generated from a pinned Python SQLGlot checkout. Before proposing it upstream, I would appreciate feedback on what should constitute the long-term Rust contract.
cargo testenforcesStrategy 2 most closely follows the existing style in this repository, including the Rust-owned cases in
tests/test_transpile.rs. It would require expanding the focused tests in this draft before removing the generated fixture; the current representative cases are not an equivalent replacement.Strategy 3 adds broader drift discovery without turning every incidental Python classification into a permanent Rust API guarantee. It carries additional tooling and maintenance compared with strategy 2.
Strategy 1 provides the broadest immediate regression gate and requires no Python installation during normal Rust CI, but it commits a generated artifact and makes the pinned Python classifications normative for this surface.
Validation
cargo fmt --all -- --checkcargo test --features clipython scripts/python_type_parity.py verifyagainst the pinned Python SQLGlot checkoutcargo clippy --all-targets --features cli -- -D warningsstill reports warnings that are also present onmasterwith Rust 1.97. The files and ranges changed by this PR do not introduce additional clippy diagnostics.