ORC 1.7.4-SNAPSHOT fails with Iceberg Nan count tests about orc HOT 12 CLOSED

Thanks to capturing these failed tests, @williamhyun
I conducted some investigation.
Prior to #1055, it was just coincidence that these tests would pass, after #1055 was fixed so that the push-down judgement could continue to be executed.

1. testIsNaN[format = orc] This test is no longer working.
   When orc is not written to Double.NaN, and DoubleStatistics sum is a finite value, we can correctly push down no_nans = NaN and skip this stripe.
2. testNotNaN[format = orc] is still a valid test.
   The reason this test doesn't pass is that the statistics check for the Float type is missing.
   

   orc/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java

   Lines 696 to 703 in 70c504c

   	} else if (category == TypeDescription.Category.DOUBLE) {
   	DoubleColumnStatistics dstas = (DoubleColumnStatistics) cs;
   	if (!Double.isFinite(dstas.getSum())) {
   	LOG.debug("Not using predication pushdown on {} because stats contain NaN values",
   	predicate.getColumnName());
   	return dstas.hasNull() ? TruthValue.YES_NO_NULL : TruthValue.YES_NO;
   	}
   	}

   
   category == TypeDescription.Category.DOUBLE || category == TypeDescription.Category.FLOAT

from orc.

Thank you @guiyanakuang !
I am retesting on both Iceberg and Spark.

from orc.

Thank you, I am rerunning the tests here:

• williamhyun/iceberg#1
• williamhyun/spark#3

from orc.

Thank you for reporting, @williamhyun .

from orc.

Oh, thank you, @guiyanakuang !

from orc.

I also quickly re-checked the Iceberg test. So, TestMetricsRowGroupFilter passed except the following one mentioned by @guiyanakuang .

shouldRead = shouldRead(isNaN("no_nans"));
Assert.assertTrue("Should read: NaN counts are not tracked in Parquet metrics", shouldRead);

from orc.

If you get the results, please share them to the dev mailing list, @williamhyun .

from orc.

@williamhyun
ORC-1147 landed to main/branch-1.7/1.6. Could you rerun the integration tests, please?

from orc.

Thank you for rerunning it. Yes, I saw Spark passed and Iceberg has only the following. If we comment out the test, it will pass cleanly.

org.apache.iceberg.data.TestMetricsRowGroupFilter > testIsNaN[format = orc] FAILED
    java.lang.AssertionError: Should read: NaN counts are not tracked in Parquet metrics
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.assertTrue(Assert.java:42)
        at org.apache.iceberg.data.TestMetricsRowGroupFilter.testIsNaN(TestMetricsRowGroupFilter.java:308)

from orc.

BTW,

• if you feel ok, please close this issue and move forward, @williamhyun . :)
• Since the same patches are in the branch-1.6, I'll trigger integration test for them separately as a release manager of Apache ORC 1.6.14 as a part of #1081 .

from orc.

One more thing. @williamhyun . If you don't mind, please follow the style of #1081 and update your release issue, #1046 . It will be greatly helpful for people to track your efforts and progress.

from orc.

Thank you for verifying and closing this.

from orc.