You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "joellubi (via GitHub)" <gi...@apache.org> on 2024/01/06 23:47:11 UTC
[I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
joellubi opened a new issue, #39489:
URL: https://github.com/apache/arrow/issues/39489
### Describe the bug, including details regarding any error messages, version, and platform.
The Parquet format [document](https://github.com/apache/parquet-format/blob/eb4b31c1d64a01088d02a2f9aefc6c17c54cc6fc/LogicalTypes.md?plain=1#L480-L485) indicates that converted types should have `isAdjustedToUtc=true` for backward compatibility, but the current C++ [implementation](https://github.com/apache/arrow/blob/37a8bf04bc713858a5b247d4424c1e8505e61947/cpp/src/parquet/arrow/schema_internal.cc#L92-L93) sets this to `false` for converted types.
The [Rust implementation](https://github.com/apache/arrow-rs/blob/a61e824abdd7b38ea214828480430ff2a13f2ead/parquet/src/arrow/schema/primitive.rs#L211-L239) already follows the spec as described and the Go implementation has #39467 open to correct this.
### Component(s)
C++, Parquet
--
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: issues-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1884403856
Actually, my comment above was wrong:
> when converting a TimestampLogicalType to an Arrow timestamp type, we do ignore the `isAdjustedToUtc` flag if the logical type has been annotated to come from a converted type. So then changing `isAdjustedToUtc` in those cases shouldn't actually impact any behaviour in the Arrow API layer of the Parquet library?
This is exactly what your PR changed, i.e. it's the conversion from a Parquet type to Arrow type (`MakeArrowTimestamp`) that now _no longer_ ignores `isAdjustedToUtc` for converted types.
So of course this does change behaviour, also for the Arrow APIs.
And so the consequence of this is that reading an older file written by Arrow, will now be read differently (and this was a compatibility that was explicitly added in https://github.com/apache/arrow/pull/4856).
It's maybe OK to change this behaviour, so we give up faithful roundtrip for those older files written by Arrow (in the end, it are old files), in favor of better compatibility with the Parquet spec and other producers, but we need to be explicit about that this is actually a behaviour change for users.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1879966745
Thanks for point this out!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1884421268
Yeah.
```
<pyarrow._parquet.ParquetSchema object at 0x7ff21be310c0>
required group field_id=-1 schema {
optional int64 field_id=-1 col (Timestamp(isAdjustedToUTC=true, timeUnit=microseconds, is_from_converted_type=true, force_set_converted_type=false));
}
```
Hmmm this regard `isAdjustedToUTC=true`, however, the col is `col: timestamp[us]`.
Do you think we need an extra config, or just changing this?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1888764091
Comment from @pitrou on the PR at https://github.com/apache/arrow/pull/39491#issuecomment-1884825697:
> > Nowadays it is maybe OK to make a different trade-off, and to prioritize the Parquet spec / non-Arrow legacy files above compatibility of reading Arrow legacy files, given that it is about files written by an Arrow version of several years old (Arrow < 0.15).
>
> If we really care about this, we should perhaps detect the offending Arrow version, since the Parquet file records the producer name and version.
We could indeed infer if the file was written by Arrow (the version shouldn't matter, I think, because in the end we care about whether the LogicalType is present or not, which is always the case starting from a certain version).
I don't know if that wouldn't make things more complex / confusing for users though, that a similar file (just by a different producer) would give different results.
> But we would also need an actual file to test with.
Above (https://github.com/apache/arrow/issues/39489#issuecomment-1884416205) I posted a small example, and updated that comment to attach the generated file that I used.
We do actually have a file in the pyarrow tests that covers this ([python/pyarrow/tests/data/parquet/v0.7.1.column-metadata-handling.parquet](https://github.com/apache/arrow/blob/main/python/pyarrow/tests/data/parquet/v0.7.1.column-metadata-handling.parquet)), but the test didn't catch that, because it only tested roundtrip to/from pandas, and for pandas it was using additional _pandas_ metadata we did already store in the schema at the time, and so that still correctly roundtrips.
But, yes, in general we have a lack of testing for Parquet on this front: https://github.com/apache/arrow/issues/22325
---
> Would you be in favor of adding a field to `ArrowReaderProperties` to control this behavior?
To be honest, I am not sure what would be the best option. Add a way to control the behaviour of course make this more flexible for the user/library using arrow, but 1) is it still needed to add this given it is for legacy files? (I don't know how many producers still generate them), and 2) we still need to make a choice of a default in the Arrow libraries.
Assume we would keep the default in Arrow as it was before, do you have applications that would set it differently? (maybe yes, as that might have triggered this discussion?)
And again: this whole change is maybe fine nowadays. I mostly wanted to point out this was _not_ a bug, but a deliberate design decision in the past, and if we change this, we should do it deliberately and flag it as a breaking change.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "joellubi (via GitHub)" <gi...@apache.org>.
joellubi commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1884672151
Thank you for adding those details @jorisvandenbossche, it really helped clarify the context of the previous decision. In thinking about this, it seems there may actually be two separate but related goals for the conversion:
1. Preservation of the original Parquet semantics when converting to Arrow
2. Preservation of the original Arrow type when a roundtrip occurs through Parquet and back to Arrow
Since the legacy Parquet types cannot distinguish between instant and local semantics, consumers have had to make an arbitrary choice between the two as you described. It does seem, at least now, that the Parquet spec has an explicit position that the deprecated convertedType timestamps always had instant semantics, and that local semantics were unsupported ([source](https://github.com/apache/parquet-format/blob/eb4b31c1d64a01088d02a2f9aefc6c17c54cc6fc/LogicalTypes.md#deprecated-timestamp-convertedtype)). This helps make the current position clear but doesn't change the reality of real-world Parquet usage that has not followed these semantics.
In the case that a Parquet file is being read and we don't know (or care) how it was produced, I do think it makes sense to follow compatibility guidelines (as the recently-merged PR has). This does break roundtrip behavior (thank you for pointing this out), but I do think Arrow already has an independent solution to this case. The metadata associated with the `ARROW:schema` key _should_ be able to preserve this information on roundtrip, and I was actually surprised that it didn't in your example. I found [this](https://github.com/apache/arrow/blob/72ed58449ea71aab1343d9adce19f177f20705cf/cpp/src/parquet/arrow/schema.cc#L922-L933) relevant code for how we currently handle the restoration of the original type. It seems that we will restore a particular timezone if it was present in the original schema, but _do not_ remove the timezone if one was not present. Perhaps by independently updating this logic we can get the desired behavior for both scenarios.
Desired behavior IMO:
```python
>>> pq.read_metadata("test_timestamp_convertedtype_writer_unknown.parquet").schema.to_arrow_schema()
col: timestamp[us, tz=UTC]
>>> pq.read_metadata("test_timestamp_convertedtype_written_by_arrow_no_tz.parquet").schema.to_arrow_schema()
col: timestamp[us]
>>> pq.read_metadata("test_timestamp_convertedtype_written_by_arrow_with_tz.parquet").schema.to_arrow_schema()
col: timestamp[us, tz=UTC]
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU closed issue #39489: [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType
URL: https://github.com/apache/arrow/issues/39489
--
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: issues-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU closed issue #39489: [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType
URL: https://github.com/apache/arrow/issues/39489
--
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: issues-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1884471274
> Hmmm this regard `isAdjustedToUTC=true`, however, the col is `col: timestamp[us]`. So previously I think it's a bug :-(
As I tried to explain: no, it's not a bug, it was an explicit design decision (the links I posted above, https://github.com/apache/arrow/issues/22303 / https://github.com/apache/arrow/pull/4856).
When creating the ParquetSchema from parsing the Parquet file/thrift, we did set `isAdjustedToUTC=true`, but we also added a `is_from_converted_type=true` (if it was created from a legacy file with ConvertedTypes). So the ParquetSchema follows the Parquet spec, but the on conversion of this ParsuetSchema to an ArrowSchema, we explicitly had logic to ignore `isAdjustedToUTC` when ``is_from_converted_type=true`` (that's the line of code you changed in the PR).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-2094686330
Re-close this
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1884476349
Ah I got your meaning, however, does this means the legacy file cannot have any timezone?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1884485534
> does this means the legacy file cannot have any timezone?
Yes, reading such a legacy file would essentially never give you a timezone (but that was a generic limitation of the Parquet spec, not something Arrow could do anything about). Essentially back then the choice needed to be made between "always returning no timezone" (and potentially losing some information, if the original data was tz-aware) or "always returning with timezone" (and potentially adding _wrong_ information, if the original data was not in UTC).
And so we made what we thought to be the "safest" choice that losing some information is better than adding wrong information.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1884714178
Guess a explicit cast in required in this scenerio...
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1891248917
>
For what it's worth, I do not. I was having issues in an application using the Go Parquet implementation which had a bug different from the logic here. As part of updating the Go implementation, I referred to the Parquet format document as well as a few other existing implementations which is when I noticed this inconsistency here.
Anyway, I think I can document the behavior here.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "joellubi (via GitHub)" <gi...@apache.org>.
joellubi commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1884789666
> This additional ARROW:schema metadata was only added around the same time that we started to support LogicalType (around Arrow 0.14.0), so unfortunately it doesn't help for this use case.
I see, indeed unfortunate. Would you be in favor of adding a field to `ArrowReaderProperties` to control this behavior?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1879993475
@joellubi I found this is only called in the code path below:
```
FromInt64Statistics/GetArrowType
- FromInt64
-- MakeArrowTimestamp
```
I guess this will not frequently call in code path, because:
1. `FromInt64Statistics` is used for filtering, and this might making filtering invalid
2. `GetArrowType` is harmful, but if the type is annotated, it might not use this?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1880005024
Some related discussions:
* https://github.com/apache/arrow/pull/4825
* https://github.com/apache/arrow/issues/22303
* https://github.com/apache/arrow/pull/4856
Now I suppose that because of those mentioned issues, when converting a TimestampLogicalType to an Arrow timestamp type, we do ignore the `isAdjustedToUtc` flag if the logical type has been annotated to come from a converted type. So then changing `isAdjustedToUtc` in those cases shouldn't actually impact any behaviour in the Arrow API layer of the Parquet library?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1884416205
To make it concrete: assume I have a file written with pyarrow 11.0 (I don't know exactly at which version number we started supporting LogicalType, but I had this version available, and here we still only write ConvertedType), for data in Arrow with tz-naive timestamps:
```python
>>> import pyarrow as pa
>>> import pyarrow.parquet as pq
>>> pa.__version__
'0.11.1'
>>> table = pa.Table.from_arrays([pa.array([1, 2, 3], pa.timestamp("us"))], ['col'])
>>> pq.write_table(table, "test_timestamp_pa011.parquet")
```
and then we read it with latest released pyarrow, we faithfully roundtrip the Arrow data:
```python
>>> pa.__version__
'14.0.1'
>>> pq.read_metadata("test_timestamp_pa011.parquet").schema
<pyarrow._parquet.ParquetSchema object at 0x7ff21be310c0>
required group field_id=-1 schema {
optional int64 field_id=-1 col (Timestamp(isAdjustedToUTC=true, timeUnit=microseconds, is_from_converted_type=true, force_set_converted_type=false));
}
>>> pq.read_metadata("test_timestamp_pa011.parquet").schema.to_arrow_schema()
col: timestamp[us]
>>> pq.read_table("test_timestamp_pa011.parquet")
pyarrow.Table
col: timestamp[us]
----
col: [[1970-01-01 00:00:00.000001,1970-01-01 00:00:00.000002,1970-01-01 00:00:00.000003]]
```
But now with latest main (after https://github.com/apache/arrow/pull/39491), the read data is no longer the same as the original data (the type now says that the data is in UTC, while the original data were local timestamps in an unknown timezone):
```python
>>> pa.__version__
'15.0.0.dev402+gc752bdb08'
>>> pq.read_metadata("test_timestamp_pa011.parquet").schema
<pyarrow._parquet.ParquetSchema object at 0x7f6969c44c80>
required group field_id=-1 schema {
optional int64 field_id=-1 col (Timestamp(isAdjustedToUTC=true, timeUnit=microseconds, is_from_converted_type=true, force_set_converted_type=false));
}
>>> pq.read_metadata("test_timestamp_pa011.parquet").schema.to_arrow_schema()
col: timestamp[us, tz=UTC]
>>> pq.read_table("test_timestamp_pa011.parquet")
pyarrow.Table
col: timestamp[us, tz=UTC]
----
col: [[1970-01-01 00:00:00.000001Z,1970-01-01 00:00:00.000002Z,1970-01-01 00:00:00.000003Z]]
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "joellubi (via GitHub)" <gi...@apache.org>.
joellubi commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1888869766
> I don't know if that wouldn't make things more complex / confusing for users though, that a similar file (just by a different producer) would give different results.
I agree this could be confusing. For example datalakes are often written to by many potentially different producers so inconsistencies when reading could surface in this scenario.
> To be honest, I am not sure what would be the best option. Add a way to control the behaviour of course make this more flexible for the user/library using arrow, but 1) is it still needed to add this given it is for legacy files? (I don't know how many producers still generate them), and 2) we still need to make a choice of a default in the Arrow libraries.
I think that an option to control the behavior would be valuable, but a default that doesn't prompt users to update their files or application logic probably isn't helpful. Do we use warnings or any other ways deprecating certain usage in other parts of Arrow?
> Assume we would keep the default in Arrow as it was before, do you have applications that would set it differently? (maybe yes, as that might have triggered this discussion?)
For what it's worth, I do not. I was having issues in an application using the Go Parquet implementation which had a bug different from the logic here. As part of updating the Go implementation, I referred to the Parquet format document as well as a few other existing implementations which is when I noticed this inconsistency here.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1884328105
> Now I suppose that because of those mentioned issues, when converting a TimestampLogicalType to an Arrow timestamp type, we do ignore the isAdjustedToUtc flag if the logical type has been annotated to come from a converted type. So then changing isAdjustedToUtc in those cases shouldn't actually impact any behaviour in the Arrow API layer of the Parquet library?
@jorisvandenbossche Yes. As I mentioned, this might only happens when:
1. Filtering ( Statistics might got the wrong time )
2. Read a legacy file in parquet-mr
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1884405945
Yes, we can document it before we releasing 16.0
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1884491281
I'll draft another patch which adding these in doc, and revert the change there `docs/source/cpp/parquet.rst`
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Timestamp conversion from Parquet to Arrow does not follow compatibility guidelines for convertedType [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on issue #39489:
URL: https://github.com/apache/arrow/issues/39489#issuecomment-1884730021
> I do think Arrow already has an independent solution to this case. The metadata associated with the `ARROW:schema` key _should_ be able to preserve this information on roundtrip, and I was actually surprised that it didn't in your example.
This additional `ARROW:schema` metadata was only added around the same time that we started to support LogicalType (around Arrow 0.14.0), so unfortunately it doesn't help for this use case. When it is present, it indeed allows to preserve the actual timezone (also for logical types, which otherwise only knows if it is UTC or not).
But so when `ARROW:schema` is present, the file thus also has LogicalType annotations, and this whole discussion of course only applies to older files that only have ConvertedTypes.
So while I certainly agree with your "desired behaviour" being desired, it's not possible to distinguish with or without tz (even for files written by Arrow), because the only information there is in the file is the ConvertedType.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org