ARROW-11992: [Rust][Parquet] Add upgrade notes on 4.0 rename of LogicalType #9731
Conversation
alamb
changed the title
|
Thanks for opening a pull request! Could you open an issue for this pull request on JIRA? Then could you also rename pull request title in the following format? See also: |
rust/parquet/README.md
Outdated
There was a problem hiding this comment.
It's fine to add this temporarily in README.md but I think we should put them this alongside other breaking changes from 4.0 (seems there're quite a few of them) in a separate doc eventually. Something similar from Apache Spark.
There was a problem hiding this comment.
Summarizing breaking changes would definitely be valuable. I don't think I have the time to do so at this moment, but I am very supportive of the idea
There was a problem hiding this comment.
I'll add a guide as part of https://issues.apache.org/jira/browse/ARROW-12019, to provide examples of how one can start using LogicalType.
rust/parquet/README.md
Outdated
There was a problem hiding this comment.
nit: remove *Upgrade Note*? it looks redundant.
rust/parquet/src/basic.rs
Outdated
There was a problem hiding this comment.
I'm not sure if its useful to have this message here as well. Maybe the one in README is enough?
There was a problem hiding this comment.
I think it is important to put a hint as close to where people will hit the compiler error as possible (e.g. I wouldn't have known to look in the README when I hit the error on upgrade).
That being said, I think you are right that the actual content doesn't need to be duplicated here -- I will change it to point people at the readme for further details.
rust/parquet/README.md
Outdated
There was a problem hiding this comment.
nit: to align with the standard -> to align with the LogicalType introduced in Parquet Format 2.4.0 and up.
alamb
force-pushed
the
better-docs-on-rename
branch
from
1e27c5c to
487b631
Compare
3 participants
Rationale
While updating arrow deps in https://github.com/influxdata/influxdb_iox/pull/1003, I got (very) confused for a while with the parquet upgrade as
LogicalTypewas renamed toConvertedTypebut then a new type calledLogicalTypewas added in #9592.Changes
Add some comments to try and help future users save some time during upgrade