I'm +1 on the change for the Rust side as well. It probably won't be as disruptive as the C++ side.
On Wed, May 29, 2019 at 7:09 AM Wes McKinney <wesmck...@gmail.com> wrote: > I'm in favor of making the change -- it's slightly disruptive for > library-users, but the fix is no more complicated than a > search-and-replace. When the C++ project was started, the LogicalType > union didn't exist and "LogicalType" seemed like a more appropriate > name for ConvertedType. > > On Wed, May 29, 2019 at 7:11 AM Wes McKinney <wesmck...@gmail.com> wrote: > > > > You all probably want to join d...@parquet.apache.org and have the > > discussion there. From a governance perspective that's where we need > > to talk about making breaking changes to the Parquet C++ library > > > > LogicalType was introduced into the Parquet format in October 2017 to > > be a more flexible and future-proof replacement for the original > > ConvertedType metadata, see > > > > > https://github.com/apache/parquet-format/commit/863875e0be3237c6aa4ed71733d54c91a51deabe > > > > Support and forward/backwards compatibility for the new LogicalType > > union was just developed in PARQUET-1411 > > > > > https://github.com/apache/arrow/commit/38b1ddfb7f5def825ac57c8f27ffe5afa7fcb483 > > > > On Wed, May 29, 2019 at 4:44 AM Joris Van den Bossche > > <jorisvandenboss...@gmail.com> wrote: > > > > > > Yes, the LogicalType is newer than ConvertedType in the parquet > format, and > > > was until recently not implemented in parquet-cpp. > > > The problem is that originally, the parquet thrift::ConvertedType was > > > implemented in parquet-cpp as LogicalType. Now, support is added in > > > parquet-cpp for this newer parquet thrift::LogicalType, but the obvious > > > name for that in parquet-cpp was already taken. Therefore, it was > added as > > > parquet::LogicalAnnotation. See this PR for context: > > > https://github.com/apache/arrow/pull/4185 > > > > > > So Deepak's question is if we can rename parquet-cpp's > parquet::LogicalType > > > to parquet::ConvertedType (to match the thrift format naming), so we > can > > > actually use the logical name parquet::LogicalType instead of > > > parquet::LogicalAnnotation for the new implementation. > > > And to avoid the confusion we are having here .. > > > > > > But renaming like that would be hard break in parquet-cpp for libraries > > > depending on that, though. But I can't really assess the impact of > that. > > > > > > Best, > > > Joris > > > > > > Op wo 29 mei 2019 om 11:04 schreef Antoine Pitrou <anto...@python.org > >: > > > > > > > > > > > Le 29/05/2019 à 10:47, Deepak Majeti a écrit : > > > > > "ConvertedType" term is used by the parquet specification below. > This > > > > type > > > > > is used to map client data types to Parquet types. > > > > > > > > > > https://github.com/apache/parquet-format/blob/master/src/main/thrift/parquet.thrift#L48 > > > > > > > > But apparently there's also "LogicalType": > > > > > > > > > https://github.com/apache/parquet-format/blob/master/src/main/thrift/parquet.thrift#L315 > > > > > > > > "LogicalType annotations to replace ConvertedType" > > > > > > > > Regards > > > > > > > > Antoine. > > > > >
