-
Notifications
You must be signed in to change notification settings - Fork 70
feat: define table properties with default values #212
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
6723d40 to
30b86fe
Compare
Just copied everything from the TableProperties.java as of today
| "write.delete.avro.compression-level", ""}; | ||
|
|
||
| // ORC properties | ||
| inline static Entry<int64_t> kOrcStripeSizeBytes{"write.orc.stripe-size-bytes", |
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.
Why are we going with int64 here, and with int32 in Parquet? Int32 seems to be sufficient?
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.
These align with the Java definitions.
src/iceberg/table_properties.h
Outdated
| inline static Entry<bool> kSnapshotIdInheritanceEnabled{ | ||
| "compatibility.snapshot-id-inheritance.enabled", false}; |
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 would not backport this part. It was used before V2 was released to test out the snapshot inheritance, but I don't think anyone is really using it since upgrading to V2 is easier.
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.
Just removed it. Thanks for the information!
Fokko
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.
Left two minor comments, but apart from that this looks good to me 👍
|
Nice one, thanks for adding this @wgtmac 🙌 |
Just copied everything from the TableProperties.java as of today