Skip to content

Conversation

@1sand0s
Copy link

@1sand0s 1sand0s commented Jun 12, 2025

This pull request merges the latest four commits on aditya/sym_eval into michael/sym-eval.

SHA   Commit message
54daa6c Propagate symbolic data from input primitive array to primitive array constructed from indexes in take
0d8607c Change sym! macro to pair every element of the two batches (quadratic join)
c0dc1a4 Introduce VarType enum and arrow_type_to_var_type for correct Z3 typing. Add row offset field to symbolic variable to support unions
a7740a7 Remove stray dbg! calls across array / select modules

1sand0s and others added 5 commits June 9, 2025 01:01
Exposing arrow datatype to z3 type converter in mod
The  module defines methods to construct primitive array from indexes of the input primitive array. We need to make sure that the symbolic data of the input primitive array is copied into the constructed primitive array
@1sand0s 1sand0s requested review from gliga and milevin June 12, 2025 10:18
@github-actions github-actions bot added the arrow label Jun 12, 2025
}

fn to_symbolic_data(&self) -> super::SymbolicArrayData {
dbg!(&self.data_type, &self, &self.symbolic_data);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how did we end up with dbg lines in the first place? For other repos we have a protection, should we lint here as well?

Boolean,
/// A string variable
String,
/// A int variable
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: An int

DataType::Int8 | DataType::Int16 | DataType::Int32 | DataType::Int64 |
DataType::UInt8 | DataType::UInt16 | DataType::UInt32 | DataType::UInt64 |
DataType::Decimal128(_, _) | DataType::Decimal256(_, _) => VarType::Int,
_ => VarType::String, // Default to String for other types
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are those other types and why is string ok? Is it safer to return an error in this case

r.clone(),
));
for j in 0..syms2.len() {
let l = &syms1[i];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this not outside the nested loop?

} else {
vec![]
};
all_constraints.extend(constraints.unwrap_or_default());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

potentially move into the branches above to avoid mut

Copy link

@gliga gliga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me, but check with @milevin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants