Skip to content

Commit e6471ae

Browse files
authored
Merge branch 'main' into fix/track-c-empty-branch-handling
2 parents 95f0580 + e5ca181 commit e6471ae

14 files changed

Lines changed: 451 additions & 67 deletions

File tree

benchmarks/social_network/schemas/social_benchmark.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ graph_schema:
6262
- type: AUTHORED
6363
database: brahmand
6464
table: posts_bench
65-
from_id: author_id
65+
from_id: user_id # FIXED: Actual column is user_id, not author_id
6666
to_id: post_id
6767
edge_id: post_id
6868
from_node: User
@@ -78,7 +78,7 @@ graph_schema:
7878
from_node: User
7979
to_node: Post
8080
property_mappings:
81-
like_date: like_date
81+
like_date: liked_at # FIXED: Actual column is liked_at, not like_date
8282

8383
- type: FRIENDS_WITH
8484
database: brahmand

src/clickhouse_query_generator/to_sql.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -295,17 +295,17 @@ impl ToSql for LogicalExpr {
295295
))
296296
}
297297
LogicalExpr::MapLiteral(entries) => {
298-
// Map literals are handled specially by function translator
299-
// If we reach here directly, just format as key-value pairs for debugging
300-
// In practice, duration({days: 5}) is handled by translate_scalar_function
301-
let pairs: Result<Vec<String>, _> = entries
302-
.iter()
303-
.map(|(k, v)| {
304-
let val_sql = v.to_sql()?;
305-
Ok(format!("'{}': {}", k, val_sql))
306-
})
307-
.collect();
308-
Ok(format!("{{{}}}", pairs?.join(", ")))
298+
// Use ClickHouse map() function for map literals
299+
// map('key1', val1, 'key2', val2, ...)
300+
if entries.is_empty() {
301+
Ok("map()".to_string())
302+
} else {
303+
let args: Result<Vec<String>, _> = entries
304+
.iter()
305+
.flat_map(|(k, v)| vec![Ok(format!("'{}'", k)), v.to_sql()])
306+
.collect();
307+
Ok(format!("map({})", args?.join(", ")))
308+
}
309309
}
310310
LogicalExpr::LabelExpression { variable, label } => {
311311
// Label expression should typically be resolved at planning time

src/clickhouse_query_generator/to_sql_query.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2365,17 +2365,20 @@ impl RenderExpr {
23652365
)
23662366
}
23672367
RenderExpr::MapLiteral(entries) => {
2368-
// Map literals are handled specially by function translator
2369-
// If we reach here directly, just format as key-value pairs for debugging
2370-
// In practice, duration({days: 5}) is handled by translate_scalar_function
2371-
let pairs: Vec<String> = entries
2372-
.iter()
2373-
.map(|(k, v)| {
2374-
let val_sql = v.to_sql();
2375-
format!("'{}': {}", k, val_sql)
2376-
})
2377-
.collect();
2378-
format!("{{{}}}", pairs.join(", "))
2368+
// Use ClickHouse map() function for map literals
2369+
// map('key1', val1, 'key2', val2, ...)
2370+
if entries.is_empty() {
2371+
"map()".to_string()
2372+
} else {
2373+
let args: Vec<String> = entries
2374+
.iter()
2375+
.flat_map(|(k, v)| {
2376+
let val_sql = v.to_sql();
2377+
vec![format!("'{}'", k), val_sql]
2378+
})
2379+
.collect();
2380+
format!("map({})", args.join(", "))
2381+
}
23792382
}
23802383
RenderExpr::PatternCount(pc) => {
23812384
// Use the pre-generated SQL from PatternCount (correlated subquery)

src/query_planner/analyzer/filter_tagging.rs

Lines changed: 93 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -686,21 +686,65 @@ impl FilterTagging {
686686
}
687687

688688
// Get the label for this table
689+
// First try plan_ctx (works for typed patterns)
690+
// If None, try to find it in the plan tree (works for UNION branches where
691+
// each branch has a typed GraphNode but the global plan_ctx still has untyped entry)
689692
let label = match table_ctx.get_label_opt() {
690-
Some(l) => l,
691-
None => {
692-
// No label yet - this is an untyped pattern like MATCH (n)
693-
// Property validation will happen during scan generation when we filter types
694-
// For now, pass through the property access as-is
695-
log::info!(
696-
"🔧 FilterTagging: Skipping validation for untyped pattern '{}', property='{}' (will validate during scan generation)",
697-
property_access.table_alias.0,
698-
property_access.column.raw()
699-
);
700-
return Ok(LogicalExpr::PropertyAccessExp(property_access));
693+
Some(l) if !l.is_empty() => l, // Skip empty labels (from pruned branches)
694+
_ => {
695+
// Try to get label from the plan tree (for UNION branches)
696+
if let Some(plan_ref) = plan {
697+
if let Some(label_from_plan) =
698+
Self::find_label_in_plan(plan_ref, &property_access.table_alias.0)
699+
{
700+
if label_from_plan.is_empty() {
701+
// Empty label means this branch was pruned (no matching types)
702+
// Skip property mapping - the branch will return 0 rows anyway
703+
log::info!(
704+
"🔧 FilterTagging: Skipping property mapping for pruned branch '{}', property='{}'",
705+
property_access.table_alias.0,
706+
property_access.column.raw()
707+
);
708+
return Ok(LogicalExpr::PropertyAccessExp(property_access));
709+
}
710+
log::info!(
711+
"🔧 FilterTagging: Found label '{}' from plan tree for untyped pattern '{}', property='{}'",
712+
label_from_plan,
713+
property_access.table_alias.0,
714+
property_access.column.raw()
715+
);
716+
label_from_plan
717+
} else {
718+
// No label in plan either - truly untyped, pass through as-is
719+
log::info!(
720+
"🔧 FilterTagging: Skipping validation for untyped pattern '{}', property='{}' (will validate during scan generation)",
721+
property_access.table_alias.0,
722+
property_access.column.raw()
723+
);
724+
return Ok(LogicalExpr::PropertyAccessExp(property_access));
725+
}
726+
} else {
727+
// No plan reference - pass through as-is
728+
log::info!(
729+
"🔧 FilterTagging: Skipping validation for untyped pattern '{}', property='{}' (no plan context)",
730+
property_access.table_alias.0,
731+
property_access.column.raw()
732+
);
733+
return Ok(LogicalExpr::PropertyAccessExp(property_access));
734+
}
701735
}
702736
};
703737

738+
// Handle empty label case (branch was pruned by property-based filtering)
739+
if label.is_empty() {
740+
log::info!(
741+
"🔧 FilterTagging: Skipping property mapping for pruned branch '{}' (empty label), property='{}'",
742+
property_access.table_alias.0,
743+
property_access.column.raw()
744+
);
745+
return Ok(LogicalExpr::PropertyAccessExp(property_access));
746+
}
747+
704748
// Check if this node uses EmbeddedInEdge strategy (denormalized access)
705749
let (is_embedded_in_edge, _owning_edge_info) = if let Some(plan) = plan {
706750
// Also check by traversing the plan to find which edge owns this node
@@ -1680,6 +1724,44 @@ impl FilterTagging {
16801724
}
16811725
}
16821726

1727+
/// Find the label for an alias by looking at GraphNode in the plan tree
1728+
/// This is used for UNION branches where the branch has a typed GraphNode
1729+
/// but the global plan_ctx still has the untyped entry
1730+
fn find_label_in_plan(plan: &LogicalPlan, alias: &str) -> Option<String> {
1731+
match plan {
1732+
LogicalPlan::GraphNode(node) => {
1733+
if node.alias == alias {
1734+
return node.label.clone();
1735+
}
1736+
Self::find_label_in_plan(&node.input, alias)
1737+
}
1738+
LogicalPlan::Filter(filter) => Self::find_label_in_plan(&filter.input, alias),
1739+
LogicalPlan::Projection(proj) => Self::find_label_in_plan(&proj.input, alias),
1740+
LogicalPlan::GraphJoins(joins) => Self::find_label_in_plan(&joins.input, alias),
1741+
LogicalPlan::GroupBy(gb) => Self::find_label_in_plan(&gb.input, alias),
1742+
LogicalPlan::OrderBy(ob) => Self::find_label_in_plan(&ob.input, alias),
1743+
LogicalPlan::Skip(skip) => Self::find_label_in_plan(&skip.input, alias),
1744+
LogicalPlan::Limit(limit) => Self::find_label_in_plan(&limit.input, alias),
1745+
LogicalPlan::Cte(cte) => Self::find_label_in_plan(&cte.input, alias),
1746+
LogicalPlan::GraphRel(rel) => {
1747+
if let Some(l) = Self::find_label_in_plan(&rel.left, alias) {
1748+
return Some(l);
1749+
}
1750+
if let Some(l) = Self::find_label_in_plan(&rel.center, alias) {
1751+
return Some(l);
1752+
}
1753+
Self::find_label_in_plan(&rel.right, alias)
1754+
}
1755+
LogicalPlan::CartesianProduct(cp) => {
1756+
if let Some(l) = Self::find_label_in_plan(&cp.left, alias) {
1757+
return Some(l);
1758+
}
1759+
Self::find_label_in_plan(&cp.right, alias)
1760+
}
1761+
_ => None,
1762+
}
1763+
}
1764+
16831765
/// Check if a plan has a CartesianProduct descendant
16841766
/// Used to identify cross-table join conditions that should not be extracted
16851767
fn has_cartesian_product_descendant(plan: &LogicalPlan) -> bool {

src/query_planner/analyzer/graph_join/cross_branch.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,11 @@ pub fn generate_relationship_uniqueness_constraints(
9898
continue;
9999
}
100100

101+
// Skip if either edge has no relationship types (filtered by property filtering)
102+
if edge1.rel_types.is_empty() || edge2.rel_types.is_empty() {
103+
continue;
104+
}
105+
101106
// Get relationship schemas to determine edge ID columns
102107
let rel1_schema = match graph_schema.get_rel_schema(&edge1.rel_types[0]) {
103108
Ok(schema) => schema,
@@ -244,6 +249,16 @@ pub fn generate_cross_branch_joins_from_metadata(
244249
let mut to_edges: HashMap<String, Vec<&super::metadata::PatternEdgeInfo>> = HashMap::new();
245250

246251
for edge in &edges {
252+
// Skip edges with no relationship types (filtered out by property filtering)
253+
if edge.rel_types.is_empty() {
254+
log::debug!(
255+
"🔍 Skipping edge {}->{}->{} with no relationship types (filtered)",
256+
edge.from_node,
257+
edge.alias,
258+
edge.to_node
259+
);
260+
continue;
261+
}
247262
let rel_schema = graph_schema
248263
.get_rel_schema(&edge.rel_types[0])
249264
.map_err(|e| AnalyzerError::GraphSchema {
@@ -336,6 +351,20 @@ fn create_cross_branch_join_from_edges(
336351
if is_from_side { "from_node" } else { "to_node" }
337352
);
338353

354+
// Guard: Skip if either edge has no relationship types (filtered by property filtering)
355+
if edge1.rel_types.is_empty() {
356+
return Err(AnalyzerError::SchemaNotFound(format!(
357+
"Edge '{}' has no relationship types (filtered by property filtering)",
358+
edge1.alias
359+
)));
360+
}
361+
if edge2.rel_types.is_empty() {
362+
return Err(AnalyzerError::SchemaNotFound(format!(
363+
"Edge '{}' has no relationship types (filtered by property filtering)",
364+
edge2.alias
365+
)));
366+
}
367+
339368
// Get relationship schemas
340369
let rel1_schema = graph_schema
341370
.get_rel_schema(&edge1.rel_types[0])

src/query_planner/analyzer/view_resolver.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,20 @@ impl<'a> ViewResolver<'a> {
153153
from_node: Option<&str>,
154154
to_node: Option<&str>,
155155
) -> Result<crate::graph_catalog::expression_parser::PropertyValue, AnalyzerError> {
156+
// Handle empty type_name (from pruned/empty branches)
157+
// Return property as-is - the branch will return 0 rows anyway
158+
if type_name.is_empty() {
159+
log::info!(
160+
"ViewResolver: Empty relationship type - returning property '{}' as-is (branch was pruned)",
161+
property
162+
);
163+
return Ok(
164+
crate::graph_catalog::expression_parser::PropertyValue::Column(
165+
property.to_string(),
166+
),
167+
);
168+
}
169+
156170
log::debug!(
157171
"ViewResolver: Resolving rel property: type_name={}, property={}, from_node={:?}, to_node={:?}",
158172
type_name, property, from_node, to_node

src/query_planner/logical_plan/match_clause/helpers.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,16 +133,19 @@ pub fn generate_scan(
133133
}
134134
} else {
135135
// Multiple node types - create UNION ALL of ViewScans
136-
log::info!(
137-
"Creating UNION of {} node types for labelless query: {:?}",
136+
log::warn!(
137+
"🔍 Creating UNION of {} node types for labelless query: {:?}",
138138
labels_to_use.len(),
139139
labels_to_use
140140
);
141141

142142
let mut union_inputs = Vec::new();
143143
for label in &labels_to_use {
144144
match super::try_generate_view_scan(&alias, label, plan_ctx)? {
145-
Some(view_scan) => union_inputs.push(view_scan),
145+
Some(view_scan) => {
146+
log::warn!("🔍 Added ViewScan for label '{}': {:?}", label, view_scan);
147+
union_inputs.push(view_scan);
148+
}
146149
None => {
147150
log::warn!("Skipping label '{}' - not found in schema", label);
148151
}
@@ -396,11 +399,18 @@ pub fn register_node_in_context(
396399
node_props: Vec<Property>,
397400
is_explicitly_named: bool,
398401
) {
402+
// Use Some(vec![]) for unlabeled nodes instead of None
403+
// This distinguishes unlabeled nodes from path variables (which use None)
404+
let labels_for_ctx = match node_label {
405+
Some(l) => Some(vec![l.clone()]),
406+
None => Some(vec![]), // Explicitly empty = unlabeled node (not a path)
407+
};
408+
399409
plan_ctx.insert_table_ctx(
400410
node_alias.to_string(),
401411
TableCtx::build(
402412
node_alias.to_string(),
403-
node_label.clone().map(|l| vec![l]),
413+
labels_for_ctx,
404414
node_props,
405415
false, // is_rel
406416
is_explicitly_named,

src/query_planner/logical_plan/match_clause/traversal.rs

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,27 @@ fn traverse_connected_pattern_with_mode<'a>(
298298
}
299299
};
300300

301+
// === HANDLE NO MATCHING RELATIONSHIP TYPES ===
302+
// If property filtering removed all relationship types, return Empty plan
303+
if rel_labels.is_none() {
304+
log::warn!(
305+
"🔀 No relationship types found for alias '{}' after property filtering - returning Empty",
306+
rel_alias
307+
);
308+
// Register the relationship alias with empty labels to prevent downstream errors
309+
plan_ctx.insert_table_ctx(
310+
rel_alias.clone(),
311+
crate::query_planner::plan_ctx::TableCtx::build(
312+
rel_alias.clone(),
313+
Some(vec![]), // Empty labels
314+
vec![],
315+
true,
316+
false,
317+
),
318+
);
319+
return Ok(Arc::new(LogicalPlan::Empty));
320+
}
321+
301322
// === FULLY UNTYPED MULTI-TYPE UNION EXPANSION ===
302323
// For patterns like ()-->() where BOTH nodes are untyped AND we have multiple relationship types,
303324
// generate a UNION where each branch processes as a typed pattern through the normal flow.
@@ -1356,10 +1377,42 @@ pub(super) fn traverse_node_pattern(
13561377
.iter()
13571378
.map(|branch| {
13581379
let is_denorm = is_denormalized_scan(branch);
1380+
// For UNION branches from untyped patterns (MATCH (n)), extract the label
1381+
// from the ViewScan source_table to enable property mapping in FilterTagging.
1382+
// The ViewScan was created by generate_scan with a specific node type.
1383+
let branch_label = if node_label.is_none() {
1384+
// Extract label from ViewScan's source_table
1385+
// The source_table format is "database.table_name" e.g. "brahmand.users_bench"
1386+
if let LogicalPlan::ViewScan(vs) = branch.as_ref() {
1387+
// Try to find the node label by looking up which node type uses this table
1388+
let table_name = vs.source_table.split('.').last().unwrap_or(&vs.source_table);
1389+
plan_ctx.schema().all_node_schemas()
1390+
.iter()
1391+
.find_map(|(label, schema)| {
1392+
// Check if this schema's table matches
1393+
let schema_table = schema.table_name.split('.').last().unwrap_or(&schema.table_name);
1394+
if schema_table == table_name {
1395+
Some(label.clone())
1396+
} else {
1397+
None
1398+
}
1399+
})
1400+
} else {
1401+
None
1402+
}
1403+
} else {
1404+
node_label.clone().map(|s| s.to_string())
1405+
};
1406+
1407+
log::info!(
1408+
" ✓ Wrapping branch with alias='{}', label={:?} (original node_label={:?})",
1409+
node_alias, branch_label, node_label
1410+
);
1411+
13591412
Arc::new(LogicalPlan::GraphNode(GraphNode {
13601413
input: branch.clone(),
13611414
alias: node_alias.clone(),
1362-
label: node_label.clone().map(|s| s.to_string()),
1415+
label: branch_label,
13631416
is_denormalized: is_denorm,
13641417
projected_columns: None,
13651418
}))

0 commit comments

Comments
 (0)