Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions crates/lance-graph/src/logical_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1386,6 +1386,17 @@ impl<'a> LogicalPlanner<'a> {
}
}

// RETURN aliases shadow input entities in ORDER BY, after every item is bound.
// Whole-node aliases expand to `alias.property`, so keep their entity binding.
self.variables.retain(|variable, _| {
!projections.iter().any(|projection| {
projection
.alias
.as_ref()
.is_some_and(|alias| alias.eq_ignore_ascii_case(variable))
})
});

let mut plan = LogicalOperator::Project {
input: Box::new(input),
projections,
Expand Down Expand Up @@ -1530,6 +1541,44 @@ mod tests {
use super::*;
use crate::{parser::parse_cypher_query, NodeMapping};

#[test]
fn return_alias_shadows_node_only_after_projection() {
let config = GraphConfig::builder()
.node_table("Person", "id")
.build()
.unwrap();
let query =
parse_cypher_query("MATCH (p:Person) RETURN p.name AS P, p.id AS id ORDER BY p")
.unwrap();
let LogicalOperator::Sort { input, sort_items } =
LogicalPlanner::new(&config).plan(&query).unwrap()
else {
panic!("expected sort");
};
assert_eq!(
sort_items[0].expression,
ValueExpression::Variable("p".into())
);
let LogicalOperator::Project { projections, .. } = *input else {
panic!("expected projection");
};
assert_eq!(
projections[1].expression,
ValueExpression::Property(PropertyRef {
variable: "p".into(),
property: "id".into(),
})
);

let query = parse_cypher_query("MATCH (p:Person) RETURN p AS p ORDER BY p").unwrap();
let LogicalOperator::Sort { sort_items, .. } =
LogicalPlanner::new(&config).plan(&query).unwrap()
else {
panic!("expected sort");
};
assert_eq!(sort_items[0].expression, projections[1].expression);
}

#[test]
fn test_relationship_query_logical_plan_structure() {
let query_text = r#"MATCH (p:Person {name: "Alice"})-[:KNOWS]->(f:Person) WHERE f.age > 30 RETURN f.name"#;
Expand Down
8 changes: 4 additions & 4 deletions docs/cypher.md
Original file line number Diff line number Diff line change
Expand Up @@ -330,10 +330,10 @@ print(result.to_pylist())
Keywords, function names, variables, labels, relationship types and property names are all
case-insensitive. `match (P:person) return p.NAME` works, and the output column keeps the spelling you wrote.

!!! bug "Known issue: an alias that shadows a variable"

`RETURN p.name AS p` reuses the variable name `p` as an output alias, which can fail to plan once the
query also aggregates or uses `OPTIONAL MATCH`. Pick a different alias, such as `AS name`.
An output alias may reuse an input variable's name. Expressions in the same `RETURN` still use the input
variable; the following `ORDER BY` uses the output alias. For example,
`RETURN p.name AS p, p.id AS id ORDER BY p` returns names and IDs, sorted by name. This also works with
aggregation and `OPTIONAL MATCH`.

## Errors

Expand Down
74 changes: 74 additions & 0 deletions python/python/tests/test_alias_shadowing.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
"""Output aliases shadow input nodes only after their expressions are evaluated."""

import pytest
from lance_csr import CypherEngine
from test_engine_csr import CONFIG, KNOWS, PEOPLE


@pytest.mark.parametrize("use_csr", [False, True])
@pytest.mark.parametrize(
("query", "columns", "expected"),
[
(
"MATCH (p:Person) RETURN p.name AS p ORDER BY p",
["p"],
[(name,) for name in "abcdef"],
),
(
"MATCH (p:Person) RETURN p.name AS p, count(*) AS n ORDER BY p",
["p", "n"],
[(name, 1) for name in "abcdef"],
),
(
"MATCH (p:Person) OPTIONAL MATCH (p)-[:KNOWS]->(f:Person) "
"RETURN p.name AS p, f.name AS f ORDER BY p, f",
["p", "f"],
[
("a", "b"),
("a", "b"),
("a", "c"),
("b", "c"),
("c", "c"),
("d", None),
("e", "a"),
("f", None),
],
),
(
"MATCH (p:Person) OPTIONAL MATCH (p)-[:KNOWS]->(f:Person) "
"RETURN p.name AS p, count(f) AS n ORDER BY p",
["p", "n"],
list(zip("abcdef", [3, 1, 1, 0, 1, 0], strict=True)),
),
(
"MATCH (p:Person) RETURN p.name AS p, p.id AS id ORDER BY p",
["p", "id"],
list(zip("abcdef", range(1, 7), strict=True)),
),
(
"MATCH (p:Person) RETURN p.name AS P ORDER BY p DESC",
["p"],
[(name,) for name in "fedcba"],
),
(
"MATCH (p:Person) RETURN p.name AS p ORDER BY toUpper(p) DESC",
["p"],
[(name,) for name in "fedcba"],
),
(
"MATCH (p:Person) WITH p.name AS p ORDER BY p RETURN p",
["p"],
[(name,) for name in "abcdef"],
),
(
"MATCH (p:Person) RETURN p AS p ORDER BY p",
["p.id"],
[(key,) for key in range(1, 7)],
),
],
)
def test_output_alias_shadows_node(use_csr, query, columns, expected):
engine = CypherEngine(CONFIG, {"Person": PEOPLE, "KNOWS": KNOWS}, use_csr=use_csr)
result = engine.execute(query)
assert result.column_names == columns
assert [tuple(row.values()) for row in result.to_pylist()] == expected
Loading