Skip to content

Commit 0962924

Browse files
authored
Expression VTable (#5191)
Experiment with new style of vtable. Overview: * No more vtable macro. * "Encoding" struct is now combined with the vtable struct (we had an empty vtable struct before for no reason). * Instead of Arc<dyn Expr>, we have an Expression struct with vtable, opaque instance data, and children. * We may want this struct to be Arc'd also? * Because private data can be anything, simple expressions do not need a separate instance struct (e.g. Binary simply stores Operator enum as private data) * We retain benefits of vtables in that we can run pre/post logic around typed impl functions. We can also expose a smaller API surface on Expression than we do on VTable. --------- Signed-off-by: Nicholas Gates <[email protected]>
1 parent 45ff246 commit 0962924

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

89 files changed

+2975
-3259
lines changed

Cargo.lock

Lines changed: 0 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

fuzz/fuzz_targets/file_io.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ fuzz_target!(|fuzz: FuzzFileAction| -> Corpus {
4242
let filtered = filter(&array_data, &mask).vortex_unwrap();
4343
projection_expr
4444
.clone()
45-
.unwrap_or_else(|| root())
45+
.unwrap_or_else(root)
4646
.evaluate(&Scope::new(filtered))
4747
.vortex_unwrap()
4848
};
@@ -69,7 +69,7 @@ fuzz_target!(|fuzz: FuzzFileAction| -> Corpus {
6969
.vortex_unwrap()
7070
.scan()
7171
.vortex_unwrap()
72-
.with_projection(projection_expr.unwrap_or_else(|| root()))
72+
.with_projection(projection_expr.unwrap_or_else(root))
7373
.with_some_filter(filter_expr)
7474
.into_array_iter(&*RUNTIME)
7575
.vortex_unwrap()

fuzz/src/file/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,16 @@
44
use libfuzzer_sys::arbitrary::{Arbitrary, Unstructured};
55
use vortex_array::ArrayRef;
66
use vortex_array::arrays::arbitrary::ArbitraryArray;
7-
use vortex_expr::ExprRef;
7+
use vortex_expr::Expression;
88
use vortex_expr::arbitrary::{filter_expr, projection_expr};
99

1010
use crate::array::CompressorStrategy;
1111

1212
#[derive(Debug)]
1313
pub struct FuzzFileAction {
1414
pub array: ArrayRef,
15-
pub projection_expr: Option<ExprRef>,
16-
pub filter_expr: Option<ExprRef>,
15+
pub projection_expr: Option<Expression>,
16+
pub filter_expr: Option<Expression>,
1717
pub compressor_strategy: CompressorStrategy,
1818
}
1919

java/testfiles/Cargo.lock

Lines changed: 0 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

java/vortex-jni/src/main/java/dev/vortex/api/expressions/Binary.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ public static Binary ltEq(Expression left, Expression right) {
172172

173173
@Override
174174
public String id() {
175-
return "binary";
175+
return "vortex.binary";
176176
}
177177

178178
@Override

java/vortex-jni/src/main/java/dev/vortex/api/expressions/GetItem.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public String getPath() {
7878

7979
@Override
8080
public String id() {
81-
return "get_item";
81+
return "vortex.get_item";
8282
}
8383

8484
@Override

java/vortex-jni/src/main/java/dev/vortex/api/expressions/Literal.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public T getValue() {
7272

7373
@Override
7474
public String id() {
75-
return "literal";
75+
return "vortex.literal";
7676
}
7777

7878
@Override

java/vortex-jni/src/main/java/dev/vortex/api/expressions/Not.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public int hashCode() {
6363

6464
@Override
6565
public String id() {
66-
return "not";
66+
return "vortex.not";
6767
}
6868

6969
@Override

java/vortex-jni/src/main/java/dev/vortex/api/expressions/Root.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public static Root parse(byte[] _metadata, List<Expression> children) {
3939

4040
@Override
4141
public String id() {
42-
return "root";
42+
return "vortex.root";
4343
}
4444

4545
@Override

java/vortex-jni/src/main/java/dev/vortex/api/proto/Expressions.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,15 @@ public static Expression deserialize(ExprProtos.Expr expr) {
4646
expr.getChildrenList().stream().map(Expressions::deserialize).collect(Collectors.toList());
4747

4848
switch (expr.getId()) {
49-
case "binary":
49+
case "vortex.binary":
5050
return Binary.parse(metadata, children);
51-
case "get_item":
51+
case "vortex.get_item":
5252
return GetItem.parse(metadata, children);
53-
case "root":
53+
case "vortex.root":
5454
return Root.parse(metadata, children);
55-
case "literal":
55+
case "vortex.literal":
5656
return Literal.parse(metadata, children);
57-
case "not":
57+
case "vortex.not":
5858
return Not.parse(metadata, children);
5959
default:
6060
return new Unknown(expr.getId(), children, expr.getMetadata().toByteArray());

0 commit comments

Comments
 (0)