This is an automated email from the ASF dual-hosted git repository.
omalley pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/orc.git
The following commit(s) were added to refs/heads/master by this push:
new 3444c89 ORC-438 - Fix possible NPEs in StringStatisticsImpl.merge()
3444c89 is described below
commit 3444c89d281bb5919c0ca8d1d8eb602443f9a3dc
Author: Sandeep More <[email protected]>
AuthorDate: Mon Nov 26 11:37:06 2018 -0500
ORC-438 - Fix possible NPEs in StringStatisticsImpl.merge()
Fixes #341
Signed-off-by: Owen O'Malley <[email protected]>
---
.../org/apache/orc/impl/ColumnStatisticsImpl.java | 56 ++++++++--------------
.../test/org/apache/orc/TestColumnStatistics.java | 11 ++++-
java/pom.xml | 1 +
3 files changed, 31 insertions(+), 37 deletions(-)
diff --git a/java/core/src/java/org/apache/orc/impl/ColumnStatisticsImpl.java
b/java/core/src/java/org/apache/orc/impl/ColumnStatisticsImpl.java
index 62a7563..1dd0418 100644
--- a/java/core/src/java/org/apache/orc/impl/ColumnStatisticsImpl.java
+++ b/java/core/src/java/org/apache/orc/impl/ColumnStatisticsImpl.java
@@ -605,49 +605,31 @@ public class ColumnStatisticsImpl implements
ColumnStatistics {
public void merge(ColumnStatisticsImpl other) {
if (other instanceof StringStatisticsImpl) {
StringStatisticsImpl str = (StringStatisticsImpl) other;
- if (minimum == null) {
- if (str.minimum != null) {
- maximum = new Text(str.getMaximum());
- minimum = new Text(str.getMinimum());
- }
- /* str.minimum == null when lower bound set */
- else if (str.isLowerBoundSet) {
- minimum = new Text(str.getLowerBound());
+ if (count == 0) {
+ if (str.count != 0) {
+ minimum = new Text(str.minimum);
isLowerBoundSet = str.isLowerBoundSet;
-
- /* check for upper bound before setting max */
- if (str.isUpperBoundSet) {
- maximum = new Text(str.getUpperBound());
- isUpperBoundSet = str.isUpperBoundSet;
- } else {
- maximum = new Text(str.getMaximum());
- }
- }
- else {
- /* both are empty */
+ maximum = new Text(str.maximum);
+ isUpperBoundSet = str.isUpperBoundSet;
+ } else {
+ /* both are empty */
maximum = minimum = null;
+ isLowerBoundSet = false;
+ isUpperBoundSet = false;
}
- } else if (str.minimum != null) {
+ } else if (str.count != 0) {
if (minimum.compareTo(str.minimum) > 0) {
- if(str.isLowerBoundSet) {
- minimum = new Text(str.getLowerBound());
- isLowerBoundSet = str.isLowerBoundSet;
- } else {
- minimum = new Text(str.getMinimum());
- }
+ minimum = new Text(str.minimum);
+ isLowerBoundSet = str.isLowerBoundSet;
}
if (maximum.compareTo(str.maximum) < 0) {
- if(str.isUpperBoundSet) {
- maximum = new Text(str.getUpperBound());
- isUpperBoundSet = str.isUpperBoundSet;
- }else {
- maximum = new Text(str.getMaximum());
- }
+ maximum = new Text(str.maximum);
+ isUpperBoundSet = str.isUpperBoundSet;
}
}
sum += str.sum;
} else {
- if (isStatsExists() && minimum != null) {
+ if (isStatsExists()) {
throw new IllegalArgumentException("Incompatible merging of string
column statistics");
}
}
@@ -660,8 +642,10 @@ public class ColumnStatisticsImpl implements
ColumnStatistics {
OrcProto.StringStatistics.Builder str =
OrcProto.StringStatistics.newBuilder();
if (getNumberOfValues() != 0) {
- str.setMinimum(getMinimum());
- str.setMaximum(getMaximum());
+ /* getLowerBound() will ALWAYS return min value */
+ str.setMinimum(getLowerBound());
+ /* getUpperBound() will ALWAYS return max value */
+ str.setMaximum(getUpperBound());
str.setSum(sum);
}
result.setStringStatistics(str);
@@ -1638,7 +1622,7 @@ public class ColumnStatisticsImpl implements
ColumnStatistics {
}
}
- private long count = 0;
+ protected long count = 0;
private boolean hasNull = false;
private long bytesOnDisk = 0;
diff --git a/java/core/src/test/org/apache/orc/TestColumnStatistics.java
b/java/core/src/test/org/apache/orc/TestColumnStatistics.java
index e6d6953..5908675 100644
--- a/java/core/src/test/org/apache/orc/TestColumnStatistics.java
+++ b/java/core/src/test/org/apache/orc/TestColumnStatistics.java
@@ -98,11 +98,14 @@ public class TestColumnStatistics {
ColumnStatisticsImpl stats1 = ColumnStatisticsImpl.create(schema);
ColumnStatisticsImpl stats2 = ColumnStatisticsImpl.create(schema);
+ stats1.increment(3);
stats1.updateString(new Text("bob"));
stats1.updateString(new Text("david"));
stats1.updateString(new Text("charles"));
+ stats2.increment(2);
stats2.updateString(new Text("anne"));
byte[] erin = new byte[]{0, 1, 2, 3, 4, 5, 101, 114, 105, 110};
+ stats2.increment();
stats2.updateString(erin, 6, 4, 5);
assertEquals(24, ((StringColumnStatistics)stats2).getSum());
stats1.merge(stats2);
@@ -111,6 +114,7 @@ public class TestColumnStatistics {
assertEquals("erin", typed.getMaximum());
assertEquals(39, typed.getSum());
stats1.reset();
+ stats1.increment(2);
stats1.updateString(new Text("aaa"));
stats1.updateString(new Text("zzz"));
stats1.merge(stats2);
@@ -131,6 +135,7 @@ public class TestColumnStatistics {
final ColumnStatisticsImpl stats2 = ColumnStatisticsImpl.create(schema);
/* test a scenario for the first max string */
+ stats1.increment();
stats1.updateString(new Text(test));
final StringColumnStatistics typed = (StringColumnStatistics) stats1;
@@ -146,6 +151,7 @@ public class TestColumnStatistics {
stats1.reset();
/* test a scenario for the first max bytes */
+ stats1.increment();
stats1.updateString(test.getBytes(StandardCharsets.UTF_8), 0,
test.getBytes(StandardCharsets.UTF_8).length, 0);
@@ -157,10 +163,12 @@ public class TestColumnStatistics {
stats1.reset();
/* test upper bound - merging */
+ stats1.increment(3);
stats1.updateString(new Text("bob"));
stats1.updateString(new Text("david"));
stats1.updateString(new Text("charles"));
+ stats2.increment(2);
stats2.updateString(new Text("anne"));
stats2.updateString(new Text(fragment));
@@ -177,9 +185,10 @@ public class TestColumnStatistics {
stats1.reset();
stats2.reset();
+ stats1.increment(2);
stats1.updateString(new Text("david"));
stats1.updateString(new Text("charles"));
-
+ stats2.increment(2);
stats2.updateString(new Text("jane"));
stats2.updateString(new Text(fragmentLowerBound));
diff --git a/java/pom.xml b/java/pom.xml
index 0f930b1..d742088 100644
--- a/java/pom.xml
+++ b/java/pom.xml
@@ -150,6 +150,7 @@
<artifactId>maven-surefire-plugin</artifactId>
<version>2.21.0</version>
<configuration>
+ <trimStackTrace>false</trimStackTrace>
<reuseForks>false</reuseForks>
<argLine>-Xmx2048m</argLine>
<environmentVariables>
