jun-he commented 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 can apply transform function 
to a python data and then return it in python. 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]

Reply via email to