From 3d51304ad0ed1b4ae89380d496ab3f783155a50e Mon Sep 17 00:00:00 2001 From: prrao87 Date: Tue, 3 Nov 2026 20:31:55 -0500 Subject: [PATCH] feat(query): support quoted identifiers in Cypher and SQL Cypher accepts backtick-quoted labels, relationship types, variables, property names and aliases, as openCypher does: a quoted name may hold spaces or other characters, may match a keyword, and writes a backtick as two. Inside GRAPH_TABLE, double quotes name identifiers as in SQL/PGQ and the surrounding SQL, so strings there need single quotes. --- crates/lance-csr/src/graph_table.rs | 42 ++++++- crates/lance-csr/src/parser.rs | 50 +++++--- .../tests/test_quoted_identifiers.rs | 112 ++++++++++++++++++ docs/cypher.md | 13 ++ docs/sql.md | 20 +++- .../python/tests/test_quoted_identifiers.py | 67 +++++++++++ 6 files changed, 284 insertions(+), 20 deletions(-) create mode 100644 crates/lance-csr/tests/test_quoted_identifiers.rs create mode 100644 python/python/tests/test_quoted_identifiers.py diff --git a/crates/lance-csr/src/graph_table.rs b/crates/lance-csr/src/graph_table.rs index 8205165..a20aa8a 100644 --- a/crates/lance-csr/src/graph_table.rs +++ b/crates/lance-csr/src/graph_table.rs @@ -146,12 +146,38 @@ fn to_cypher(body: &str) -> Result { if columns.is_empty() { return Err(syntax_error("COLUMNS (...) is empty")); } - Ok(format!( + sql_identifiers(&format!( "{} RETURN {columns}", body[match_at..columns_at].trim() )) } +/// SQL/PGQ quoting in Cypher text: each `"name"` (with `""` for `"`) becomes the Cypher +/// identifier `` `name` `` (with ``` `` ``` for `` ` ``). `'...'` strings and `` `...` `` names are kept. +fn sql_identifiers(text: &str) -> Result { + let b = text.as_bytes(); + let mut out = String::with_capacity(text.len()); + let (mut copied, mut i) = (0, 0); + while i < b.len() { + if !is_quote(b[i]) { + i += 1; + continue; + } + let end = skip_quoted(b, i)?; + if b[i] == b'"' { + out.push_str(&text[copied..i]); + let name = text[i + 1..end - 1].replace("\"\"", "\""); + out.push('`'); + out.push_str(&name.replace('`', "``")); + out.push('`'); + copied = end; + } + i = end; + } + out.push_str(&text[copied..]); + Ok(out) +} + /// Byte offsets of `word` outside quotes and brackets. fn top_level_words(b: &[u8], word: &str) -> Result> { let (mut found, mut depth, mut i) = (Vec::new(), 0usize, 0); @@ -199,7 +225,7 @@ fn is_quote(c: u8) -> bool { } /// Offset just past the quoted string or identifier starting at `i`; a doubled quote is an escape. -// shortcut: Cypher's backslash escapes (`'it\'s'`) aren't recognized; use a doubled quote or "..." instead. +// shortcut: Cypher's backslash escapes (`'it\'s'`) aren't recognized; use a doubled quote instead. fn skip_quoted(b: &[u8], i: usize) -> Result { let quote = b[i]; let mut j = i + 1; @@ -265,6 +291,18 @@ mod tests { } } + #[test] + fn double_quotes_name_identifiers() { + let sql = "SELECT * FROM GRAPH_TABLE (MATCH (\"match\":\"Device Type\") \ + WHERE \"match\".\"asset id\" = 'a\"b' \ + COLUMNS (\"match\".\"a\"\"b`c\" AS \"select\"))"; + assert_eq!( + rewrite_graph_table(sql).unwrap(), + "SELECT * FROM graph_table('MATCH (`match`:`Device Type`) \ + WHERE `match`.`asset id` = ''a\"b'' RETURN `match`.`a\"b``c` AS `select`')" + ); + } + #[test] fn rewrites_several_clauses() { let sql = "SELECT * FROM GRAPH_TABLE(MATCH (a) COLUMNS (a.id)) x \ diff --git a/crates/lance-csr/src/parser.rs b/crates/lance-csr/src/parser.rs index 18c7a48..d55ed8c 100644 --- a/crates/lance-csr/src/parser.rs +++ b/crates/lance-csr/src/parser.rs @@ -263,10 +263,7 @@ fn shortest_path_pattern(input: &str) -> IResult<&str, PathPattern> { path_pattern, tuple((multispace0, char(')'))), )(input)?; - path.shortest = Some(ShortestPathSpec { - all, - variable: variable.map(str::to_string), - }); + path.shortest = Some(ShortestPathSpec { all, variable }); Ok((input, path)) } @@ -392,15 +389,15 @@ fn relationship_pattern(input: &str) -> IResult<&str, RelationshipPattern> { } // Type alias for complex relationship content return type -type RelationshipContentResult<'a> = ( - Option<&'a str>, - Vec<&'a str>, +type RelationshipContentResult = ( + Option, + Vec, Option>, Option, ); // Parse relationship content inside brackets -fn relationship_content(input: &str) -> IResult<&str, RelationshipContentResult<'_>> { +fn relationship_content(input: &str) -> IResult<&str, RelationshipContentResult> { let (input, _) = multispace0(input)?; let (input, variable) = opt(identifier)(input)?; // `:A`, alternation `:A|B` or `:A|:B`, and upstream's `:A:B` (also read as A or B). @@ -415,7 +412,7 @@ fn relationship_content(input: &str) -> IResult<&str, RelationshipContentResult< )), identifier, ))(input)?; - let types: Vec<&str> = first.into_iter().chain(rest).collect(); + let types: Vec = first.into_iter().chain(rest).collect(); let (input, _) = multispace0(input)?; let (input, length) = opt(length_range)(input)?; let (input, _) = multispace0(input)?; @@ -804,7 +801,7 @@ fn reduce_expression(input: &str) -> IResult<&str, ValueExpression> { let (input, mut body) = preceded(tuple((multispace0, char('|'), multispace0)), expression)(input)?; let (input, _) = tuple((multispace0, char(')')))(input)?; - mark_locals(&mut body, &[accumulator, variable]); + mark_locals(&mut body, &[&accumulator, &variable]); Ok(( input, ValueExpression::Reduce { @@ -835,7 +832,7 @@ fn list_predicate(input: &str) -> IResult<&str, ValueExpression> { expression, )(input)?; let (input, _) = tuple((multispace0, char(')')))(input)?; - mark_locals(&mut predicate, &[variable]); + mark_locals(&mut predicate, &[&variable]); Ok(( input, ValueExpression::ListPredicate { @@ -1011,7 +1008,7 @@ fn case_expression(input: &str) -> IResult<&str, ValueExpression> { fn single_value_expression(input: &str) -> IResult<&str, ValueExpression> { // Peek at first identifier to dispatch to correct parser // This eliminates failed parser attempts for every non-vector expression - if let Ok((_, first_ident)) = peek(identifier)(input) { + if let Ok((_, first_ident)) = peek(bare_identifier)(input) { let ident_lower = first_ident.to_lowercase(); match ident_lower.as_str() { @@ -1111,7 +1108,10 @@ fn parse_parameter(input: &str) -> IResult<&str, ValueExpression> { // Parse a function call: function_name(args) fn function_call(input: &str) -> IResult<&str, ValueExpression> { // Namespaced names such as `duration.inSeconds`. - let (input, name) = recognize(pair(identifier, many0(preceded(char('.'), identifier))))(input)?; + let (input, name) = recognize(pair( + bare_identifier, + many0(preceded(char('.'), bare_identifier)), + ))(input)?; let (input, _) = multispace0(input)?; let (input, _) = char('(')(input)?; let (input, _) = multispace0(input)?; @@ -1484,8 +1484,26 @@ fn pagination_clauses(input: &str) -> IResult<&str, (Option, Option IResult<&str, &str> { +// Parse an identifier: a bare name, or a backtick-quoted one in which a doubled backtick +// stands for one backtick. A quoted name may contain any character and may match a keyword. +fn identifier(input: &str) -> IResult<&str, String> { + alt(( + map( + delimited( + char('`'), + many0(alt(( + map(tag("``"), |_| '`'), + nom::character::complete::none_of("`"), + ))), + char('`'), + ), + |chars| chars.into_iter().collect(), + ), + map(bare_identifier, str::to_string), + ))(input) +} + +fn bare_identifier(input: &str) -> IResult<&str, &str> { take_while1(|c: char| c.is_alphanumeric() || c == '_')(input) } @@ -1554,7 +1572,7 @@ fn boolean_literal(input: &str) -> IResult<&str, bool> { // Parse a parameter reference fn parameter(input: &str) -> IResult<&str, String> { // Only support $param syntax - map(preceded(char('$'), identifier), |s| s.to_string())(input) + map(preceded(char('$'), bare_identifier), |s| s.to_string())(input) } // Parse comma with optional whitespace diff --git a/crates/lance-csr/tests/test_quoted_identifiers.rs b/crates/lance-csr/tests/test_quoted_identifiers.rs new file mode 100644 index 0000000..2cfc65e --- /dev/null +++ b/crates/lance-csr/tests/test_quoted_identifiers.rs @@ -0,0 +1,112 @@ +use std::collections::HashMap; +use std::sync::Arc; + +use arrow_array::{cast::AsArray, Int64Array, RecordBatch, StringArray}; +use arrow_schema::{DataType, Field, Schema}; +use lance_csr::{CypherEngine, EngineSource, GraphConfig, SqlEngine}; + +fn edges(edges: &[(i64, i64)]) -> RecordBatch { + RecordBatch::try_new( + Arc::new(Schema::new(vec![ + Field::new("src_id", DataType::Int64, false), + Field::new("dst_id", DataType::Int64, false), + ])), + vec![ + Arc::new(Int64Array::from_iter_values(edges.iter().map(|e| e.0))), + Arc::new(Int64Array::from_iter_values(edges.iter().map(|e| e.1))), + ], + ) + .unwrap() +} + +#[tokio::test] +async fn backtick_quoted_identifiers() { + let config = GraphConfig::builder() + .node_table("Device Type", "id") + .rel_table("PART OF", "Device Type", "Device Type", "src_id", "dst_id") + .build() + .unwrap(); + let devices = RecordBatch::try_new( + Arc::new(Schema::new(vec![ + Field::new("id", DataType::Int64, false), + Field::new("asset id", DataType::Utf8, false), + Field::new("name", DataType::Utf8, false), + ])), + vec![ + Arc::new(Int64Array::from(vec![1, 2, 3])), + Arc::new(StringArray::from(vec!["a-1", "a-2", "a-3"])), + Arc::new(StringArray::from(vec!["pump", "valve", "tank"])), + ], + ) + .unwrap(); + for use_csr in [false, true] { + let sources = HashMap::from([ + ( + "Device Type".to_string(), + EngineSource::Arrow(devices.clone()), + ), + ( + "PART OF".to_string(), + EngineSource::Arrow(edges(&[(1, 3), (2, 3)])), + ), + ]); + let engine = CypherEngine::new(config.clone(), sources, use_csr) + .await + .unwrap(); + let strings = |batch: &RecordBatch, column: &str| -> Vec { + let column = batch.column_by_name(column).unwrap().as_string::(); + column.iter().map(|v| v.unwrap().to_string()).collect() + }; + + let keys = HashMap::from([("keys".to_string(), serde_json::json!(["a-1", "a-3"]))]); + let batch = engine + .execute( + "MATCH (x:`Device Type`) WHERE x.`asset id` IN $keys \ + RETURN x.`asset id` AS `asset id`, x.name AS `a``b` ORDER BY `asset id`", + keys, + ) + .await + .unwrap(); + assert_eq!(strings(&batch, "asset id"), ["a-1", "a-3"]); + assert_eq!(strings(&batch, "a`b"), ["pump", "tank"]); + + let batch = engine + .execute( + "MATCH (`match`:`Device Type`)-[:`PART OF`]->(`return`) \ + RETURN `match`.name AS `where` ORDER BY `where`", + HashMap::new(), + ) + .await + .unwrap(); + assert_eq!( + strings(&batch, "where"), + ["pump", "valve"], + "use_csr={use_csr}" + ); + + let batch = engine + .execute( + "MATCH (x:`Device Type` {`asset id`: 'a-2'}) RETURN x.name AS name", + HashMap::new(), + ) + .await + .unwrap(); + assert_eq!(strings(&batch, "name"), ["valve"]); + + // SQL and GRAPH_TABLE quote with double quotes; tables are registered lowercase. + let sql = SqlEngine::from_engine(Arc::new(engine)); + let batch = sql + .execute( + "SELECT g.\"asset id\", d.name AS \"select\" FROM GRAPH_TABLE ( + MATCH (\"match\":\"Device Type\")-[:`PART OF`]->(t) WHERE t.name = 'tank' + COLUMNS (\"match\".\"asset id\" AS \"asset id\", t.id AS tid) + ) g JOIN \"device type\" d ON d.id = g.tid + WHERE d.\"asset id\" = 'a-3' ORDER BY g.\"asset id\"", + HashMap::new(), + ) + .await + .unwrap(); + assert_eq!(strings(&batch, "asset id"), ["a-1", "a-2"]); + assert_eq!(strings(&batch, "select"), ["tank", "tank"]); + } +} diff --git a/docs/cypher.md b/docs/cypher.md index 4ce29eb..d09e6a4 100644 --- a/docs/cypher.md +++ b/docs/cypher.md @@ -566,6 +566,19 @@ 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`. +## Quoted names + +Backticks quote a label, relationship type, variable, property name or alias that contains spaces or +other characters, or that matches a keyword. Inside a quoted name, two backticks stand for one. Quoting +doesn't change the case rules above. + +```python +engine.execute( + "MATCH (x:`Device Type`) WHERE x.`asset id` IN $keys RETURN x.`asset id` AS `asset id`", + {"keys": ["a-1"]}, +).to_pylist() # [{'asset id': 'a-1'}] +``` + ## Errors | Exception | Raised when | diff --git a/docs/sql.md b/docs/sql.md index 8d91e77..cc0ee37 100644 --- a/docs/sql.md +++ b/docs/sql.md @@ -62,6 +62,7 @@ print(friends.to_pylist()) !!! note "Table names are lower-cased" `SqlEngine` lower-cases dataset names when it registers them, so a `"Person"` key is queried as `person`. + A quoted table name must be lower-case too: a `"Device Type"` key is queried as `"device type"`. Constructing an engine with an empty dict raises `ValueError`. The engine keeps up to 128 plans per snapshot, keyed by the query text and the names and types of its scalar @@ -105,13 +106,28 @@ print(engine.execute(""" # [{'person': 'Alice', 'friend_age': 29}, {'person': 'Alice', 'friend_age': 34}] ``` -Inside `GRAPH_TABLE`, the `MATCH` pattern, the `WHERE` condition and the `COLUMNS` list are Cypher: the clause runs -as `MATCH ... WHERE ... RETURN ` would, so any pattern `CypherEngine.execute` supports works there, and +Inside `GRAPH_TABLE`, the `MATCH` pattern, the `WHERE` condition and the `COLUMNS` list are Cypher with SQL's +quoting: the clause runs as `MATCH ... WHERE ... RETURN ` would, so any pattern `CypherEngine.execute` supports works there, and the columns take the names their aliases give them. One `params` dict serves the SQL and every pattern. Only the clause's shape comes from SQL/PGQ. There is no `CREATE PROPERTY GRAPH`: the graph is the engine's `GraphConfig`, and a graph name before `MATCH` is accepted and ignored. `graph_table('')` is the same table written as a function call. Without a `config`, `GRAPH_TABLE` raises `ValueError`. +As in the rest of the SQL query, strings inside `GRAPH_TABLE` use single quotes, and double quotes name a label, +relationship type, variable, property or column that contains spaces or other characters, or that matches a +keyword, such as `x."asset id"` or `(x:"Device Type")`. Inside a double-quoted name, `""` stands for one `"`. +Cypher's backticks work too. The direct `graph_table('...')` call takes Cypher text as written, so there `"..."` +is a string. + +```python +engine.execute(""" + SELECT g."asset id" FROM GRAPH_TABLE ( + MATCH (x:"Device Type") WHERE x.status = 'active' + COLUMNS (x."asset id" AS "asset id") + ) g +""") +``` + To run Cypher and SQL over the same data, build the `CypherEngine` and share its snapshot. Nothing is rebuilt, and because the two engines share one snapshot, `checkout_latest()` on either moves both: diff --git a/python/python/tests/test_quoted_identifiers.py b/python/python/tests/test_quoted_identifiers.py new file mode 100644 index 0000000..88c89b5 --- /dev/null +++ b/python/python/tests/test_quoted_identifiers.py @@ -0,0 +1,67 @@ +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright The lance-csr Authors + +"""Quoted identifiers: backticks in Cypher, double quotes in SQL and GRAPH_TABLE.""" + +import pyarrow as pa +import pytest +from lance_csr import CypherEngine, GraphConfig, SqlEngine + + +@pytest.mark.parametrize("csr", [False, True], ids=["join", "csr"]) +def test_backtick_quoted_identifiers(csr: bool) -> None: + config = ( + GraphConfig.builder() + .node_table("Device Type", "id") + .rel_table("PART OF", "Device Type", "Device Type", "src_id", "dst_id") + .build() + ) + engine = CypherEngine( + config, + { + "Device Type": pa.table( + { + "id": [1, 2, 3], + "asset id": ["a-1", "a-2", "a-3"], + "name": ["pump", "valve", "tank"], + } + ), + "PART OF": pa.table({"src_id": [1, 2], "dst_id": [3, 3]}), + }, + use_csr=csr, + ) + + rows = engine.execute( + "MATCH (x:`Device Type`) WHERE x.`asset id` IN $keys " + "RETURN x.`asset id` AS `asset id`, x.name AS `a``b` ORDER BY `asset id`", + {"keys": ["a-1", "a-3"]}, + ).to_pylist() + assert rows == [ + {"asset id": "a-1", "a`b": "pump"}, + {"asset id": "a-3", "a`b": "tank"}, + ] + + rows = engine.execute( + "MATCH (`match`:`Device Type`)-[:`PART OF`]->(`return`) " + "RETURN `match`.name AS `where` ORDER BY `where`" + ).to_pylist() + assert rows == [{"where": "pump"}, {"where": "valve"}] + + # SQL quotes with double quotes; tables are registered under lowercase names. + rows = ( + SqlEngine.from_engine(engine) + .execute( + """ + SELECT g."asset id", d.name AS "select" FROM GRAPH_TABLE ( + MATCH ("match":"Device Type")-[:"PART OF"]->(t) WHERE t.name = 'tank' + COLUMNS ("match"."asset id" AS "asset id", t.id AS tid) + ) g JOIN "device type" d ON d.id = g.tid + ORDER BY g."asset id" + """ + ) + .to_pylist() + ) + assert rows == [ + {"asset id": "a-1", "select": "tank"}, + {"asset id": "a-2", "select": "tank"}, + ]