Skip to content

Commit

Permalink
Resolve a issue that JNA could not map bool correctly (#2376)
Browse files Browse the repository at this point in the history
ref: https://stackoverflow.com/questions/55225896/jna-maps-java-boolean-to-1-integer

Fix the bug of #1814 roots from the
mapping of Boolean in Java to C. 
Probably [this PR](rust-lang/rust#89887) within rust version
1.61 trigger that, though I'm not sure.

Co-authored-by: shirly121 <yihe.zxl@alibaba-inc.com>
Co-authored-by: BingqingLyu <lv_bingqing@163.com>
  • Loading branch information
3 people committed Jan 10, 2023
1 parent 553b517 commit 59367f8
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 15 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/k8s-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -419,9 +419,9 @@ jobs:
- name: Helm Test
run: |
cd charts
helm template graphscope --set image.registry="",image.tag=${SHORT_SHA} \
helm template graphscope --set image.registry="",image.tag="${SHORT_SHA}" \
./graphscope
helm install graphscope --set image.registry="",image.tag=${SHORT_SHA} \
helm install graphscope --set image.registry="",image.tag="${SHORT_SHA}" \
./graphscope
helm test graphscope --timeout 5m0s
export NODE_IP=$(kubectl get pod -lgraphscope.coordinator.name=coordinator-graphscope -ojsonpath="{.items[0].status.hostIP}")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.alibaba.graphscope.common.jna;

import com.sun.jna.FromNativeContext;
import com.sun.jna.ToNativeContext;
import com.sun.jna.TypeConverter;

// to convert java boolean to i32 and define i32 as the native bool type
public class BooleanConverter implements TypeConverter {
@Override
public Object toNative(Object value, ToNativeContext context) {
return Integer.valueOf(Boolean.TRUE.equals(value) ? 1 : 0);
}

@Override
public Object fromNative(Object value, FromNativeContext context) {
return ((Integer) value).intValue() != 0 ? Boolean.TRUE : Boolean.FALSE;
}

@Override
public Class<?> nativeType() {
// BOOL is 32-bit int
return Integer.class;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ public class IrTypeMapper extends DefaultTypeMapper {
private IrTypeMapper() {
super();
addTypeConverter(IntEnum.class, new EnumConverter());
addTypeConverter(Boolean.class, new BooleanConverter());
}
}
5 changes: 0 additions & 5 deletions interactive_engine/executor/ir/core/rust-toolchain.toml

This file was deleted.

9 changes: 6 additions & 3 deletions interactive_engine/executor/ir/core/src/plan/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ impl TryFrom<FfiVariable> for common_pb::Variable {
#[derive(Default)]
pub struct FfiAlias {
alias: FfiNameOrId,
is_query_given: bool,
is_query_given: i32,
}

impl TryFrom<FfiAlias> for Option<common_pb::NameOrId> {
Expand Down Expand Up @@ -942,8 +942,11 @@ mod project {
use super::*;
/// To initialize a project operator.
#[no_mangle]
pub extern "C" fn init_project_operator(is_append: bool) -> *const c_void {
let project = Box::new(pb::Project { mappings: vec![], is_append });
pub extern "C" fn init_project_operator(is_append: i32) -> *const c_void {
let project = Box::new(pb::Project {
mappings: vec![],
is_append: if is_append == 0 { false } else { true },
});
Box::into_raw(project) as *const c_void
}

Expand Down
5 changes: 0 additions & 5 deletions interactive_engine/executor/rust-toolchain.toml

This file was deleted.

0 comments on commit 59367f8

Please sign in to comment.