paimon icon indicating copy to clipboard operation
paimon copied to clipboard

[core]IcebergConversions addType for Timestamp and Time

Open dbac opened this issue 1 year ago • 11 comments

Purpose

Linked issue: close #3788

IcebergConversions addType for Timestamp and Time

Tests

IcebergDataTypeCompatibilityTest

API and Format

Documentation

dbac avatar Aug 01 '24 05:08 dbac

PostgresSyncTableActionITCase.testOptionsChange » Timeout testOptionsChange()
This timeout check failed, please tell me how to re-check, thank you very much @tsreaper

dbac avatar Aug 05 '24 15:08 dbac

@tsreaper please Review code again,thanks

dbac avatar Aug 07 '24 01:08 dbac

@tsreaper Could you please help me take a look when you have time? Thank you

dbac avatar Aug 09 '24 04:08 dbac

@tsreaper I am reading the source code now and solved this problem. Please review it for me.

dbac avatar Aug 27 '24 12:08 dbac

I found that a lot of the core code was written by you. I will learn from you. I think the source code should make me familiar with paimon quickly. I hope this PR will be submitted as soon as possible. I will continue to contribute to the next one.

dbac avatar Aug 27 '24 12:08 dbac

Hi @dbac , sorry for keeping you waiting. I've just introduced statistics to IcebergDataFileMeta, so now Iceberg will filter files according to the statistics we converted from IcebergConversions, and we can test the conversion reliably.

Please resolve the conflicts and add tests about timestamp and time to IcebergCompatibilityTest#testAllTypeStatistics. More specifically, do not add new test method. Just add a few columns with timestamp and time type to that test.

tsreaper avatar Aug 29 '24 07:08 tsreaper

@tsreaper image time only support 3 pecesion (so, ms ), and use int writer.

so IcebergConversions time value should int not long.

dbac avatar Aug 29 '24 09:08 dbac

image

dbac avatar Aug 29 '24 09:08 dbac

@tsreaper image Is it a bug? Lessthan invalid, greaterthan ok ? Is there a problem with writing iceberg data?

dbac avatar Aug 30 '24 11:08 dbac

https://github.com/apache/iceberg/issues/9431

dbac avatar Aug 30 '24 11:08 dbac

@tsreaper ? Could you help me review it again?

dbac avatar Sep 06 '24 01:09 dbac

Thanks @dbac for the contribution. @tsreaper has created another version #4318 to solve this issue. Close this PR now.

JingsongLi avatar Oct 15 '24 06:10 JingsongLi