Skip to content

Commit de8529c

Browse files
author
Yurui Zhou
committed
resolve comments
1 parent fc80175 commit de8529c

5 files changed

Lines changed: 48 additions & 44 deletions

File tree

‎cpp/src/jni/orc/jni_wrapper.cpp‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ JNIEXPORT jlong JNICALL Java_org_apache_arrow_adapter_orc_OrcReaderJniWrapper_op
136136

137137
arrow::Status ret;
138138
if (path.find("hdfs://") == 0) {
139-
env->ThrowNew(io_exception_class, "hdfs path not support yet.");
139+
env->ThrowNew(io_exception_class, "hdfs path not supported yet.");
140140
} else {
141141
ret = arrow::io::ReadableFile::Open(path, &in_file);
142142
}
@@ -183,6 +183,11 @@ Java_org_apache_arrow_adapter_orc_OrcReaderJniWrapper_nextStripeReader(JNIEnv* e
183183
jlong id,
184184
jlong batch_size) {
185185
auto reader = orc_reader_holder_.Lookup(id);
186+
if (!reader) {
187+
std::string error_message = "invalid reader id " + std::to_string(id);
188+
env->ThrowNew(exception_class, error_message.c_str());
189+
}
190+
186191
std::shared_ptr<RecordBatchReader> stripe_reader;
187192
auto status = reader->NextStripeReader(batch_size, &stripe_reader);
188193

@@ -202,6 +207,11 @@ Java_org_apache_arrow_adapter_orc_OrcStripeReaderJniWrapper_getSchema(JNIEnv* en
202207
jclass this_cls,
203208
jlong id) {
204209
auto stripe_reader = orc_stripe_reader_holder_.Lookup(id);
210+
if (!stripe_reader) {
211+
std::string error_message = "invalid stripe reader id " + std::to_string(id);
212+
env->ThrowNew(exception_class, error_message.c_str());
213+
}
214+
205215
auto schema = stripe_reader->schema();
206216

207217
std::shared_ptr<arrow::Buffer> out;
@@ -222,6 +232,11 @@ Java_org_apache_arrow_adapter_orc_OrcStripeReaderJniWrapper_next(JNIEnv* env,
222232
jclass this_cls,
223233
jlong id) {
224234
auto stripe_reader = orc_stripe_reader_holder_.Lookup(id);
235+
if (!stripe_reader) {
236+
std::string error_message = "invalid stripe reader id " + std::to_string(id);
237+
env->ThrowNew(exception_class, error_message.c_str());
238+
}
239+
225240
std::shared_ptr<arrow::RecordBatch> record_batch;
226241
auto status = stripe_reader->ReadNext(&record_batch);
227242
if (!status.ok() || !record_batch) {

‎java/adapter/orc/src/main/java/org/apache/arrow/adapter/orc/OrcMemoryJniWrapper.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
*/
2323
class OrcMemoryJniWrapper implements AutoCloseable {
2424

25-
private final long id;
25+
private final long nativeInstanceId;
2626

2727
private final long memoryAddress;
2828

@@ -32,13 +32,13 @@ class OrcMemoryJniWrapper implements AutoCloseable {
3232

3333
/**
3434
* Construct a new instance.
35-
* @param id unique id of the underlying memory.
35+
* @param nativeInstanceId unique id of the underlying memory.
3636
* @param memoryAddress starting memory address of the the underlying memory.
3737
* @param size size of the valid data.
3838
* @param capacity allocated memory size.
3939
*/
40-
OrcMemoryJniWrapper(long id, long memoryAddress, long size, long capacity) {
41-
this.id = id;
40+
OrcMemoryJniWrapper(long nativeInstanceId, long memoryAddress, long size, long capacity) {
41+
this.nativeInstanceId = nativeInstanceId;
4242
this.memoryAddress = memoryAddress;
4343
this.size = size;
4444
this.capacity = capacity;
@@ -70,7 +70,7 @@ long getMemoryAddress() {
7070

7171
@Override
7272
public void close() {
73-
release(id);
73+
release(nativeInstanceId);
7474
}
7575

7676
private native void release(long id);

‎java/adapter/orc/src/main/java/org/apache/arrow/adapter/orc/OrcReader.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public class OrcReader implements AutoCloseable {
3535
/**
3636
* reference to native reader instance.
3737
*/
38-
private final long id;
38+
private final long nativeInstanceId;
3939

4040
/**
4141
* Create an OrcReader that iterate over orc stripes.
@@ -46,7 +46,7 @@ public class OrcReader implements AutoCloseable {
4646
public OrcReader(String filePath, BufferAllocator allocator) throws IOException {
4747
this.allocator = allocator;
4848
this.jniWrapper = OrcReaderJniWrapper.getInstance();
49-
this.id = jniWrapper.open(filePath);
49+
this.nativeInstanceId = jniWrapper.open(filePath);
5050
}
5151

5252
/**
@@ -56,7 +56,7 @@ public OrcReader(String filePath, BufferAllocator allocator) throws IOException
5656
* @return true if seek operation is succeeded
5757
*/
5858
public boolean seek(int rowNumber) {
59-
return jniWrapper.seek(id, rowNumber);
59+
return jniWrapper.seek(nativeInstanceId, rowNumber);
6060
}
6161

6262
/**
@@ -66,7 +66,7 @@ public boolean seek(int rowNumber) {
6666
* @return ArrowReader that iterate over current stripes
6767
*/
6868
public ArrowReader nextStripeReader(long batchSize) {
69-
long stripeReaderId = jniWrapper.nextStripeReader(id, batchSize);
69+
long stripeReaderId = jniWrapper.nextStripeReader(nativeInstanceId, batchSize);
7070
if (stripeReaderId < 0) {
7171
return null;
7272
}
@@ -80,11 +80,11 @@ public ArrowReader nextStripeReader(long batchSize) {
8080
* @return number of stripes
8181
*/
8282
public int getNumberOfStripes() {
83-
return jniWrapper.getNumberOfStripes(id);
83+
return jniWrapper.getNumberOfStripes(nativeInstanceId);
8484
}
8585

8686
@Override
8787
public void close() throws Exception {
88-
jniWrapper.close(id);
88+
jniWrapper.close(nativeInstanceId);
8989
}
9090
}

‎java/adapter/orc/src/main/java/org/apache/arrow/adapter/orc/OrcReferenceManager.java‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -102,18 +102,7 @@ public ArrowBuf deriveBuffer(ArrowBuf sourceBuffer, int index, int length) {
102102

103103
@Override
104104
public OwnershipTransferResult transferOwnership(ArrowBuf sourceBuffer, BufferAllocator targetAllocator) {
105-
retain();
106-
return new OwnershipTransferResult() {
107-
@Override
108-
public boolean getAllocationFit() {
109-
return false;
110-
}
111-
112-
@Override
113-
public ArrowBuf getTransferredBuffer() {
114-
return sourceBuffer;
115-
}
116-
};
105+
throw new UnsupportedOperationException();
117106
}
118107

119108
@Override

‎java/adapter/orc/src/main/java/org/apache/arrow/adapter/orc/OrcStripeReader.java‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -43,23 +43,22 @@ public class OrcStripeReader extends ArrowReader {
4343
/**
4444
* reference to native stripe reader instance.
4545
*/
46-
private final long id;
47-
private MessageChannelReader schemaReader;
46+
private final long nativeInstanceId;
4847

4948
/**
5049
* Construct a new instance.
51-
* @param id id of the stripe reader instance, obtained by
50+
* @param nativeInstanceId nativeInstanceId of the stripe reader instance, obtained by
5251
* calling nextStripeReader from OrcReaderJniWrapper
5352
* @param allocator memory allocator for accounting.
5453
*/
55-
OrcStripeReader(long id, BufferAllocator allocator) {
54+
OrcStripeReader(long nativeInstanceId, BufferAllocator allocator) {
5655
super(allocator);
57-
this.id = id;
56+
this.nativeInstanceId = nativeInstanceId;
5857
}
5958

6059
@Override
6160
public boolean loadNextBatch() throws IOException {
62-
OrcRecordBatch recordBatch = OrcStripeReaderJniWrapper.next(id);
61+
OrcRecordBatch recordBatch = OrcStripeReaderJniWrapper.next(nativeInstanceId);
6362
if (recordBatch == null) {
6463
return false;
6564
}
@@ -91,28 +90,29 @@ public long bytesRead() {
9190

9291
@Override
9392
protected void closeReadSource() throws IOException {
94-
OrcStripeReaderJniWrapper.close(id);
93+
OrcStripeReaderJniWrapper.close(nativeInstanceId);
9594
}
9695

9796
@Override
9897
protected Schema readSchema() throws IOException {
99-
byte[] schemaBytes = OrcStripeReaderJniWrapper.getSchema(id);
100-
schemaReader = new MessageChannelReader(
101-
new ReadChannel(
102-
new ByteArrayReadableSeekableByteChannel(schemaBytes)), allocator);
98+
byte[] schemaBytes = OrcStripeReaderJniWrapper.getSchema(nativeInstanceId);
10399

104-
MessageResult result = schemaReader.readNext();
105-
schemaReader.close();
100+
try (MessageChannelReader schemaReader =
101+
new MessageChannelReader(
102+
new ReadChannel(
103+
new ByteArrayReadableSeekableByteChannel(schemaBytes)), allocator)) {
106104

107-
if (result == null) {
108-
throw new IOException("Unexpected end of input. Missing schema.");
109-
}
105+
MessageResult result = schemaReader.readNext();
106+
if (result == null) {
107+
throw new IOException("Unexpected end of input. Missing schema.");
108+
}
110109

111-
if (result.getMessage().headerType() != MessageHeader.Schema) {
112-
throw new IOException("Expected schema but header was " + result.getMessage().headerType());
113-
}
110+
if (result.getMessage().headerType() != MessageHeader.Schema) {
111+
throw new IOException("Expected schema but header was " + result.getMessage().headerType());
112+
}
114113

115-
return MessageSerializer.deserializeSchema(result.getMessage());
114+
return MessageSerializer.deserializeSchema(result.getMessage());
115+
}
116116
}
117117

118118
@Override

0 commit comments

Comments
 (0)