This is an automated email from the ASF dual-hosted git repository.
agrove pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
The following commit(s) were added to refs/heads/main by this push:
new 9bbfddfc chore: Remove some calls to `unwrap` (#598)
9bbfddfc is described below
commit 9bbfddfc90dd47d78fc7a16b829c9f4efdce15ab
Author: Andy Grove <[email protected]>
AuthorDate: Thu Jun 27 11:08:43 2024 -0700
chore: Remove some calls to `unwrap` (#598)
* remove an unwrap from ScanExec
* remove another unwrap
* remove another unwrap
* remove more unwraps
---
core/Cargo.toml | 2 +-
.../execution/datafusion/expressions/subquery.rs | 2 +-
core/src/execution/memory_pool.rs | 4 +-
core/src/execution/operators/scan.rs | 9 ++--
core/src/jvm_bridge/batch_iterator.rs | 2 +-
core/src/jvm_bridge/comet_exec.rs | 52 ++++++++--------------
core/src/jvm_bridge/comet_metric_node.rs | 16 +++----
core/src/jvm_bridge/mod.rs | 13 ++++--
8 files changed, 43 insertions(+), 57 deletions(-)
diff --git a/core/Cargo.toml b/core/Cargo.toml
index 10c595e6..7c22876d 100644
--- a/core/Cargo.toml
+++ b/core/Cargo.toml
@@ -108,7 +108,7 @@ strip = "debuginfo"
[lib]
name = "comet"
# "rlib" is for benchmarking with criterion.
-crate_type = ["cdylib", "rlib"]
+crate-type = ["cdylib", "rlib"]
[[bench]]
name = "parquet_read"
diff --git a/core/src/execution/datafusion/expressions/subquery.rs
b/core/src/execution/datafusion/expressions/subquery.rs
index 9b1be2df..cf6f8d84 100644
--- a/core/src/execution/datafusion/expressions/subquery.rs
+++ b/core/src/execution/datafusion/expressions/subquery.rs
@@ -90,7 +90,7 @@ impl PhysicalExpr for Subquery {
}
fn evaluate(&self, _: &RecordBatch) ->
datafusion_common::Result<ColumnarValue> {
- let mut env = JVMClasses::get_env();
+ let mut env = JVMClasses::get_env()?;
unsafe {
let is_null = jni_static_call!(&mut env,
diff --git a/core/src/execution/memory_pool.rs
b/core/src/execution/memory_pool.rs
index ff236909..4eb18c3f 100644
--- a/core/src/execution/memory_pool.rs
+++ b/core/src/execution/memory_pool.rs
@@ -59,7 +59,7 @@ impl CometMemoryPool {
}
fn acquire(&self, additional: usize) -> CometResult<i64> {
- let mut env = JVMClasses::get_env();
+ let mut env = JVMClasses::get_env()?;
let handle = self.task_memory_manager_handle.as_obj();
unsafe {
jni_call!(&mut env,
@@ -68,7 +68,7 @@ impl CometMemoryPool {
}
fn release(&self, size: usize) -> CometResult<()> {
- let mut env = JVMClasses::get_env();
+ let mut env = JVMClasses::get_env()?;
let handle = self.task_memory_manager_handle.as_obj();
unsafe {
jni_call!(&mut env,
comet_task_memory_manager(handle).release_memory(size as i64) -> ())
diff --git a/core/src/execution/operators/scan.rs
b/core/src/execution/operators/scan.rs
index bd518eda..de532821 100644
--- a/core/src/execution/operators/scan.rs
+++ b/core/src/execution/operators/scan.rs
@@ -146,7 +146,7 @@ impl ScanExec {
return Ok(InputBatch::EOF);
}
- let mut env = JVMClasses::get_env();
+ let mut env = JVMClasses::get_env()?;
if iter.is_null() {
return Err(CometError::from(ExecutionError::GeneralError(format!(
@@ -318,13 +318,12 @@ impl ScanStream {
.zip(schema_fields.iter())
.map(|(column, f)| {
if column.data_type() != f.data_type() {
- cast_with_options(column, f.data_type(),
&cast_options).unwrap()
+ cast_with_options(column, f.data_type(), &cast_options)
} else {
- column.clone()
+ Ok(column.clone())
}
})
- .collect();
-
+ .collect::<Result<Vec<_>, _>>()?;
let options = RecordBatchOptions::new().with_row_count(Some(num_rows));
RecordBatch::try_new_with_options(self.schema.clone(), new_columns,
&options)
.map_err(|e| arrow_datafusion_err!(e))
diff --git a/core/src/jvm_bridge/batch_iterator.rs
b/core/src/jvm_bridge/batch_iterator.rs
index f6f5e06f..06f43a8c 100644
--- a/core/src/jvm_bridge/batch_iterator.rs
+++ b/core/src/jvm_bridge/batch_iterator.rs
@@ -37,7 +37,7 @@ impl<'a> CometBatchIterator<'a> {
Ok(CometBatchIterator {
class,
- method_next: env.get_method_id(Self::JVM_CLASS, "next",
"()[J").unwrap(),
+ method_next: env.get_method_id(Self::JVM_CLASS, "next", "()[J")?,
method_next_ret: ReturnType::Array,
})
}
diff --git a/core/src/jvm_bridge/comet_exec.rs
b/core/src/jvm_bridge/comet_exec.rs
index 9d4a76eb..1bcbbc4a 100644
--- a/core/src/jvm_bridge/comet_exec.rs
+++ b/core/src/jvm_bridge/comet_exec.rs
@@ -56,49 +56,35 @@ impl<'a> CometExec<'a> {
let class = env.find_class(Self::JVM_CLASS)?;
Ok(CometExec {
- method_get_bool: env
- .get_static_method_id(Self::JVM_CLASS, "getBoolean", "(JJ)Z")
- .unwrap(),
+ method_get_bool: env.get_static_method_id(Self::JVM_CLASS,
"getBoolean", "(JJ)Z")?,
method_get_bool_ret: ReturnType::Primitive(Primitive::Boolean),
- method_get_byte: env
- .get_static_method_id(Self::JVM_CLASS, "getByte", "(JJ)B")
- .unwrap(),
+ method_get_byte: env.get_static_method_id(Self::JVM_CLASS,
"getByte", "(JJ)B")?,
method_get_byte_ret: ReturnType::Primitive(Primitive::Byte),
- method_get_short: env
- .get_static_method_id(Self::JVM_CLASS, "getShort", "(JJ)S")
- .unwrap(),
+ method_get_short: env.get_static_method_id(Self::JVM_CLASS,
"getShort", "(JJ)S")?,
method_get_short_ret: ReturnType::Primitive(Primitive::Short),
- method_get_int: env
- .get_static_method_id(Self::JVM_CLASS, "getInt", "(JJ)I")
- .unwrap(),
+ method_get_int: env.get_static_method_id(Self::JVM_CLASS,
"getInt", "(JJ)I")?,
method_get_int_ret: ReturnType::Primitive(Primitive::Int),
- method_get_long: env
- .get_static_method_id(Self::JVM_CLASS, "getLong", "(JJ)J")
- .unwrap(),
+ method_get_long: env.get_static_method_id(Self::JVM_CLASS,
"getLong", "(JJ)J")?,
method_get_long_ret: ReturnType::Primitive(Primitive::Long),
- method_get_float: env
- .get_static_method_id(Self::JVM_CLASS, "getFloat", "(JJ)F")
- .unwrap(),
+ method_get_float: env.get_static_method_id(Self::JVM_CLASS,
"getFloat", "(JJ)F")?,
method_get_float_ret: ReturnType::Primitive(Primitive::Float),
- method_get_double: env
- .get_static_method_id(Self::JVM_CLASS, "getDouble", "(JJ)D")
- .unwrap(),
+ method_get_double: env.get_static_method_id(Self::JVM_CLASS,
"getDouble", "(JJ)D")?,
method_get_double_ret: ReturnType::Primitive(Primitive::Double),
- method_get_decimal: env
- .get_static_method_id(Self::JVM_CLASS, "getDecimal", "(JJ)[B")
- .unwrap(),
+ method_get_decimal: env.get_static_method_id(
+ Self::JVM_CLASS,
+ "getDecimal",
+ "(JJ)[B",
+ )?,
method_get_decimal_ret: ReturnType::Array,
- method_get_string: env
- .get_static_method_id(Self::JVM_CLASS, "getString",
"(JJ)Ljava/lang/String;")
- .unwrap(),
+ method_get_string: env.get_static_method_id(
+ Self::JVM_CLASS,
+ "getString",
+ "(JJ)Ljava/lang/String;",
+ )?,
method_get_string_ret: ReturnType::Object,
- method_get_binary: env
- .get_static_method_id(Self::JVM_CLASS, "getBinary", "(JJ)[B")
- .unwrap(),
+ method_get_binary: env.get_static_method_id(Self::JVM_CLASS,
"getBinary", "(JJ)[B")?,
method_get_binary_ret: ReturnType::Array,
- method_is_null: env
- .get_static_method_id(Self::JVM_CLASS, "isNull", "(JJ)Z")
- .unwrap(),
+ method_is_null: env.get_static_method_id(Self::JVM_CLASS,
"isNull", "(JJ)Z")?,
method_is_null_ret: ReturnType::Primitive(Primitive::Boolean),
class,
})
diff --git a/core/src/jvm_bridge/comet_metric_node.rs
b/core/src/jvm_bridge/comet_metric_node.rs
index 821dc3ff..55423a68 100644
--- a/core/src/jvm_bridge/comet_metric_node.rs
+++ b/core/src/jvm_bridge/comet_metric_node.rs
@@ -38,17 +38,13 @@ impl<'a> CometMetricNode<'a> {
let class = env.find_class(Self::JVM_CLASS)?;
Ok(CometMetricNode {
- method_get_child_node: env
- .get_method_id(
- Self::JVM_CLASS,
- "getChildNode",
- format!("(I)L{:};", Self::JVM_CLASS).as_str(),
- )
- .unwrap(),
+ method_get_child_node: env.get_method_id(
+ Self::JVM_CLASS,
+ "getChildNode",
+ format!("(I)L{:};", Self::JVM_CLASS).as_str(),
+ )?,
method_get_child_node_ret: ReturnType::Object,
- method_add: env
- .get_method_id(Self::JVM_CLASS, "add",
"(Ljava/lang/String;J)V")
- .unwrap(),
+ method_add: env.get_method_id(Self::JVM_CLASS, "add",
"(Ljava/lang/String;J)V")?,
method_add_ret: ReturnType::Primitive(Primitive::Void),
class,
})
diff --git a/core/src/jvm_bridge/mod.rs b/core/src/jvm_bridge/mod.rs
index 91789adc..f91ae8fe 100644
--- a/core/src/jvm_bridge/mod.rs
+++ b/core/src/jvm_bridge/mod.rs
@@ -73,7 +73,7 @@ macro_rules! jni_call {
let ret = $env.call_method_unchecked($obj, method_id, ret_type, args);
// Check if JVM has thrown any exception, and handle it if so.
- let result = if let Some(exception) =
$crate::jvm_bridge::check_exception($env).unwrap() {
+ let result = if let Some(exception) =
$crate::jvm_bridge::check_exception($env)? {
Err(exception.into())
} else {
$crate::jvm_bridge::jni_map_error!($env, ret)
@@ -100,7 +100,7 @@ macro_rules! jni_static_call {
let ret = $env.call_static_method_unchecked(clazz, method_id,
ret_type, args);
// Check if JVM has thrown any exception, and handle it if so.
- let result = if let Some(exception) =
$crate::jvm_bridge::check_exception($env).unwrap() {
+ let result = if let Some(exception) =
$crate::jvm_bridge::check_exception($env)? {
Err(exception.into())
} else {
$crate::jvm_bridge::jni_map_error!($env, ret)
@@ -262,10 +262,15 @@ impl JVMClasses<'_> {
}
/// Gets the JNIEnv for the current thread.
- pub fn get_env() -> AttachGuard<'static> {
+ pub fn get_env() -> CometResult<AttachGuard<'static>> {
unsafe {
let java_vm = JAVA_VM.get_unchecked();
- java_vm.attach_current_thread().unwrap()
+ java_vm.attach_current_thread().map_err(|e| {
+ CometError::Internal(format!(
+ "JVMClasses::get_env() failed to attach current thread:
{}",
+ e
+ ))
+ })
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
