Skip to content

PARQUET-1386: Fix issues of NaN and +-0.0 in case of float/double column indexes #515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gszadovszky merged 1 commit into from
Aug 23, 2018
Merged

PARQUET-1386: Fix issues of NaN and +-0.0 in case of float/double column indexes #515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ...lumn/src/main/java/org/apache/parquet/internal/column/columnindex/ColumnIndexBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,10 @@ private ColumnIndexBase<?> build(PrimitiveType type) {
return null;
}
ColumnIndexBase<?> columnIndex = createColumnIndex(type);
if (columnIndex == null) {
// Might happen if the specialized builder discovers invalid min/max values
return null;
}
columnIndex.nullPages = nullPages.toBooleanArray();
// Null counts is optional so keep it null if the builder has no values
if (!nullCounts.isEmpty()) {
Expand Down
23 changes: 21 additions & 2 deletions ...rc/main/java/org/apache/parquet/internal/column/columnindex/DoubleColumnIndexBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ int compareValueToMax(int arrayIndex) {

private final DoubleList minValues = new DoubleArrayList();
private final DoubleList maxValues = new DoubleArrayList();
private boolean invalid;

private static double convert(ByteBuffer buffer) {
return buffer.order(LITTLE_ENDIAN).getDouble(0);
Expand All @@ -101,12 +102,30 @@ void addMinMaxFromBytes(ByteBuffer min, ByteBuffer max) {

@Override
void addMinMax(Object min, Object max) {
minValues.add((double) min);
maxValues.add((double) max);
double dMin = (double) min;
double dMax = (double) max;
if (Double.isNaN(dMin) || Double.isNaN(dMax)) {
// Invalidate this column index in case of NaN as the sorting order of values is undefined for this case
invalid = true;
}

// Sorting order is undefined for -0.0 so let min = -0.0 and max = +0.0 to ensure that no 0.0 values are skipped
if (Double.compare(dMin, +0.0) == 0) {
dMin = -0.0;
}
if (Double.compare(dMax, -0.0) == 0) {
dMax = +0.0;
}

minValues.add(dMin);
maxValues.add(dMax);
}

@Override
ColumnIndexBase<Double> createColumnIndex(PrimitiveType type) {
if (invalid) {
return null;
}
DoubleColumnIndex columnIndex = new DoubleColumnIndex(type);
columnIndex.minValues = minValues.toDoubleArray();
columnIndex.maxValues = maxValues.toDoubleArray();
Expand Down
23 changes: 21 additions & 2 deletions ...src/main/java/org/apache/parquet/internal/column/columnindex/FloatColumnIndexBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ int compareValueToMax(int arrayIndex) {

private final FloatList minValues = new FloatArrayList();
private final FloatList maxValues = new FloatArrayList();
private boolean invalid;

private static float convert(ByteBuffer buffer) {
return buffer.order(LITTLE_ENDIAN).getFloat(0);
Expand All @@ -101,12 +102,30 @@ void addMinMaxFromBytes(ByteBuffer min, ByteBuffer max) {

@Override
void addMinMax(Object min, Object max) {
minValues.add((float) min);
maxValues.add((float) max);
float fMin = (float) min;
float fMax = (float) max;
if (Float.isNaN(fMin) || Float.isNaN(fMax)) {
// Invalidate this column index in case of NaN as the sorting order of values is undefined for this case
invalid = true;
}

// Sorting order is undefined for -0.0 so let min = -0.0 and max = +0.0 to ensure that no 0.0 values are skipped
if (Float.compare(fMin, +0.0f) == 0) {
fMin = -0.0f;
}
if (Float.compare(fMax, -0.0f) == 0) {
fMax = +0.0f;
}

minValues.add(fMin);
maxValues.add(fMax);
}

@Override
ColumnIndexBase<Float> createColumnIndex(PrimitiveType type) {
if (invalid) {
return null;
}
FloatColumnIndex columnIndex = new FloatColumnIndex(type);
columnIndex.minValues = minValues.toFloatArray();
columnIndex.maxValues = maxValues.toFloatArray();
Expand Down
43 changes: 41 additions & 2 deletions .../src/test/java/org/apache/parquet/internal/column/columnindex/TestColumnIndexBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.math.BigDecimal;
Expand Down Expand Up @@ -810,6 +811,25 @@ public void testBuildDouble() {
assertCorrectFiltering(columnIndex, invert(userDefined(col, DoubleIsInteger.class)), 0, 1, 2, 3, 4, 5, 6, 7, 8);
}

@Test
public void testBuildDoubleZeroNaN() {
PrimitiveType type = Types.required(DOUBLE).named("test_double");
ColumnIndexBuilder builder = ColumnIndexBuilder.getBuilder(type, Integer.MAX_VALUE);
StatsBuilder sb = new StatsBuilder();
builder.add(sb.stats(type, -1.0, -0.0));
builder.add(sb.stats(type, 0.0, 1.0));
builder.add(sb.stats(type, 1.0, 100.0));
ColumnIndex columnIndex = builder.build();
assertCorrectValues(columnIndex.getMinValues(), -1.0, -0.0, 1.0);
assertCorrectValues(columnIndex.getMaxValues(), 0.0, 1.0, 100.0);

builder = ColumnIndexBuilder.getBuilder(type, Integer.MAX_VALUE);
builder.add(sb.stats(type, -1.0, -0.0));
builder.add(sb.stats(type, 0.0, Double.NaN));
builder.add(sb.stats(type, 1.0, 100.0));
assertNull(builder.build());
}

@Test
public void testStaticBuildDouble() {
ColumnIndex columnIndex = ColumnIndexBuilder.build(
Expand Down Expand Up @@ -921,6 +941,25 @@ public void testBuildFloat() {
assertCorrectFiltering(columnIndex, invert(userDefined(col, FloatIsInteger.class)), 0, 1, 2, 3, 4, 5, 6, 7, 8);
}

@Test
public void testBuildFloatZeroNaN() {
PrimitiveType type = Types.required(FLOAT).named("test_float");
ColumnIndexBuilder builder = ColumnIndexBuilder.getBuilder(type, Integer.MAX_VALUE);
StatsBuilder sb = new StatsBuilder();
builder.add(sb.stats(type, -1.0f, -0.0f));
builder.add(sb.stats(type, 0.0f, 1.0f));
builder.add(sb.stats(type, 1.0f, 100.0f));
ColumnIndex columnIndex = builder.build();
assertCorrectValues(columnIndex.getMinValues(), -1.0f, -0.0f, 1.0f);
assertCorrectValues(columnIndex.getMaxValues(), 0.0f, 1.0f, 100.0f);

builder = ColumnIndexBuilder.getBuilder(type, Integer.MAX_VALUE);
builder.add(sb.stats(type, -1.0f, -0.0f));
builder.add(sb.stats(type, 0.0f, Float.NaN));
builder.add(sb.stats(type, 1.0f, 100.0f));
assertNull(builder.build());
}

@Test
public void testStaticBuildFloat() {
ColumnIndex columnIndex = ColumnIndexBuilder.build(
Expand Down Expand Up @@ -1391,7 +1430,7 @@ private static void assertCorrectValues(List<ByteBuffer> values, Double... expec
assertFalse("The byte buffer should be empty for null pages", value.hasRemaining());
} else {
assertEquals("The byte buffer should be 8 bytes long for double", 8, value.remaining());
assertEquals("Invalid value for page " + i, expectedValue.doubleValue(), value.getDouble(0), 0.0);
assertTrue("Invalid value for page " + i, Double.compare(expectedValue.doubleValue(), value.getDouble(0)) == 0);
}
}
}
Expand All @@ -1405,7 +1444,7 @@ private static void assertCorrectValues(List<ByteBuffer> values, Float... expect
assertFalse("The byte buffer should be empty for null pages", value.hasRemaining());
} else {
assertEquals("The byte buffer should be 4 bytes long for double", 4, value.remaining());
assertEquals("Invalid value for page " + i, expectedValue.floatValue(), value.getFloat(0), 0.0f);
assertTrue("Invalid value for page " + i, Float.compare(expectedValue.floatValue(), value.getFloat(0)) == 0);
}
}
}
Expand Down