You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@orc.apache.org by "jeanlyn (via GitHub)" <gi...@apache.org> on 2026/10/22 05:49:52 UTC

[PR] ORC-2226: Allow seeking to the end of stream in InStream.seek [orc]

jeanlyn opened a new pull request, #2722:
URL: https://github.com/apache/orc/pull/2722

   ### What changes were proposed in this pull request?
   
   Allow `InStream.UncompressedStream.seek(long)` and `InStream.CompressedStream.seek(long)` to seek to the end of the stream (`desired == length`) by parking the stream at EOF, instead of throwing `IllegalArgumentException` / `IOException`.
   
   ### Why are the changes needed?
   
   The row group index of an **all-null trailing row group** legitimately points at the end of the stream (both the Java and C++ writers record such positions). When predicate pushdown (SARG) selects *only* that row group, `StripePlanner.processStream` computes a zero-byte read range for the stream (`start == end`), so the stream is created with `bytes == null`. `UncompressedStream.seek()` then only special-cases `desired == 0 && bytes == null`, and a seek to `desired == length` falls through the range loop and throws:
   
   ```
   Caused by: java.lang.IllegalArgumentException: Seek in column 2 kind DATA to 8192 is outside of the data
       at org.apache.orc.impl.InStream$UncompressedStream.seek(InStream.java:...)
   ```
   
   A full scan (no SARG) reads the same file fine, as does the C++ reader. See [ORC-2226](https://issues.apache.org/jira/browse/ORC-2226) for a minimal reproduction (also reproduced on 2.1.2 and 2.3.1).
   
   ### How was this patch tested?
   
   New unit tests:
   
   - `TestInStream#testUncompressedSeekToEndOfStream` — seeking an `UncompressedStream` with no backing bytes to `length` is accepted, `read()` then returns `-1`, `available()` returns `0`, and seeking *past* the end (`length + 1`) still throws.
   - `TestInStream#testCompressedSeekToEndOfStream` — same for `CompressedStream`.
   - `TestRecordReaderImpl#testSargOnAllNullTrailingRowGroup` — end-to-end regression: writes an uncompressed file whose trailing row group is all-NULL for a string column, then reads it with a SARG that selects only that row group (fails before the fix, passes after).
   
   ### Notes for reviewers
   
   - The change only *relaxes* an over-strict bounds check. Seeks beyond the stream end (`desired > length`), and non-zero seeks on streams with no backing bytes other than `desired == length`, still throw — genuinely corrupt files are not masked.
   - For compressed streams, a stream-end position recorded by the writer always has an uncompressed offset of 0, so the `seek(PositionProvider)` follow-up never attempts to read a chunk header at EOF.
   - Behavior now matches the C++ reader, which tolerates seek-to-end.
   - Cross-module impact: none; no file-format or API changes. Subclasses (`EncryptedStream`, `EncryptedCompressedStream`) inherit the fix.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ORC-2226: Allow seeking to the end of stream in InStream.seek [orc]

Posted by "jeanlyn (via GitHub)" <gi...@apache.org>.
jeanlyn commented on code in PR #2722:
URL: https://github.com/apache/orc/pull/2722#discussion_r4346952892


##########
java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java:
##########
@@ -2817,4 +2817,121 @@ public  void testDecimalIsRepeatingFlag() throws IOException {
       }
     }
   }
