Skip to content

Update to Arrow-5.0.0#214

Merged
GPSnoopy merged 7 commits intomasterfrom
Arrow-5.0.0
Sep 2, 2021
Merged

Update to Arrow-5.0.0#214
GPSnoopy merged 7 commits intomasterfrom
Arrow-5.0.0

Conversation

@GPSnoopy
Copy link
Contributor

No description provided.

GPSnoopy added 3 commits July 30, 2021 10:27
Review Resharper warnings (fix memory leak in ColumPath).
@GPSnoopy GPSnoopy requested a review from jgiannuzzi August 31, 2021 14:34
static_assert(Repetition::REQUIRED == 0);
static_assert(Repetition::OPTIONAL == 1);
static_assert(Repetition::REPEATED == 2);
static_assert(Repetition::UNDEFINED == 3);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whilst we're at it, we should add Encoding::UNKNOWN with value 999.
See https://github.com/apache/arrow/blob/apache-arrow-5.0.0/cpp/src/parquet/types.h#L478

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left it out since we currently do not expose Encoding.Unknown in C#. Thus no risk of ABI breaks here.

Also, a bit of a WTF here. What's the difference from Arrow's point of view between Encoding::UNDEFINED and Encoding::UNKNOWN ?

Copy link
Member

@jgiannuzzi jgiannuzzi Sep 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I get your point about ABI breaks. Why do we start exposing Repetition.Undefined in C# now though?

Also, no idea about the difference between Encoding::UNDEFINED and Encoding::UNKNOWN either. It sounds as WTF to me as to you 🤔

@GPSnoopy GPSnoopy merged commit d7f5c40 into master Sep 2, 2021
@GPSnoopy GPSnoopy deleted the Arrow-5.0.0 branch September 2, 2021 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants