Skip to content
Merged
75 changes: 57 additions & 18 deletions src/clickhouse_query_generator/to_sql_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1153,16 +1153,38 @@ pub(crate) fn rewrite_expr_for_vlp(
})
}

RenderExpr::AggregateFnCall(agg) => RenderExpr::AggregateFnCall(AggregateFnCall {
name: agg.name.clone(),
args: agg
.args
.iter()
.map(|a| {
rewrite_expr_for_vlp(a, start_alias, end_alias, path_variable, skip_start_alias)
})
.collect(),
}),
RenderExpr::AggregateFnCall(agg) => {
// COUNT(path_variable) → COUNT(*) since each row represents a path
if let Some(path_var) = path_variable {
if agg.args.len() == 1 && agg.name.to_lowercase() == "count" {
if let RenderExpr::TableAlias(alias) = &agg.args[0] {
if &alias.0 == path_var {
log::info!("🔧 VLP path aggregate: count({}) → count(*)", path_var);
return RenderExpr::AggregateFnCall(AggregateFnCall {
name: agg.name.clone(),
args: vec![RenderExpr::Star],
});
}
}
}
}
RenderExpr::AggregateFnCall(AggregateFnCall {
name: agg.name.clone(),
args: agg
.args
.iter()
.map(|a| {
rewrite_expr_for_vlp(
a,
start_alias,
end_alias,
path_variable,
skip_start_alias,
)
})
.collect(),
})
}

RenderExpr::ColumnAlias(ColumnAlias(alias_str))
if path_variable.as_ref() == Some(alias_str) =>
Expand Down Expand Up @@ -1563,14 +1585,31 @@ fn rewrite_expr_for_fixed_path(
})
}

RenderExpr::AggregateFnCall(agg) => RenderExpr::AggregateFnCall(AggregateFnCall {
name: agg.name.clone(),
args: agg
.args
.iter()
.map(|a| rewrite_expr_for_fixed_path(a, path_variable, hop_count))
.collect(),
}),
RenderExpr::AggregateFnCall(agg) => {
// COUNT(path_variable) → COUNT(*) since each row represents a path
if agg.args.len() == 1 {
if let RenderExpr::TableAlias(alias) = &agg.args[0] {
if alias.0 == *path_variable && agg.name.to_lowercase() == "count" {
log::info!(
"🔧 Fixed path aggregate: count({}) → count(*)",
path_variable
);
return RenderExpr::AggregateFnCall(AggregateFnCall {
name: agg.name.clone(),
args: vec![RenderExpr::Star],
});
}
}
}
RenderExpr::AggregateFnCall(AggregateFnCall {
name: agg.name.clone(),
args: agg
.args
.iter()
.map(|a| rewrite_expr_for_fixed_path(a, path_variable, hop_count))
.collect(),
})
}

// Leave other expressions unchanged
other => other.clone(),
Expand Down
41 changes: 36 additions & 5 deletions src/open_cypher_parser/use_clause.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use nom::{
bytes::complete::{tag_no_case, take_while1},
branch::alt,
bytes::complete::{tag, tag_no_case, take_while1},
error::context,
sequence::delimited,
IResult, Parser,
};

Expand All @@ -10,17 +12,20 @@ use super::{ast::UseClause, common::ws, errors::OpenCypherParsingError};
/// Examples:
/// USE social_network
/// USE ecommerce
/// USE mydb
/// USE `my-database`
pub fn parse_use_clause<'a>(
input: &'a str,
) -> IResult<&'a str, UseClause<'a>, OpenCypherParsingError<'a>> {
let (input, _) = ws(tag_no_case("USE")).parse(input)?;

let (input, database_name) = context(
"Error parsing database name in USE clause",
ws(take_while1(|c: char| {
c.is_alphanumeric() || c == '_' || c == '.'
})),
ws(alt((
// Backtick-quoted identifier: USE `my-database`
delimited(tag("`"), take_while1(|c: char| c != '`'), tag("`")),
// Unquoted identifier
take_while1(|c: char| c.is_alphanumeric() || c == '_' || c == '.'),
))),
)
.parse(input)?;

Expand Down Expand Up @@ -92,6 +97,32 @@ mod tests {
assert!(res.is_err(), "Should fail when database name is missing");
}

#[test]
fn test_parse_use_clause_backtick_quoted() {
let input = "USE `my-database` MATCH (n) RETURN n";
let res = parse_use_clause(input);
match res {
Ok((remaining, use_clause)) => {
assert_eq!(use_clause.database_name, "my-database");
assert_eq!(remaining, "MATCH (n) RETURN n");
}
Err(e) => panic!("Failed to parse backtick-quoted USE clause: {:?}", e),
}
}

#[test]
fn test_parse_use_clause_backtick_with_special_chars() {
let input = "USE `zeek_logs` MATCH (n) RETURN n";
let res = parse_use_clause(input);
match res {
Ok((remaining, use_clause)) => {
assert_eq!(use_clause.database_name, "zeek_logs");
assert_eq!(remaining, "MATCH (n) RETURN n");
}
Err(e) => panic!("Failed to parse backtick-quoted USE clause: {:?}", e),
}
}