+
+  @Test
+  public void testSargOnAllNullTrailingRowGroup() throws Exception {
+    // ORC-2226: with predicate pushdown, selecting only an all-null trailing
+    // row group of an uncompressed stripe makes the read planner assign zero
+    // bytes to the column's DATA stream; seeking it to the recorded position
+    // (the end of the stream) must not fail and must read all-null values.
+    Path path = new Path(workDir, "allNullTrailingRowGroup.orc");
+    FileSystem.get(conf).delete(path, true);
+    TypeDescription schema = TypeDescription.fromString("struct<k:string,v:string>");
+    Writer writer = OrcFile.createWriter(path, OrcFile.writerOptions(conf)
+        .setSchema(schema)
+        .compress(CompressionKind.NONE)
+        .rowIndexStride(1024));
+    VectorizedRowBatch batch = schema.createRowBatch();
+    BytesColumnVector k = (BytesColumnVector) batch.cols[0];
+    BytesColumnVector v = (BytesColumnVector) batch.cols[1];
+    batch.size = 1024;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "a".getBytes(StandardCharsets.UTF_8));
+      // distinct values so that the writer uses DIRECT encoding
+      v.setVal(i, String.format("%07dx", i).getBytes(StandardCharsets.UTF_8));
+    }
+    writer.addRowBatch(batch);
+    batch.reset();
+    batch.size = 512;
+    v.noNulls = false;
+    v.isRepeating = true;
+    v.isNull[0] = true;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "c".getBytes(StandardCharsets.UTF_8));
+    }
+    writer.addRowBatch(batch);
+    writer.close();
+
+    Reader reader = OrcFile.createReader(path, OrcFile.readerOptions(conf));
+    SearchArgument sarg = SearchArgumentFactory.newBuilder()
+        .equals("k", PredicateLeaf.Type.STRING, "c")
+        .build();
+    VectorizedRowBatch readBatch = reader.getSchema().createRowBatch();
+    long rowCount = 0;
+    try (RecordReader rows = reader.rows(reader.options()
+        .searchArgument(sarg, new String[] {"k"}))) {
+      while (rows.nextBatch(readBatch)) {
+        BytesColumnVector readK = (BytesColumnVector) readBatch.cols[0];
+        BytesColumnVector readV = (BytesColumnVector) readBatch.cols[1];
+        for (int i = 0; i < readBatch.size; i++) {
+          assertEquals("c", readK.toString(i));
+          assertTrue(readV.isRepeating ? readV.isNull[0] : readV.isNull[i]);
+        }
+        rowCount += readBatch.size;
+      }
+    }
+    assertEquals(512, rowCount);
+  }
+
+  @Test
+  public void testSargOnAllNullMiddleRowGroup() throws Exception {
+    // An all-null row group in the middle of a stripe is covered by the
+    // following row group's estimated end offset; make sure that such a
+    // selection keeps working.
+    Path path = new Path(workDir, "allNullMiddleRowGroup.orc");
+    FileSystem.get(conf).delete(path, true);
+    TypeDescription schema = TypeDescription.fromString("struct<k:string,v:string>");
+    Writer writer = OrcFile.createWriter(path, OrcFile.writerOptions(conf)
+        .setSchema(schema)
+        .compress(CompressionKind.NONE)
+        .rowIndexStride(1024));
+    VectorizedRowBatch batch = schema.createRowBatch();
+    BytesColumnVector k = (BytesColumnVector) batch.cols[0];
+    BytesColumnVector v = (BytesColumnVector) batch.cols[1];
+    batch.size = 1024;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "a".getBytes(StandardCharsets.UTF_8));
+      v.setVal(i, String.format("%07dx", i).getBytes(StandardCharsets.UTF_8));

Review Comment:
   Addressed in 3209741 — switched to assertThrows with the exact exception type, and String.format now uses Locale.ROOT. Thanks!



##########
java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java:
##########
@@ -2817,4 +2817,121 @@ public  void testDecimalIsRepeatingFlag() throws IOException {
       }
     }
   }
