[GitHub] orc pull request #222: ORC-310 better error handling for codec

[sershe]

Github user sershe commented on a diff in the pull request:

https://github.com/apache/orc/pull/222#discussion_r173943752
  
--- Diff: java/core/src/java/org/apache/orc/impl/RecordReaderUtils.java ---
@@ -306,15 +330,24 @@ public void releaseBuffer(ByteBuffer buffer) {
 
 @Override
 public DataReader clone() {
+  if (this.file != null) {
+throw new UnsupportedOperationException(
+"Cannot clone a DataReader that is already opened");
+  }
   try {
-return (DataReader) super.clone();
+DefaultDataReader clone = (DefaultDataReader) super.clone();
+// Make sure we don't share the same codec between two readers.
+clone.codec = OrcCodecPool.getCodec(clone.compressionKind);
--- End diff --

super.clone() is Object.clone(), so no 


---

[GitHub] orc pull request #222: ORC-310 better error handling for codec

[prasanthj]

Github user prasanthj commented on a diff in the pull request:

https://github.com/apache/orc/pull/222#discussion_r173941203
  
--- Diff: java/core/src/java/org/apache/orc/impl/RecordReaderUtils.java ---
@@ -306,15 +330,24 @@ public void releaseBuffer(ByteBuffer buffer) {
 
 @Override
 public DataReader clone() {
+  if (this.file != null) {
+throw new UnsupportedOperationException(
+"Cannot clone a DataReader that is already opened");
+  }
   try {
-return (DataReader) super.clone();
+DefaultDataReader clone = (DefaultDataReader) super.clone();
+// Make sure we don't share the same codec between two readers.
+clone.codec = OrcCodecPool.getCodec(clone.compressionKind);
--- End diff --

can this be moved inside super.clone()?


---

[GitHub] orc pull request #222: ORC-310 better error handling for codec

[sershe]

Github user sershe commented on a diff in the pull request:

https://github.com/apache/orc/pull/222#discussion_r171728816
  
--- Diff: java/core/src/java/org/apache/orc/impl/WriterImpl.java ---
@@ -645,6 +645,8 @@ public void 
appendUserMetadata(List userMetadata) {
 return ReaderImpl.deserializeStats(builder.getStatisticsList());
   }
 
+  // TODO: remove this
--- End diff --

Removed the attribute, clarified the TODO


---

[GitHub] orc pull request #222: ORC-310 better error handling for codec

[omalley]

Github user omalley commented on a diff in the pull request:

https://github.com/apache/orc/pull/222#discussion_r171719585
  
--- Diff: java/core/src/java/org/apache/orc/impl/WriterImpl.java ---
@@ -645,6 +645,8 @@ public void 
appendUserMetadata(List userMetadata) {
 return ReaderImpl.deserializeStats(builder.getStatisticsList());
   }
 
+  // TODO: remove this
--- End diff --

Remove the changes in this file. We do not want a guice dependency.


---

[GitHub] orc pull request #222: ORC-310 better error handling for codec

[omalley]

Github user omalley commented on a diff in the pull request:

https://github.com/apache/orc/pull/222#discussion_r171720836
  
--- Diff: java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java ---
@@ -1353,6 +1353,8 @@ public static String encodeTranslatedSargColumn(int 
rootColumn, Integer indexInS
 return result;
   }
 
+  // TODO: remove this
--- End diff --

Revert the changes in this file.


---

[GitHub] orc pull request #222: ORC-310 better error handling for codec

[omalley]

Github user omalley commented on a diff in the pull request:

https://github.com/apache/orc/pull/222#discussion_r171720713
  
--- Diff: java/core/src/java/org/apache/orc/impl/ReaderImpl.java ---
@@ -472,8 +473,9 @@ public static OrcTail extractFileTail(ByteBuffer 
buffer, long fileLength, long m
 .setPostscript(ps)
 .setFooter(footer)
 .setFileLength(fileLength);
+  isCodecError = false;
 } finally {
--- End diff --

try {} finally is pretty broken. You are better off using the 
try-with-resources statement.


---

[GitHub] orc pull request #222: ORC-310 better error handling for codec

[sershe]

GitHub user sershe opened a pull request:

https://github.com/apache/orc/pull/222

ORC-310 better error handling for codec



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/sershe/orc orc-310

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/orc/pull/222.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #222


commit d9727cb36c9b1e9f1c045e29d928401a680062cb
Author: sergey 
Date:   2018-03-01T00:00:51Z

ORC-310
better error handling for codec




---