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

Bug Fix: Allow GlueCatalog to create table with TimestampzType #366

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

HonahX
Copy link
Contributor

@HonahX HonahX commented Feb 5, 2024

Pyiceberg uses TimestampType for timestamp without time zone and TimestampzType for timestamp with time zone.

This PR adds the missing conversion from TimestampzType to glue type string "timestamp". Otherwise, table creation fails with a ValueError when generating StorageDescriptor

@@ -150,7 +152,7 @@ def primitive(self, primitive: PrimitiveType) -> str:
if isinstance(primitive, DecimalType):
return f"decimal({primitive.precision},{primitive.scale})"
if (primitive_type := type(primitive)) not in GLUE_PRIMITIVE_TYPES:
raise ValueError(f"Unknown primitive type: {primitive}")
return str(primitive_type.root)
Copy link
Contributor Author

@HonahX HonahX Feb 5, 2024

Choose a reason for hiding this comment

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

Since this is for the optional StorageDescriptor, I think it should not block the table creation even when encountering unknown primitive types

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

I agree @HonahX Thanks for fixing this 👍

@Fokko Fokko merged commit 40ab60a into apache:main Feb 5, 2024
6 checks passed
@Fokko Fokko added this to the PyIceberg 0.6.0 release milestone Feb 5, 2024
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.

2 participants