This is an automated email from the ASF dual-hosted git repository.
omalley pushed a commit to branch branch-1.6
in repository https://gitbox.apache.org/repos/asf/orc.git
The following commit(s) were added to refs/heads/branch-1.6 by this push:
new b457818 ORC-569: Fixed empty positions list in first row index entry.
b457818 is described below
commit b457818ba57a9ddce2c1b2b5671e9edd73b71730
Author: Owen O'Malley <[email protected]>
AuthorDate: Wed Nov 13 14:03:29 2019 -0800
ORC-569: Fixed empty positions list in first row index entry.
Fixes #451
Signed-off-by: Owen O'Malley <[email protected]>
---
java/core/src/findbugs/exclude.xml | 4 ++
.../orc/impl/writer/StringBaseTreeWriter.java | 1 +
.../test/org/apache/orc/TestStringDictionary.java | 71 +++++++++++++++++++---
.../src/java/org/apache/orc/tools/Driver.java | 2 +-
4 files changed, 68 insertions(+), 10 deletions(-)
diff --git a/java/core/src/findbugs/exclude.xml
b/java/core/src/findbugs/exclude.xml
index 6112afd..9e21433 100644
--- a/java/core/src/findbugs/exclude.xml
+++ b/java/core/src/findbugs/exclude.xml
@@ -61,4 +61,8 @@
<Bug pattern="RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE"/>
<Class name="org.apache.orc.impl.TestSchemaEvolution"/>
</Match>
+ <Match>
+ <Bug pattern="RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE"/>
+ <Class name="org.apache.orc.TestStringDictionary"/>
+ </Match>
</FindBugsFilter>
diff --git
a/java/core/src/java/org/apache/orc/impl/writer/StringBaseTreeWriter.java
b/java/core/src/java/org/apache/orc/impl/writer/StringBaseTreeWriter.java
index c3f56a6..08f0964 100644
--- a/java/core/src/java/org/apache/orc/impl/writer/StringBaseTreeWriter.java
+++ b/java/core/src/java/org/apache/orc/impl/writer/StringBaseTreeWriter.java
@@ -86,6 +86,7 @@ public abstract class StringBaseTreeWriter extends
TreeWriterBase {
if (dictionaryKeySizeThreshold <= 0.0) {
useDictionaryEncoding = false;
doneDictionaryCheck = true;
+ recordDirectStreamPosition();
} else {
doneDictionaryCheck = false;
}
diff --git a/java/core/src/test/org/apache/orc/TestStringDictionary.java
b/java/core/src/test/org/apache/orc/TestStringDictionary.java
index b301dd6..9ffe533 100644
--- a/java/core/src/test/org/apache/orc/TestStringDictionary.java
+++ b/java/core/src/test/org/apache/orc/TestStringDictionary.java
@@ -1,4 +1,4 @@
-/**
+/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
@@ -18,9 +18,9 @@
package org.apache.orc;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
import java.io.File;
-import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.Map;
@@ -32,6 +32,7 @@ import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.orc.impl.OrcIndex;
import org.apache.orc.impl.OutStream;
import org.apache.orc.impl.RecordReaderImpl;
import org.apache.orc.impl.StreamName;
@@ -49,12 +50,12 @@ import org.junit.rules.TestName;
public class TestStringDictionary {
- Path workDir = new Path(System.getProperty("test.tmp.dir", "target" +
File.separator + "test"
+ private Path workDir = new Path(System.getProperty("test.tmp.dir", "target"
+ File.separator + "test"
+ File.separator + "tmp"));
- Configuration conf;
- FileSystem fs;
- Path testFilePath;
+ private Configuration conf;
+ private FileSystem fs;
+ private Path testFilePath;
@Rule
public TestName testCaseName = new TestName();
@@ -63,7 +64,7 @@ public class TestStringDictionary {
public void openFileSystem() throws Exception {
conf = new Configuration();
fs = FileSystem.getLocal(conf);
- testFilePath = new Path(workDir, "TestOrcFile." +
testCaseName.getMethodName() + ".orc");
+ testFilePath = new Path(workDir, "TestStringDictionary." +
testCaseName.getMethodName() + ".orc");
fs.delete(testFilePath, false);
}
@@ -173,7 +174,7 @@ public class TestStringDictionary {
}
@Override
- public OutStream createStream(StreamName name) throws IOException {
+ public OutStream createStream(StreamName name) {
TestInStream.OutputCollector collect = new
TestInStream.OutputCollector();
streams.put(name, collect);
return new OutStream("test", new StreamOptions(1000), collect);
@@ -230,7 +231,7 @@ public class TestStringDictionary {
}
@Override
- public void writeStatistics(StreamName name,
OrcProto.ColumnStatistics.Builder stats) throws IOException {
+ public void writeStatistics(StreamName name,
OrcProto.ColumnStatistics.Builder stats) {
}
@@ -506,4 +507,56 @@ public class TestStringDictionary {
}
}
+ @Test
+ public void testForcedNonDictionary() throws Exception {
+ // Set the row stride to 16k so that it is a multiple of the batch size
+ final int INDEX_STRIDE = 16 * 1024;
+ final int NUM_BATCHES = 50;
+ // Explicitly turn off dictionary encoding.
+ OrcConf.DICTIONARY_KEY_SIZE_THRESHOLD.setDouble(conf, 0);
+ TypeDescription schema = TypeDescription.fromString("struct<str:string>");
+ try (Writer writer = OrcFile.createWriter(testFilePath,
+ OrcFile.writerOptions(conf)
+ .setSchema(schema)
+ .rowIndexStride(INDEX_STRIDE))) {
+ // Write 50 batches where each batch has a single value for str.
+ VectorizedRowBatch batch = schema.createRowBatchV2();
+ BytesColumnVector col = (BytesColumnVector) batch.cols[0];
+ for(int b=0; b < NUM_BATCHES; ++b) {
+ batch.reset();
+ batch.size = 1024;
+ col.setVal(0, ("Value for " + b).getBytes(StandardCharsets.UTF_8));
+ col.isRepeating = true;
+ writer.addRowBatch(batch);
+ }
+ }
+
+ try (Reader reader = OrcFile.createReader(testFilePath,
+ OrcFile.readerOptions(conf));
+ RecordReaderImpl rows = (RecordReaderImpl) reader.rows()) {
+ VectorizedRowBatch batch = reader.getSchema().createRowBatchV2();
+ BytesColumnVector col = (BytesColumnVector) batch.cols[0];
+
+ // Get the index for the str column
+ OrcProto.RowIndex index = rows.readRowIndex(0, null, null)
+ .getRowGroupIndex()[1];
+ // We assume that it fits in a single stripe
+ assertEquals(1, reader.getStripes().size());
+ // There are 4 entries, because ceil(NUM_BATCHES * 1024 / INDEX_STRIDE)
= 4.
+ assertEquals(4, index.getEntryCount());
+ for(int e=0; e < index.getEntryCount(); ++e) {
+ OrcProto.RowIndexEntry entry = index.getEntry(e);
+ // For a string column with direct encoding, compression & no nulls, we
+ // should have 5 positions in each entry.
+ assertEquals("position count entry " + e, 5,
entry.getPositionsCount());
+ // make sure we can seek and get the right data
+ int row = e * INDEX_STRIDE;
+ rows.seekToRow(row);
+ assertTrue("entry " + e, rows.nextBatch(batch));
+ assertEquals("entry " + e, 1024, batch.size);
+ assertEquals("entry " + e, true, col.noNulls);
+ assertEquals("entry " + e, "Value for " + (row / 1024),
col.toString(0));
+ }
+ }
+ }
}
diff --git a/java/tools/src/java/org/apache/orc/tools/Driver.java
b/java/tools/src/java/org/apache/orc/tools/Driver.java
index 3de89b9..8b16876 100644
--- a/java/tools/src/java/org/apache/orc/tools/Driver.java
+++ b/java/tools/src/java/org/apache/orc/tools/Driver.java
@@ -47,7 +47,7 @@ public class Driver {
result.addOption(OptionBuilder
.withLongOpt("define")
.withDescription("Set a configuration property")
- .hasArg()
+ .hasArgs(2)
.withValueSeparator()
.create('D'));
return result;
