adjust / parquet_fdw

Parquet foreign data wrapper for PostgreSQL
PostgreSQL License
333 stars 37 forks source link

Add support for Time64 #23

Open remilapeyre opened 3 years ago

remilapeyre commented 3 years ago

Signed-off-by: Rémi Lapeyre remi.lapeyre@lenstra.fr

zilder commented 3 years ago

Hi @remilapeyre,

thanks for your contribution! At first glance looks good. I'll do a more thorough review soon and will merge it then.

remilapeyre commented 3 years ago

Thanks @zilder, I tried to look carefully to find whether there was something special to do to handle timezones and time resolution (in the Parquet file or the PostgreSQL database) and after reading PostgreSQL source code I don't think it needs special treatment but I'm not sure about that.

zilder commented 3 years ago

Hi @remilapeyre,

As i understood parquet's time and timestamp types do not contain information on the timezone. They have isAdjustedToUTC boolean parameter, but without actual timezone information there is not much can be done about it.

One thing that wasn't taken into account in this pull request is time precision. Proposed patch implies that time is in microseconds. Though time64 type may also represent nanoseconds. Other than that the patch looks good.

remilapeyre commented 3 years ago

Hi thanks for the review, I had not the time to look at the nanoseconds yet but I will try to do it in the coming days.

mkgrgis commented 1 year ago

@remilapeyre , any activity here?