Map UUID data type to sqlalchemy.sql.sqltypes.UUID for reflection#682
Conversation
Signed-off-by: Edgar Ramírez Mondragón <edgarrm358@gmail.com>
Signed-off-by: Edgar Ramírez Mondragón <edgarrm358@gmail.com>
sqlalchemy.sql.sqltypes.UUID for reflection
- Guard `sqltypes.Uuid` usage in `_get_type_kwargs` with `IS_VERSION_20`
to avoid `AttributeError` on SA 1.4
- Conditionally add `"UUID"` to `ischema_names` on SA 2.x; remove
`_sa_uuid` from module namespace after use to prevent
`test_types_in_snowdialect` from detecting a spurious class
- Export `UUID` from `snowflake.sqlalchemy` on SA 2.x; add `_sa20_types`
to `__all__`
- Replace relative `..custom_types` import with absolute
`snowflake.sqlalchemy.custom_types`; import `IS_VERSION_20` from
`snowflake.sqlalchemy.compat` instead of reinventing the flag
- Remove `("UUID", "UUID")` from the unguarded parametrize list in
`test_unit_structured_types`; add four `@pytest.mark.feature_v20` tests
covering `parse_type`, `ischema_names`, `_get_type_kwargs`, and
`_resolve_column_type`
- Add `_SA20_ONLY_TYPES` to `test_unit_types` so `test_type_baseline`
passes on both SA versions
- Document UUID support in README and DESCRIPTION
sfc-gh-mraba
left a comment
There was a problem hiding this comment.
Hi,
I found 2 things that required attention and are addressed in last commit:
- SQLAlchemy 1.4 Incompatibility
sqlalchemy.sql.sqltypes.UUIDdoes not exist in SQLAlchemy 1.4.sqltypes.Uuid(the generic emulated type) was introduced in SA 2.0.sqltypes.UUID(the native variant) was also introduced in SA 2.0.
Consequence:
from sqlalchemy.sql.sqltypes import UUIDincustom_type_parser.py→ImportErroron SA 1.4.issubclass(col_type, sqltypes.Uuid)insnowdialect.py→AttributeError: module has no attribute 'Uuid'on SA 1.4.
The support for UUID is limited only to SA2.
parse_type()doesn't handleUUIDkwargs
parse_type() in custom_type_parser.py maps "UUID" to the UUID class via ischema_names, but the parse_type function only knows how to pass kwargs for Numeric, String/BINARY, MAP, OBJECT, ARRAY, and VECTOR types. For UUID, it calls UUID() with no args — which is fine (as_uuid=True by default in SA 2.x), but is inconsistent with the _get_type_kwargs approach in snowdialect.py that forces as_uuid=False.
I chose to go with as_uuid=False (WDYT?) as the driver treats these values as string.
I agree with that for consistency 👍. And your doc update shows users how to get
Thanks for pushing those changes! I'd completely missed that this dialect package is still compatible with 1.4. |
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes Column of type UUID reflected as
NullType#681Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
There are 2 commits here:
sqlalchemy.sql.sqltypes.UUIDfor reflection: The current behavior is to returnNullTypeuuid.UUIDinstances: This can be dropped if the maintainers think it's valuable to represent the UUID values asuuid.UUIDinstances rather than rawstrs.