#[test]
fn test_parse_use_clause_numeric_start() {
let input = "USE 123db";
Expand Down
15 changes: 7 additions & 8 deletions src/query_planner/logical_plan/match_clause/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,16 +422,15 @@ pub fn register_path_variable(
);

// Then register TableCtx for backward compatibility with code that uses alias_table_ctx_map
plan_ctx.insert_table_ctx(
let mut table_ctx = TableCtx::build(
path_var.to_string(),
TableCtx::build(
path_var.to_string(),
None, // Path variables don't have labels
vec![], // Path variables don't have properties
false, // Not a relationship
true, // Explicitly named by user
),
None, // Path variables don't have labels
vec![], // Path variables don't have properties
false, // Not a relationship
true, // Explicitly named by user
);
table_ctx.set_is_path(true);
plan_ctx.insert_table_ctx(path_var.to_string(), table_ctx);

log::info!(
"📍 Registered path variable '{}' with TypedVariable::Path (start={}, end={}, bounds={:?})",
Expand Down
24 changes: 18 additions & 6 deletions src/query_planner/optimizer/collect_unwind_elimination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,12 +518,24 @@ impl CollectUnwindElimination {

// Add the source variable as a passthrough item so it's exported
// (the UNWIND alias will be rewritten to reference this)
new_items.push(ProjectionItem {
expression: LogicalExpr::TableAlias(TableAlias(
source.clone(),
)),
col_alias: Some(ColumnAlias(source.clone())),
});
// Only add if source isn't already in the remaining items
let source_already_present =
new_items.iter().any(|item| {
match &item.expression {
LogicalExpr::TableAlias(ta) => {
ta.0 == source
}
_ => false,
}
});
if !source_already_present {
new_items.push(ProjectionItem {
expression: LogicalExpr::TableAlias(
TableAlias(source.clone()),
),
col_alias: Some(ColumnAlias(source.clone())),
});
}

// Update exported aliases: remove collection, add source
let mut new_exported_aliases: Vec<String> = with
Expand Down
15 changes: 12 additions & 3 deletions src/query_planner/plan_ctx/table_ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ pub struct TableCtx {
/// For relationships: the label of the connected to_node (target)
/// Used to resolve polymorphic relationships
to_node_label: Option<String>,
/// Whether this alias represents a path variable (from `MATCH p = ...`)
is_path: bool,
}

impl TableCtx {
Expand All @@ -63,6 +65,7 @@ impl TableCtx {
cte_reference: None,
from_node_label: None,
to_node_label: None,
is_path: false,
}
}

Expand Down Expand Up @@ -108,6 +111,7 @@ impl TableCtx {
cte_reference: Some(cte_name),
from_node_label: None,
to_node_label: None,
is_path: false,
}
}

Expand Down Expand Up @@ -135,6 +139,9 @@ impl TableCtx {
/// Path variables have no label (None) and are not relationships.
/// Anonymous nodes also have None labels, so we exclude aliases matching "t\d+" pattern.
pub fn is_path_variable(&self) -> bool {
if self.is_path {
return true;
}
if self.is_rel {
return false;
}
Expand All @@ -143,9 +150,6 @@ impl TableCtx {
}
// Computed/scalar aliases from WITH (e.g., `WITH size(...) AS cnt`)
// have explicit_alias=true but no labels — they are NOT path variables.
// Note: Real path variables (e.g., `p` in `MATCH p = ...`) are also
// explicit_alias=true, but they are registered via define_path() directly
// in match_clause/traversal.rs, not through this heuristic.
if self.explicit_alias {
return false;
}
Expand All @@ -160,6 +164,11 @@ impl TableCtx {
true // No labels, not a relationship, not an anonymous node → path variable
}

/// Mark this TableCtx as a path variable.
pub fn set_is_path(&mut self, val: bool) {
self.is_path = val;
}

// ========================================================================
// Label/Type Access
// ========================================================================
Expand Down
62 changes: 62 additions & 0 deletions src/render_plan/cte_generation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::collections::HashMap;
use crate::clickhouse_query_generator::variable_length_cte::WeightCteConfig;
use crate::clickhouse_query_generator::NodeProperty;
use crate::graph_catalog::config::Identifier;
use crate::graph_catalog::expression_parser::PropertyValue;
use crate::graph_catalog::graph_schema::GraphSchema;
use crate::query_planner::logical_expr::LogicalExpr;
use crate::query_planner::logical_plan::LogicalPlan;
Expand Down Expand Up @@ -783,6 +784,67 @@ pub fn map_property_to_column_with_relationship_context(
Ok(column.raw().to_string())
}

/// Like `map_property_to_column_with_relationship_context` but preserves
/// the `PropertyValue` variant (Column vs Expression). Use this when the
/// caller needs to generate correct SQL for expression-based mappings.
pub fn map_property_to_property_value(
property: &str,
node_label: &str,
) -> Result<PropertyValue, String> {
let current_schema = crate::server::query_context::get_current_schema_with_fallback();
let schema = current_schema.as_ref().ok_or_else(|| {
format!(
"No schema available for property '{}' on node '{}'",
property, node_label
)
})?;

let node_schema = schema
.all_node_schemas()
.get(node_label)
.ok_or_else(|| format!("Node label '{}' not found in schema", node_label))?;

node_schema
.property_mappings
.get(property)
.cloned()
.ok_or_else(|| {
format!(
"Property '{}' not found for node label '{}'",
property, node_label
)
})
}

/// Like `map_property_to_property_value` but for relationship properties.
pub fn map_rel_property_to_property_value(
property: &str,
relationship_type: &str,
) -> Result<PropertyValue, String> {
let current_schema = crate::server::query_context::get_current_schema_with_fallback();
let schema = current_schema.as_ref().ok_or_else(|| {
format!(
"No schema available for property '{}' on rel '{}'",
property, relationship_type
)
})?;

let rel_schema = schema
.get_rel_schema(relationship_type)
.map_err(|e| format!("Relationship type '{}' not found: {}", relationship_type, e))?;

rel_schema
.property_mappings
.get(property)
.cloned()
.ok_or_else(|| {
format!(
"Property '{}' not found for relationship type '{}'",
property, relationship_type
)
})
}

/// Map a relationship property to its corresponding column name in the schema.
/// This is the relationship equivalent of map_property_to_column_with_schema.
///
Expand Down
Loading