jun-he edited a comment on pull request #3601: URL: https://github.com/apache/iceberg/pull/3601#issuecomment-989120897
Thanks @CircArgs for this change, which improves the current implementation using some powerful python features. In addition to the open question about the possible extra performance cost, another concern I have is the impact on other classes. As this change is a fundamental one, it ​will bring some changes to other classes depending on it, e.g. the transform will then be implemented in a similar way using generic_class defined in types (based on [the PR](https://github.com/jun-he/incubator-iceberg/pull/4)). For example, ```Truncate[Decimal[9, 2], 10](12.34)``` will create an iceberg Decimal class instance. This will add extra cost to process the data. In the current way, we create a Truncate instance, which will be re-used to apply a transform function to a python value and then return it in python directly. So there is no iceberg type instances in the middle. Currently, the iceberg literals are types are separated. literals are mainly used for data, e.g. used expressions, and types are mainly used in metadata, e.g. transform, partition spec. This way enables us to improve the performance or shortcut the transformation when it is needed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
