Skip to content

Commit 5f65b9c

Browse files
xav-dbishaksebsib
andcommitted
feat(helixc): integrate PR #666 - fix IS_IN validation and reserved property handling
## Changes ### 1. Added Reserved Property Support - Created `ReservedProp` enum for built-in properties (id, label, version, etc.) - Added `Step::ReservedPropertyAccess` variant to generate direct field access - Reserved properties now return correct Value types (e.g., Value::Id for id field) ### 2. Updated Property Access Generator - Modified `gen_property_access()` to identify reserved vs user-defined properties - Reserved properties use direct struct field access - User-defined properties continue using `get_property()` HashMap lookup ### 3. Enhanced Boolean Operation Optimization - Updated `WhereRef` Display implementation for reserved property optimization - Generates `Value::Id(ID::from(val.id))` for id field in filters - Maintains `get_property()` optimization for user-defined properties ### 4. Fixed IS_IN Validation - Separated IS_IN from scalar boolean operations - IS_IN now correctly expects `Type::Array` arguments with scalar elements - Added `get_reserved_property_type()` helper for type validation - Updated Node/Edge/Vector validation to check reserved properties first ### 5. Testing - Added IS_IN test with both reserved (id) and user-defined (field) properties - Verified correct code generation and type safety ## Impact Resolves the issues from original PR #666: - IS_IN with arrays now validates at compile-time - Reserved property access returns correct Value types for IS_IN operations - Queries like `WHERE(_::{id}::IS_IN(node_ids))` now compile and execute correctly Co-Authored-By: ishaksebsib <[email protected]>
1 parent b146b83 commit 5f65b9c

File tree

7 files changed

+587
-69
lines changed

7 files changed

+587
-69
lines changed

helix-db/src/helixc/analyzer/methods/traversal_validation.rs

Lines changed: 223 additions & 64 deletions
Large diffs are not rendered by default.

helix-db/src/helixc/analyzer/utils.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::{
55
helixc::{
66
analyzer::{Ctx, errors::push_query_err, types::Type},
77
generator::{
8-
traversal_steps::Step,
8+
traversal_steps::{Step, ReservedProp},
99
utils::{GenRef, GeneratedValue},
1010
},
1111
parser::{location::Loc, types::*},
@@ -185,8 +185,15 @@ pub(super) fn get_field_type_from_item_fields(
185185

186186
pub(super) fn gen_property_access(name: &str) -> Step {
187187
match name {
188-
"id" => Step::PropertyFetch(GenRef::Literal("id".to_string())),
189-
"ID" => Step::PropertyFetch(GenRef::Literal("id".to_string())),
188+
"id" | "ID" | "Id" => Step::ReservedPropertyAccess(ReservedProp::Id),
189+
"label" | "Label" => Step::ReservedPropertyAccess(ReservedProp::Label),
190+
"version" | "Version" => Step::ReservedPropertyAccess(ReservedProp::Version),
191+
"from_node" | "fromNode" | "FromNode" => Step::ReservedPropertyAccess(ReservedProp::FromNode),
192+
"to_node" | "toNode" | "ToNode" => Step::ReservedPropertyAccess(ReservedProp::ToNode),
193+
"deleted" | "Deleted" => Step::ReservedPropertyAccess(ReservedProp::Deleted),
194+
"level" | "Level" => Step::ReservedPropertyAccess(ReservedProp::Level),
195+
"distance" | "Distance" => Step::ReservedPropertyAccess(ReservedProp::Distance),
196+
"data" | "Data" => Step::ReservedPropertyAccess(ReservedProp::Data),
190197
n => Step::PropertyFetch(GenRef::Literal(n.to_string())),
191198
}
192199
}

helix-db/src/helixc/generator/traversal_steps.rs

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,21 @@ impl Traversal {
192192
result
193193
}
194194
}
195+
196+
/// Reserved properties that are accessed directly from struct fields
197+
#[derive(Clone, Debug)]
198+
pub enum ReservedProp {
199+
Id,
200+
Label,
201+
Version,
202+
FromNode,
203+
ToNode,
204+
Deleted,
205+
Level,
206+
Distance,
207+
Data,
208+
}
209+
195210
#[derive(Clone)]
196211
pub enum Step {
197212
// graph steps
@@ -217,6 +232,7 @@ pub enum Step {
217232

218233
// property
219234
PropertyFetch(GenRef<String>),
235+
ReservedPropertyAccess(ReservedProp),
220236

221237
// closure
222238
// Closure(ClosureRemapping),
@@ -243,6 +259,17 @@ impl Display for Step {
243259
Step::ToN => write!(f, "to_n()"),
244260
Step::ToV(to_v) => write!(f, "{to_v}"),
245261
Step::PropertyFetch(property) => write!(f, "get_property({property})"),
262+
Step::ReservedPropertyAccess(prop) => match prop {
263+
ReservedProp::Id => write!(f, "map(|item| Ok(Value::from(uuid_str(item.id, &arena))))"),
264+
ReservedProp::Label => write!(f, "map(|item| Ok(Value::from(item.label)))"),
265+
ReservedProp::Version => write!(f, "map(|item| Ok(Value::from(item.version)))"),
266+
ReservedProp::FromNode => write!(f, "map(|item| Ok(Value::from(uuid_str(item.from_node, &arena))))"),
267+
ReservedProp::ToNode => write!(f, "map(|item| Ok(Value::from(uuid_str(item.to_node, &arena))))"),
268+
ReservedProp::Deleted => write!(f, "map(|item| Ok(Value::from(item.deleted)))"),
269+
ReservedProp::Level => write!(f, "map(|item| Ok(Value::from(item.level)))"),
270+
ReservedProp::Distance => write!(f, "map(|item| Ok(item.distance.map(Value::from).unwrap_or(Value::Empty)))"),
271+
ReservedProp::Data => write!(f, "map(|item| Ok(Value::from(item.data)))"),
272+
},
246273

247274
Step::Out(out) => write!(f, "{out}"),
248275
Step::In(in_) => write!(f, "{in_}"),
@@ -271,6 +298,7 @@ impl Debug for Step {
271298
Step::FromN => write!(f, "FromN"),
272299
Step::ToN => write!(f, "ToN"),
273300
Step::PropertyFetch(property) => write!(f, "get_property({property})"),
301+
Step::ReservedPropertyAccess(prop) => write!(f, "ReservedProperty({:?})", prop),
274302
Step::FromV(_) => write!(f, "FromV"),
275303
Step::ToV(_) => write!(f, "ToV"),
276304
Step::Out(_) => write!(f, "Out"),
@@ -401,23 +429,63 @@ impl Display for WhereRef {
401429
let is_val = matches!(var, GenRef::Std(s) | GenRef::Literal(s) if s == "val");
402430

403431
if is_val && traversal.steps.len() == 2 {
404-
// Check if we have PropertyFetch followed by BoolOp
432+
// Check if we have PropertyFetch or ReservedPropertyAccess followed by BoolOp
405433
let mut prop: Option<&GenRef<String>> = None;
434+
let mut reserved_prop: Option<&ReservedProp> = None;
406435
let mut bool_op: Option<&BoolOp> = None;
407436

408437
for step in &traversal.steps {
409438
match step {
410439
Separator::Period(Step::PropertyFetch(p))
411440
| Separator::Newline(Step::PropertyFetch(p))
412441
| Separator::Empty(Step::PropertyFetch(p)) => prop = Some(p),
442+
Separator::Period(Step::ReservedPropertyAccess(rp))
443+
| Separator::Newline(Step::ReservedPropertyAccess(rp))
444+
| Separator::Empty(Step::ReservedPropertyAccess(rp)) => reserved_prop = Some(rp),
413445
Separator::Period(Step::BoolOp(op))
414446
| Separator::Newline(Step::BoolOp(op))
415447
| Separator::Empty(Step::BoolOp(op)) => bool_op = Some(op),
416448
_ => {}
417449
}
418450
}
419451

420-
// If we found both PropertyFetch and BoolOp, generate optimized code
452+
// Handle ReservedPropertyAccess with BoolOp - generate direct field access
453+
if let (Some(reserved_prop), Some(bool_op)) = (reserved_prop, bool_op) {
454+
let value_expr = match reserved_prop {
455+
ReservedProp::Id => "Value::Id(ID::from(val.id))".to_string(),
456+
ReservedProp::Label => "Value::from(val.label)".to_string(),
457+
ReservedProp::Version => "Value::from(val.version)".to_string(),
458+
ReservedProp::FromNode => "Value::Id(ID::from(val.from_node))".to_string(),
459+
ReservedProp::ToNode => "Value::Id(ID::from(val.to_node))".to_string(),
460+
ReservedProp::Deleted => "Value::from(val.deleted)".to_string(),
461+
ReservedProp::Level => "Value::from(val.level)".to_string(),
462+
ReservedProp::Distance => "val.distance.map(Value::from).unwrap_or(Value::Empty)".to_string(),
463+
ReservedProp::Data => "Value::from(val.data)".to_string(),
464+
};
465+
let bool_expr = match bool_op {
466+
BoolOp::Gt(gt) => format!("{}{}", value_expr, gt),
467+
BoolOp::Gte(gte) => format!("{}{}", value_expr, gte),
468+
BoolOp::Lt(lt) => format!("{}{}", value_expr, lt),
469+
BoolOp::Lte(lte) => format!("{}{}", value_expr, lte),
470+
BoolOp::Eq(eq) => format!("{}{}", value_expr, eq),
471+
BoolOp::Neq(neq) => format!("{}{}", value_expr, neq),
472+
BoolOp::Contains(contains) => format!("{}{}", value_expr, contains),
473+
BoolOp::IsIn(is_in) => format!("{}{}", value_expr, is_in),
474+
};
475+
return write!(
476+
f,
477+
"filter_ref(|val, txn|{{
478+
if let Ok(val) = val {{
479+
Ok({})
480+
}} else {{
481+
Ok(false)
482+
}}
483+
}})",
484+
bool_expr
485+
);
486+
}
487+
488+
// Handle PropertyFetch with BoolOp - use get_property
421489
if let (Some(prop), Some(bool_op)) = (prop, bool_op) {
422490
let bool_expr = match bool_op {
423491
BoolOp::Gt(gt) => format!("*v{gt}"),

hql-tests/tests/is_in/helix.toml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
[project]
2+
name = "mcp_macro"
3+
queries = "."
4+
5+
[local.dev]
6+
port = 6969
7+
build_mode = "debug"
8+
9+
[cloud]

hql-tests/tests/is_in/queries.hx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
N::MyNode {
2+
field: String,
3+
}
4+
5+
6+
QUERY GetNodes (fields: [String]) =>
7+
node <- N<MyNode>::WHERE(_::{field}::IS_IN(fields))
8+
RETURN node
9+
10+
11+
QUERY GetNodesByID (node_ids: [ID]) =>
12+
node <- N<MyNode>::WHERE(_::{id}::IS_IN(node_ids))
13+
RETURN node

0 commit comments

Comments
 (0)