RussellSpitzer commented on pull request #3933: URL: https://github.com/apache/iceberg/pull/3933#issuecomment-1020123274
> @RussellSpitzer Sorry for the late reply. Yes, the fix is mostly for MigrationTableUtils. It is for this path `DataFiles.withPartitionPath` -> `DataFiles.fillFromPath` -> `Conversions.fromPartitionString`. > > After I took a look at `PartitionSpec`, I feel that timestamp is intentionally blocked in `Conversions.fromPartitionString`, because in `PartitionSpec Builder`, it has year, month, day, time, but no timestamp. I guess timestamp as a partition column is not supported because it doesn't have any real usages. However, creating table using timestamp as a partition column `CREATE TABLE test (id Integer, name String, dept String, ts Timestamp) USING iceberg PARTITIONED BY (ts)` can create the table successfully, but users don't know that the partition column `ts` is not working, so I feel that we probably should support partitioned by timestamp. If not, we probably should block creating table partitioned by timestamp. Yep makes sense to me! Yufei and I were just discussing another bug which we thought might be related (it was not). I think this is fine to fix. I just want to make sure our string parsing is correct, from what I could tell the string that is generated for "timestamp" should be environment dependent since in the Hive code it just uses "value.toString". If we are confident that this covers most use cases I'm ok with merging. -- 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]
