Skip to content

Commit 706c8dc

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

4 files changed

Lines changed: 41 additions & 27 deletions

File tree

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

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ using ORCFileReader = arrow::adapters::orc::ORCFileReader;
3535
using RecordBatchReader = arrow::RecordBatchReader;
3636

3737
static jclass io_exception_class;
38-
static jclass exception_class;
38+
static jclass illegal_access_exception_class;
39+
static jclass illegal_argument_exception_class;
3940

4041
static jclass orc_field_node_class;
4142
static jmethodID orc_field_node_constructor;
@@ -66,7 +67,7 @@ jmethodID GetMethodID(JNIEnv* env, jclass this_class, const char* name, const ch
6667
if (ret == nullptr) {
6768
std::string error_message = "Unable to find method " + std::string(name) +
6869
" within signature" + std::string(sig);
69-
env->ThrowNew(exception_class, error_message.c_str());
70+
env->ThrowNew(illegal_access_exception_class, error_message.c_str());
7071
}
7172

7273
return ret;
@@ -81,6 +82,26 @@ std::string JStringToCString(JNIEnv* env, jstring string) {
8182
return std::string(buffer.data(), clen);
8283
}
8384

85+
std::shared_ptr<ORCFileReader> GetFileReader(JNIEnv* env, jlong id) {
86+
auto reader = orc_reader_holder_.Lookup(id);
87+
if (!reader) {
88+
std::string error_message = "invalid reader id " + std::to_string(id);
89+
env->ThrowNew(illegal_argument_exception_class, error_message.c_str());
90+
}
91+
92+
return reader;
93+
}
94+
95+
std::shared_ptr<RecordBatchReader> GetStripeReader(JNIEnv* env, jlong id) {
96+
auto reader = orc_stripe_reader_holder_.Lookup(id);
97+
if (!reader) {
98+
std::string error_message = "invalid stripe reader id " + std::to_string(id);
99+
env->ThrowNew(illegal_argument_exception_class, error_message.c_str());
100+
}
101+
102+
return reader;
103+
}
104+
84105
#ifdef __cplusplus
85106
extern "C" {
86107
#endif
@@ -92,7 +113,10 @@ jint JNI_OnLoad(JavaVM* vm, void* reserved) {
92113
}
93114

94115
io_exception_class = CreateGlobalClassReference(env, "Ljava/io/IOException;");
95-
exception_class = CreateGlobalClassReference(env, "Ljava/lang/Exception;");
116+
illegal_access_exception_class =
117+
CreateGlobalClassReference(env, "Ljava/lang/IllegalAccessException;");
118+
illegal_argument_exception_class =
119+
CreateGlobalClassReference(env, "Ljava/lang/IllegalArgumentException;");
96120

97121
orc_field_node_class =
98122
CreateGlobalClassReference(env, "Lorg/apache/arrow/adapter/orc/OrcFieldNode;");
@@ -118,7 +142,8 @@ void JNI_OnUnload(JavaVM* vm, void* reserved) {
118142
JNIEnv* env;
119143
vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION);
120144
env->DeleteGlobalRef(io_exception_class);
121-
env->DeleteGlobalRef(exception_class);
145+
env->DeleteGlobalRef(illegal_access_exception_class);
146+
env->DeleteGlobalRef(illegal_argument_exception_class);
122147
env->DeleteGlobalRef(orc_field_node_class);
123148
env->DeleteGlobalRef(orc_memory_class);
124149
env->DeleteGlobalRef(record_batch_class);
@@ -165,15 +190,15 @@ JNIEXPORT void JNICALL Java_org_apache_arrow_adapter_orc_OrcReaderJniWrapper_clo
165190

166191
JNIEXPORT jboolean JNICALL Java_org_apache_arrow_adapter_orc_OrcReaderJniWrapper_seek(
167192
JNIEnv* env, jobject this_obj, jlong id, jint row_number) {
168-
auto reader = orc_reader_holder_.Lookup(id);
193+
auto reader = GetFileReader(env, id);
169194
return reader->Seek(row_number).ok();
170195
}
171196

172197
JNIEXPORT jint JNICALL
173198
Java_org_apache_arrow_adapter_orc_OrcReaderJniWrapper_getNumberOfStripes(JNIEnv* env,
174199
jobject this_obj,
175200
jlong id) {
176-
auto reader = orc_reader_holder_.Lookup(id);
201+
auto reader = GetFileReader(env, id);
177202
return reader->NumberOfStripes();
178203
}
179204

@@ -182,11 +207,7 @@ Java_org_apache_arrow_adapter_orc_OrcReaderJniWrapper_nextStripeReader(JNIEnv* e
182207
jobject this_obj,
183208
jlong id,
184209
jlong batch_size) {
185-
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-
}
210+
auto reader = GetFileReader(env, id);
190211

191212
std::shared_ptr<RecordBatchReader> stripe_reader;
192213
auto status = reader->NextStripeReader(batch_size, &stripe_reader);
@@ -206,11 +227,7 @@ JNIEXPORT jbyteArray JNICALL
206227
Java_org_apache_arrow_adapter_orc_OrcStripeReaderJniWrapper_getSchema(JNIEnv* env,
207228
jclass this_cls,
208229
jlong id) {
209-
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-
}
230+
auto stripe_reader = GetStripeReader(env, id);
214231

215232
auto schema = stripe_reader->schema();
216233

@@ -231,11 +248,7 @@ JNIEXPORT jobject JNICALL
231248
Java_org_apache_arrow_adapter_orc_OrcStripeReaderJniWrapper_next(JNIEnv* env,
232249
jclass this_cls,
233250
jlong id) {
234-
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-
}
251+
auto stripe_reader = GetStripeReader(env, id);
239252

240253
std::shared_ptr<arrow::RecordBatch> record_batch;
241254
auto status = stripe_reader->ReadNext(&record_batch);
@@ -245,6 +258,7 @@ Java_org_apache_arrow_adapter_orc_OrcStripeReaderJniWrapper_next(JNIEnv* env,
245258

246259
auto schema = stripe_reader->schema();
247260

261+
// TODO: ARROW-4714 Ensure JVM has sufficient capacity to create local references
248262
// create OrcFieldNode[]
249263
jobjectArray field_array =
250264
env->NewObjectArray(schema->num_fields(), orc_field_node_class, nullptr);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class OrcJniUtils {
3434
private OrcJniUtils() {}
3535

3636
static void loadOrcAdapterLibraryFromJar()
37-
throws IOException {
37+
throws IOException, IllegalAccessException {
3838
synchronized (OrcJniUtils.class) {
3939
if (!isLoaded) {
4040
final String libraryToLoad = System.mapLibraryName(LIBRARY_NAME);

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public class OrcReader implements AutoCloseable {
4343
* @param allocator allocator provided to ArrowReader.
4444
* @throws IOException throws exception in case of file not found
4545
*/
46-
public OrcReader(String filePath, BufferAllocator allocator) throws IOException {
46+
public OrcReader(String filePath, BufferAllocator allocator) throws IOException, IllegalAccessException {
4747
this.allocator = allocator;
4848
this.jniWrapper = OrcReaderJniWrapper.getInstance();
4949
this.nativeInstanceId = jniWrapper.open(filePath);
@@ -55,7 +55,7 @@ public OrcReader(String filePath, BufferAllocator allocator) throws IOException
5555
* @param rowNumber the rows number to seek
5656
* @return true if seek operation is succeeded
5757
*/
58-
public boolean seek(int rowNumber) {
58+
public boolean seek(int rowNumber) throws IllegalArgumentException {
5959
return jniWrapper.seek(nativeInstanceId, rowNumber);
6060
}
6161

@@ -65,7 +65,7 @@ public boolean seek(int rowNumber) {
6565
* @param batchSize the number of rows loaded on each iteration
6666
* @return ArrowReader that iterate over current stripes
6767
*/
68-
public ArrowReader nextStripeReader(long batchSize) {
68+
public ArrowReader nextStripeReader(long batchSize) throws IllegalArgumentException {
6969
long stripeReaderId = jniWrapper.nextStripeReader(nativeInstanceId, batchSize);
7070
if (stripeReaderId < 0) {
7171
return null;
@@ -79,7 +79,7 @@ public ArrowReader nextStripeReader(long batchSize) {
7979
*
8080
* @return number of stripes
8181
*/
82-
public int getNumberOfStripes() {
82+
public int getNumberOfStripes() throws IllegalArgumentException {
8383
return jniWrapper.getNumberOfStripes(nativeInstanceId);
8484
}
8585

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class OrcReaderJniWrapper {
2626

2727
private static volatile OrcReaderJniWrapper INSTANCE;
2828

29-
static OrcReaderJniWrapper getInstance() throws IOException {
29+
static OrcReaderJniWrapper getInstance() throws IOException, IllegalAccessException {
3030
if (INSTANCE == null) {
3131
synchronized (OrcReaderJniWrapper.class) {
3232
if (INSTANCE == null) {

0 commit comments

Comments
 (0)