+
+  @Test
+  public void testSargOnAllNullTrailingRowGroup() throws Exception {
+    // ORC-2226: with predicate pushdown, selecting only an all-null trailing
+    // row group of an uncompressed stripe makes the read planner assign zero
+    // bytes to the column's DATA stream; seeking it to the recorded position
+    // (the end of the stream) must not fail and must read all-null values.
+    Path path = new Path(workDir, "allNullTrailingRowGroup.orc");
+    FileSystem.get(conf).delete(path, true);
+    TypeDescription schema = TypeDescription.fromString("struct<k:string,v:string>");
+    Writer writer = OrcFile.createWriter(path, OrcFile.writerOptions(conf)
+        .setSchema(schema)
+        .compress(CompressionKind.NONE)
+        .rowIndexStride(1024));
+    VectorizedRowBatch batch = schema.createRowBatch();
+    BytesColumnVector k = (BytesColumnVector) batch.cols[0];
+    BytesColumnVector v = (BytesColumnVector) batch.cols[1];
+    batch.size = 1024;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "a".getBytes(StandardCharsets.UTF_8));
+      // distinct values so that the writer uses DIRECT encoding
+      v.setVal(i, String.format("%07dx", i).getBytes(StandardCharsets.UTF_8));
+    }
+    writer.addRowBatch(batch);
+    batch.reset();
+    batch.size = 512;
+    v.noNulls = false;
+    v.isRepeating = true;
+    v.isNull[0] = true;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "c".getBytes(StandardCharsets.UTF_8));
+    }
+    writer.addRowBatch(batch);
+    writer.close();
+
+    Reader reader = OrcFile.createReader(path, OrcFile.readerOptions(conf));
+    SearchArgument sarg = SearchArgumentFactory.newBuilder()
+        .equals("k", PredicateLeaf.Type.STRING, "c")
+        .build();
+    VectorizedRowBatch readBatch = reader.getSchema().createRowBatch();
+    long rowCount = 0;
+    try (RecordReader rows = reader.rows(reader.options()
+        .searchArgument(sarg, new String[] {"k"}))) {
+      while (rows.nextBatch(readBatch)) {
+        BytesColumnVector readK = (BytesColumnVector) readBatch.cols[0];
+        BytesColumnVector readV = (BytesColumnVector) readBatch.cols[1];
+        for (int i = 0; i < readBatch.size; i++) {
+          assertEquals("c", readK.toString(i));
+          assertTrue(readV.isRepeating ? readV.isNull[0] : readV.isNull[i]);
+        }
+        rowCount += readBatch.size;
+      }
+    }
+    assertEquals(512, rowCount);
+  }
+
+  @Test
+  public void testSargOnAllNullMiddleRowGroup() throws Exception {
+    // An all-null row group in the middle of a stripe is covered by the
+    // following row group's estimated end offset; make sure that such a
+    // selection keeps working.
+    Path path = new Path(workDir, "allNullMiddleRowGroup.orc");
+    FileSystem.get(conf).delete(path, true);
+    TypeDescription schema = TypeDescription.fromString("struct<k:string,v:string>");
+    Writer writer = OrcFile.createWriter(path, OrcFile.writerOptions(conf)
+        .setSchema(schema)
+        .compress(CompressionKind.NONE)
+        .rowIndexStride(1024));
+    VectorizedRowBatch batch = schema.createRowBatch();
+    BytesColumnVector k = (BytesColumnVector) batch.cols[0];
+    BytesColumnVector v = (BytesColumnVector) batch.cols[1];
+    batch.size = 1024;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "a".getBytes(StandardCharsets.UTF_8));
+      v.setVal(i, String.format("%07dx", i).getBytes(StandardCharsets.UTF_8));
+    }
+    writer.addRowBatch(batch);
+    batch.reset();
+    batch.size = 1024;
+    v.noNulls = false;
+    v.isRepeating = true;
+    v.isNull[0] = true;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "b".getBytes(StandardCharsets.UTF_8));
+    }
+    writer.addRowBatch(batch);
+    batch.reset();
+    batch.size = 1024;
+    v.noNulls = true;
+    v.isRepeating = false;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "d".getBytes(StandardCharsets.UTF_8));
+      v.setVal(i, String.format("%07dy", i).getBytes(StandardCharsets.UTF_8));

Review Comment:
   Addressed in 3209741 — switched to assertThrows with the exact exception type, and String.format now uses Locale.ROOT. Thanks!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ORC-2226: Allow seeking to the end of stream in InStream.seek [orc]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2722:
URL: https://github.com/apache/orc/pull/2722#discussion_r4345764222


##########
java/core/src/test/org/apache/orc/impl/TestInStream.java:
##########
@@ -1000,4 +1000,73 @@ public void testStreamResetWithoutIncreasedLength() throws IOException {
     byte[] inBuffer = new byte[5];
     assertEquals(5, inStream.read(inBuffer));
   }
+
+  @Test
+  public void testUncompressedSeekToEndOfStream() throws Exception {
+    // ORC-2226: the row group index of an all-null trailing row group points
+    // at the end of the stream, and the read planner may not have read any
+    // bytes for the stream, so seeking to the end must be accepted.
+    InStream in = InStream.create("test", null, 0, 1024);
+    InStream.UncompressedStream stream = (InStream.UncompressedStream) in;
+    stream.seek(0);
+    stream.seek(1024);
+    assertEquals(-1, in.read());
+    assertEquals(0, in.available());
+    try {
+      stream.seek(1025);
+      fail();
+    } catch (IllegalArgumentException e) {
+      // expected: past the end of the stream
+    }
+  }
+
+  @Test
+  public void testCompressedSeekToEndOfStream() throws Exception {
+    // ORC-2226: same as above, for compressed streams.
+    InStream in = InStream.create("test", null, 0, 1024,
+        InStream.options().withCodec(new ZlibCodec()).withBufferSize(300));
+    InStream.CompressedStream stream = (InStream.CompressedStream) in;
+    stream.seek(0);
+    stream.seek(1024);
+    assertEquals(-1, in.read());
+    assertEquals(0, in.available());
+    try {
+      stream.seek(1025);
+      fail();
+    } catch (IOException e) {
+      // expected: past the end of the stream
+    }

Review Comment:
   This test expects an `IOException` when seeking past the end, but the stream implementations commonly signal out-of-bounds seeks via `IllegalArgumentException` (as your uncompressed test does). If `CompressedStream.seek(long)` throws `IllegalArgumentException`, this test will fail. Prefer `assertThrows(IllegalArgumentException.class, () -> stream.seek(1025))` (JUnit 4.13+) or catch `IllegalArgumentException` here to align with the actual contract.



##########
java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java:
##########
@@ -2817,4 +2817,121 @@ public  void testDecimalIsRepeatingFlag() throws IOException {
       }
     }
   }
+
+  @Test
+  public void testSargOnAllNullTrailingRowGroup() throws Exception {
+    // ORC-2226: with predicate pushdown, selecting only an all-null trailing
+    // row group of an uncompressed stripe makes the read planner assign zero
+    // bytes to the column's DATA stream; seeking it to the recorded position
+    // (the end of the stream) must not fail and must read all-null values.
+    Path path = new Path(workDir, "allNullTrailingRowGroup.orc");
+    FileSystem.get(conf).delete(path, true);
+    TypeDescription schema = TypeDescription.fromString("struct<k:string,v:string>");
+    Writer writer = OrcFile.createWriter(path, OrcFile.writerOptions(conf)
+        .setSchema(schema)
+        .compress(CompressionKind.NONE)
+        .rowIndexStride(1024));
+    VectorizedRowBatch batch = schema.createRowBatch();
+    BytesColumnVector k = (BytesColumnVector) batch.cols[0];
+    BytesColumnVector v = (BytesColumnVector) batch.cols[1];
+    batch.size = 1024;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "a".getBytes(StandardCharsets.UTF_8));
+      // distinct values so that the writer uses DIRECT encoding
+      v.setVal(i, String.format("%07dx", i).getBytes(StandardCharsets.UTF_8));

Review Comment:
   These tests use `String.format(...)` without specifying a locale, which makes output depend on the JVM default locale. To keep tests deterministic across environments, use `String.format(Locale.ROOT, ...)` for these formatted values.



##########
java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java:
##########
@@ -2817,4 +2817,121 @@ public  void testDecimalIsRepeatingFlag() throws IOException {
       }
     }
   }
+
+  @Test
+  public void testSargOnAllNullTrailingRowGroup() throws Exception {
+    // ORC-2226: with predicate pushdown, selecting only an all-null trailing
+    // row group of an uncompressed stripe makes the read planner assign zero
+    // bytes to the column's DATA stream; seeking it to the recorded position
+    // (the end of the stream) must not fail and must read all-null values.
+    Path path = new Path(workDir, "allNullTrailingRowGroup.orc");
+    FileSystem.get(conf).delete(path, true);
+    TypeDescription schema = TypeDescription.fromString("struct<k:string,v:string>");
+    Writer writer = OrcFile.createWriter(path, OrcFile.writerOptions(conf)
+        .setSchema(schema)
+        .compress(CompressionKind.NONE)
+        .rowIndexStride(1024));
+    VectorizedRowBatch batch = schema.createRowBatch();
+    BytesColumnVector k = (BytesColumnVector) batch.cols[0];
+    BytesColumnVector v = (BytesColumnVector) batch.cols[1];
+    batch.size = 1024;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "a".getBytes(StandardCharsets.UTF_8));
+      // distinct values so that the writer uses DIRECT encoding
+      v.setVal(i, String.format("%07dx", i).getBytes(StandardCharsets.UTF_8));
+    }
+    writer.addRowBatch(batch);
+    batch.reset();
+    batch.size = 512;
+    v.noNulls = false;
+    v.isRepeating = true;
+    v.isNull[0] = true;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "c".getBytes(StandardCharsets.UTF_8));
+    }
+    writer.addRowBatch(batch);
+    writer.close();
+
+    Reader reader = OrcFile.createReader(path, OrcFile.readerOptions(conf));
+    SearchArgument sarg = SearchArgumentFactory.newBuilder()
+        .equals("k", PredicateLeaf.Type.STRING, "c")
+        .build();
+    VectorizedRowBatch readBatch = reader.getSchema().createRowBatch();
+    long rowCount = 0;
+    try (RecordReader rows = reader.rows(reader.options()
+        .searchArgument(sarg, new String[] {"k"}))) {
+      while (rows.nextBatch(readBatch)) {
+        BytesColumnVector readK = (BytesColumnVector) readBatch.cols[0];
+        BytesColumnVector readV = (BytesColumnVector) readBatch.cols[1];
+        for (int i = 0; i < readBatch.size; i++) {
+          assertEquals("c", readK.toString(i));
+          assertTrue(readV.isRepeating ? readV.isNull[0] : readV.isNull[i]);
+        }
+        rowCount += readBatch.size;
+      }
+    }
+    assertEquals(512, rowCount);
+  }
+
+  @Test
+  public void testSargOnAllNullMiddleRowGroup() throws Exception {
+    // An all-null row group in the middle of a stripe is covered by the
+    // following row group's estimated end offset; make sure that such a
+    // selection keeps working.
+    Path path = new Path(workDir, "allNullMiddleRowGroup.orc");
+    FileSystem.get(conf).delete(path, true);
+    TypeDescription schema = TypeDescription.fromString("struct<k:string,v:string>");
+    Writer writer = OrcFile.createWriter(path, OrcFile.writerOptions(conf)
+        .setSchema(schema)
+        .compress(CompressionKind.NONE)
+        .rowIndexStride(1024));
+    VectorizedRowBatch batch = schema.createRowBatch();
+    BytesColumnVector k = (BytesColumnVector) batch.cols[0];
+    BytesColumnVector v = (BytesColumnVector) batch.cols[1];
+    batch.size = 1024;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "a".getBytes(StandardCharsets.UTF_8));
+      v.setVal(i, String.format("%07dx", i).getBytes(StandardCharsets.UTF_8));

Review Comment:
   These tests use `String.format(...)` without specifying a locale, which makes output depend on the JVM default locale. To keep tests deterministic across environments, use `String.format(Locale.ROOT, ...)` for these formatted values.



##########
java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java:
##########
@@ -2817,4 +2817,121 @@ public  void testDecimalIsRepeatingFlag() throws IOException {
       }
     }
   }
+
+  @Test
+  public void testSargOnAllNullTrailingRowGroup() throws Exception {
+    // ORC-2226: with predicate pushdown, selecting only an all-null trailing
+    // row group of an uncompressed stripe makes the read planner assign zero
+    // bytes to the column's DATA stream; seeking it to the recorded position
+    // (the end of the stream) must not fail and must read all-null values.
+    Path path = new Path(workDir, "allNullTrailingRowGroup.orc");
+    FileSystem.get(conf).delete(path, true);
+    TypeDescription schema = TypeDescription.fromString("struct<k:string,v:string>");
+    Writer writer = OrcFile.createWriter(path, OrcFile.writerOptions(conf)
+        .setSchema(schema)
+        .compress(CompressionKind.NONE)
+        .rowIndexStride(1024));
+    VectorizedRowBatch batch = schema.createRowBatch();
+    BytesColumnVector k = (BytesColumnVector) batch.cols[0];
+    BytesColumnVector v = (BytesColumnVector) batch.cols[1];
+    batch.size = 1024;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "a".getBytes(StandardCharsets.UTF_8));
+      // distinct values so that the writer uses DIRECT encoding
+      v.setVal(i, String.format("%07dx", i).getBytes(StandardCharsets.UTF_8));
+    }
+    writer.addRowBatch(batch);
+    batch.reset();
+    batch.size = 512;
+    v.noNulls = false;
+    v.isRepeating = true;
+    v.isNull[0] = true;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "c".getBytes(StandardCharsets.UTF_8));
+    }
+    writer.addRowBatch(batch);
+    writer.close();
+
+    Reader reader = OrcFile.createReader(path, OrcFile.readerOptions(conf));
+    SearchArgument sarg = SearchArgumentFactory.newBuilder()
+        .equals("k", PredicateLeaf.Type.STRING, "c")
+        .build();
+    VectorizedRowBatch readBatch = reader.getSchema().createRowBatch();
+    long rowCount = 0;
+    try (RecordReader rows = reader.rows(reader.options()
+        .searchArgument(sarg, new String[] {"k"}))) {
+      while (rows.nextBatch(readBatch)) {
+        BytesColumnVector readK = (BytesColumnVector) readBatch.cols[0];
+        BytesColumnVector readV = (BytesColumnVector) readBatch.cols[1];
+        for (int i = 0; i < readBatch.size; i++) {
+          assertEquals("c", readK.toString(i));
+          assertTrue(readV.isRepeating ? readV.isNull[0] : readV.isNull[i]);
+        }
+        rowCount += readBatch.size;
+      }
+    }
+    assertEquals(512, rowCount);
+  }
+
+  @Test
+  public void testSargOnAllNullMiddleRowGroup() throws Exception {
+    // An all-null row group in the middle of a stripe is covered by the
+    // following row group's estimated end offset; make sure that such a
+    // selection keeps working.
+    Path path = new Path(workDir, "allNullMiddleRowGroup.orc");
+    FileSystem.get(conf).delete(path, true);
+    TypeDescription schema = TypeDescription.fromString("struct<k:string,v:string>");
+    Writer writer = OrcFile.createWriter(path, OrcFile.writerOptions(conf)
+        .setSchema(schema)
+        .compress(CompressionKind.NONE)
+        .rowIndexStride(1024));
+    VectorizedRowBatch batch = schema.createRowBatch();
+    BytesColumnVector k = (BytesColumnVector) batch.cols[0];
+    BytesColumnVector v = (BytesColumnVector) batch.cols[1];
+    batch.size = 1024;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "a".getBytes(StandardCharsets.UTF_8));
+      v.setVal(i, String.format("%07dx", i).getBytes(StandardCharsets.UTF_8));
+    }
+    writer.addRowBatch(batch);
+    batch.reset();
+    batch.size = 1024;
+    v.noNulls = false;
+    v.isRepeating = true;
+    v.isNull[0] = true;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "b".getBytes(StandardCharsets.UTF_8));
+    }
+    writer.addRowBatch(batch);
+    batch.reset();
+    batch.size = 1024;
+    v.noNulls = true;
+    v.isRepeating = false;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "d".getBytes(StandardCharsets.UTF_8));
+      v.setVal(i, String.format("%07dy", i).getBytes(StandardCharsets.UTF_8));

