Skip to content

Conversation

@henryr
Copy link
Contributor

@henryr henryr commented Mar 5, 2018

…lValue

What changes were proposed in this pull request?

Parquet 1.9 will change the semantics of Statistics.isEmpty slightly
to reflect if the null value count has been set. That breaks a
timestamp interoperability test that cares only about whether there
are column values present in the statistics of a written file for an
INT96 column. Fix by using Statistics.hasNonNullValue instead.

How was this patch tested?

Unit tests continue to pass against Parquet 1.8, and also pass against
a Parquet build including PARQUET-1217.

…lValue

## What changes were proposed in this pull request?

Parquet 1.9 will change the semantics of Statistics.isEmpty slightly
to reflect if the null value count has been set. That breaks a
timestamp interoperability test that cares only about whether there
are column values present in the statistics of a written file for an
INT96 column. Fix by using Statistics.hasNonNullValue instead.

## How was this patch tested?

Unit tests continue to pass against Parquet 1.8, and also pass against
a Parquet build including PARQUET-1217.
@squito
Copy link
Contributor

squito commented Mar 5, 2018

lgtm

@henryr
Copy link
Contributor Author

henryr commented Mar 5, 2018

Retest this please

@henryr
Copy link
Contributor Author

henryr commented Mar 5, 2018

argh, wrong PR, sorry for retest-spam.

@SparkQA
Copy link

SparkQA commented Mar 6, 2018

Test build #87978 has finished for PR 20740 at commit b86c0be.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 6, 2018

Test build #87974 has finished for PR 20740 at commit b86c0be.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@vanzin
Copy link
Contributor

vanzin commented Mar 6, 2018

LGTM, merging to master.

@asfgit asfgit closed this in 8c5b34c Mar 6, 2018
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.

4 participants