You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@orc.apache.org by om...@apache.org on 2019/11/14 05:06:57 UTC

[orc] branch branch-1.5 updated: ORC-569: Fixed empty positions list in first row index entry.

This is an automated email from the ASF dual-hosted git repository.

omalley pushed a commit to branch branch-1.5
in repository https://gitbox.apache.org/repos/asf/orc.git


The following commit(s) were added to refs/heads/branch-1.5 by this push:
     new 0e683a1  ORC-569: Fixed empty positions list in first row index entry.
0e683a1 is described below

commit 0e683a11783c41c2167f7a30e128fe9e1ef8e356
Author: Owen O'Malley <om...@apache.org>
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 <om...@apache.org>
---
 java/core/src/findbugs/exclude.xml                 | 18 ++++++
 .../orc/impl/writer/StringBaseTreeWriter.java      |  1 +
 .../test/org/apache/orc/TestStringDictionary.java  | 66 ++++++++++++++++++++--
 .../src/java/org/apache/orc/tools/Driver.java      |  2 +-
 4 files changed, 80 insertions(+), 7 deletions(-)

diff --git a/java/core/src/findbugs/exclude.xml b/java/core/src/findbugs/exclude.xml
index a78dda7..2fe2408 100644
--- a/java/core/src/findbugs/exclude.xml
+++ b/java/core/src/findbugs/exclude.xml
@@ -38,4 +38,22 @@
     <Class name="org.apache.orc.impl.OrcAcidUtils"/>
     <Method name="getLastFlushLength"/>
   </Match>
+  <Match>
+    <Bug pattern="RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE"/>
+    <Class name="org.apache.orc.impl.TestReaderImpl"/>
+    <Method name="testOptionSafety"/>
+  </Match>
+  <Match>
+    <Bug pattern="RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE"/>
+    <Class name="org.apache.orc.TestVectorOrcFile"/>
+    <Method name="testZstd"/>
+  </Match>
+  <Match>
+    <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 e7d3259..a747a98 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
@@ -82,6 +82,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 cc4f8d8..203f58e 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,6 +18,7 @@
 package org.apache.orc;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.io.File;
 import java.io.IOException;
@@ -32,6 +33,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;
@@ -47,12 +49,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();
@@ -61,7 +63,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);
   }
 
@@ -484,4 +486,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;