Review Comment:
   These tests use `String.format(...)` without specifying a locale, which makes output depend on the JVM default locale. To keep tests deterministic across environments, use `String.format(Locale.ROOT, ...)` for these formatted values.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ORC-2226: Allow seeking to the end of stream in InStream.seek [orc]

Posted by "jeanlyn (via GitHub)" <gi...@apache.org>.
jeanlyn commented on code in PR #2722:
URL: https://github.com/apache/orc/pull/2722#discussion_r4346951672


##########
java/core/src/test/org/apache/orc/impl/TestInStream.java:
##########
@@ -1000,4 +1000,73 @@ public void testStreamResetWithoutIncreasedLength() throws IOException {
     byte[] inBuffer = new byte[5];
     assertEquals(5, inStream.read(inBuffer));
   }
+
+  @Test
+  public void testUncompressedSeekToEndOfStream() throws Exception {
+    // ORC-2226: the row group index of an all-null trailing row group points
+    // at the end of the stream, and the read planner may not have read any
+    // bytes for the stream, so seeking to the end must be accepted.
+    InStream in = InStream.create("test", null, 0, 1024);
+    InStream.UncompressedStream stream = (InStream.UncompressedStream) in;
+    stream.seek(0);
+    stream.seek(1024);
+    assertEquals(-1, in.read());
+    assertEquals(0, in.available());
+    try {
+      stream.seek(1025);
+      fail();
+    } catch (IllegalArgumentException e) {
+      // expected: past the end of the stream
+    }
+  }
+
+  @Test
+  public void testCompressedSeekToEndOfStream() throws Exception {
+    // ORC-2226: same as above, for compressed streams.
+    InStream in = InStream.create("test", null, 0, 1024,
+        InStream.options().withCodec(new ZlibCodec()).withBufferSize(300));
+    InStream.CompressedStream stream = (InStream.CompressedStream) in;
+    stream.seek(0);
+    stream.seek(1024);
+    assertEquals(-1, in.read());
+    assertEquals(0, in.available());
+    try {
+      stream.seek(1025);
+      fail();
+    } catch (IOException e) {
+      // expected: past the end of the stream
+    }

Review Comment:
   Addressed in 3209741 — switched to assertThrows with the exact exception type, and String.format now uses Locale.ROOT. Thanks!



##########
java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java:
##########
@@ -2817,4 +2817,121 @@ public  void testDecimalIsRepeatingFlag() throws IOException {
       }
     }
   }
+
+  @Test
+  public void testSargOnAllNullTrailingRowGroup() throws Exception {
+    // ORC-2226: with predicate pushdown, selecting only an all-null trailing
+    // row group of an uncompressed stripe makes the read planner assign zero
+    // bytes to the column's DATA stream; seeking it to the recorded position
+    // (the end of the stream) must not fail and must read all-null values.
+    Path path = new Path(workDir, "allNullTrailingRowGroup.orc");
+    FileSystem.get(conf).delete(path, true);
+    TypeDescription schema = TypeDescription.fromString("struct<k:string,v:string>");
+    Writer writer = OrcFile.createWriter(path, OrcFile.writerOptions(conf)
+        .setSchema(schema)
+        .compress(CompressionKind.NONE)
+        .rowIndexStride(1024));
+    VectorizedRowBatch batch = schema.createRowBatch();
+    BytesColumnVector k = (BytesColumnVector) batch.cols[0];
+    BytesColumnVector v = (BytesColumnVector) batch.cols[1];
+    batch.size = 1024;
+    for (int i = 0; i < batch.size; i++) {
+      k.setVal(i, "a".getBytes(StandardCharsets.UTF_8));
+      // distinct values so that the writer uses DIRECT encoding
+      v.setVal(i, String.format("%07dx", i).getBytes(StandardCharsets.UTF_8));

Review Comment:
   Addressed in 3209741 — switched to assertThrows with the exact exception type, and String.format now uses Locale.ROOT. Thanks!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org