fix(tck): address review comments on array subscript validation

Four improvements following code review:

1. Remove Literal::String from infer_non_integer_kind — string-keyed
   bracket subscripts are valid Cypher map/property access (e.g.
   map['key']).  String-on-array errors are caught at SQL execution
   time by ClickHouse.

2. Expand find_invalid_array_subscript to cover all expression variants
   that walk_expression handles: AggregateFnCall, Operator (legacy),
   Case, ReduceExpr, MapLiteral, ArraySlicing.  Previously nested
   subscripts inside CASE branches, reduce expressions, etc. were
   silently skipped.

3. Extend plan coverage beyond Projection: also validate subscript
   indices in Filter predicates and WithClause items / where_clause /
   order_by expressions.  Add check_items() helper.

4. Drop toInt64() cast for non-literal array indices — emit (expr)+1
   instead, letting ClickHouse's own type checker catch bad types
   (floats, strings) rather than silently coercing them.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: genezhang

src/clickhouse_query_generator/to_sql_query.rs

@@ -5133,14 +5133,16 @@ impl RenderExpr {
                 // Array subscript in ClickHouse: array[index]
                 // Cypher uses 0-based indexing; ClickHouse uses 1-based.
                 // For integer literals we add +1 at compile time.
-                // For variable / expression indices we wrap with toInt64(...)+1.
-                // Exception: string-literal indices are ClickHouse map-key accesses
+                // For expression indices we emit (expr)+1 without an explicit cast
+                // so ClickHouse's own type checker catches bad types (e.g. floats,
+                // strings) rather than silently coercing them.
+                // Exception: string-literal indices are map-key accesses
                 //   (e.g. top['score']) and must NOT be offset.
                 let array_sql = array.to_sql();
                 let index_sql = match index.as_ref() {
                     RenderExpr::Literal(Literal::Integer(n)) => format!("{}", n + 1),
                     RenderExpr::Literal(Literal::String(_)) => index.to_sql(),
-                    _ => format!("toInt64({})+1", index.to_sql()),
+                    _ => format!("({})+1", index.to_sql()),
                 };
                 format!("{}[{}]", array_sql, index_sql)
             }

src/query_planner/analyzer/query_validation.rs

@@ -43,13 +43,16 @@ impl fmt::Display for NonIntegerKind {
 }
 
 /// Returns `Some(kind)` when `expr` is a clearly non-integer literal; `None` otherwise.
+///
+/// Note: `Literal::String` is intentionally excluded — bracket subscripts with string keys
+/// (`map['key']`, `node['prop']`) are valid Cypher map/property access and must not be
+/// rejected. String-keyed array subscripts (invalid) are caught at SQL execution time.
 fn infer_non_integer_kind(expr: &LogicalExpr) -> Option<NonIntegerKind> {
     match expr {
         LogicalExpr::Literal(lit) => match lit {
             Literal::Boolean(_) => Some(NonIntegerKind::Boolean),
             Literal::Float(_) => Some(NonIntegerKind::Float),
-            Literal::String(_) => Some(NonIntegerKind::Str),
-            Literal::Integer(_) | Literal::Null => None,
+            Literal::Integer(_) | Literal::String(_) | Literal::Null => None,
         },
         LogicalExpr::List(_) => Some(NonIntegerKind::List),
         LogicalExpr::MapLiteral(_) => Some(NonIntegerKind::Map),
@@ -74,10 +77,19 @@ fn build_non_integer_alias_map(items: &[ProjectionItem]) -> HashMap<String, NonI
 /// Walk `expr` recursively looking for an `ArraySubscript` whose index is a
 /// known non-integer type (either a direct literal or an alias bound to one).
 /// Returns `Some(error_message)` on the first violation found.
+///
+/// Covers all expression variants that `walk_expression` traverses so that
+/// subscripts nested inside CASE branches, aggregate calls, reduce expressions,
+/// map literals, array slicings, etc. are also checked.
 fn find_invalid_array_subscript(
     expr: &LogicalExpr,
     alias_types: &HashMap<String, NonIntegerKind>,
 ) -> Option<String> {
+    macro_rules! recurse {
+        ($e:expr) => {
+            find_invalid_array_subscript($e, alias_types)
+        };
+    }
     match expr {
         LogicalExpr::ArraySubscript { array, index } => {
             // Direct non-integer literal index.
@@ -101,25 +113,45 @@ fn find_invalid_array_subscript(
                     ));
                 }
             }
-            // Recurse into sub-expressions.
-            find_invalid_array_subscript(array, alias_types)
-                .or_else(|| find_invalid_array_subscript(index, alias_types))
+            recurse!(array).or_else(|| recurse!(index))
         }
-        LogicalExpr::OperatorApplicationExp(op) => op
-            .operands
-            .iter()
-            .find_map(|o| find_invalid_array_subscript(o, alias_types)),
-        LogicalExpr::ScalarFnCall(f) => f
-            .args
-            .iter()
-            .find_map(|a| find_invalid_array_subscript(a, alias_types)),
-        LogicalExpr::List(items) => items
-            .iter()
-            .find_map(|i| find_invalid_array_subscript(i, alias_types)),
+        LogicalExpr::OperatorApplicationExp(op) | LogicalExpr::Operator(op) => {
+            op.operands.iter().find_map(|o| recurse!(o))
+        }
+        LogicalExpr::ScalarFnCall(f) => f.args.iter().find_map(|a| recurse!(a)),
+        LogicalExpr::AggregateFnCall(f) => f.args.iter().find_map(|a| recurse!(a)),
+        LogicalExpr::List(items) => items.iter().find_map(|i| recurse!(i)),
+        LogicalExpr::MapLiteral(entries) => entries.iter().find_map(|(_, v)| recurse!(v)),
+        LogicalExpr::Case(c) => c
+            .expr
+            .as_ref()
+            .and_then(|e| recurse!(e))
+            .or_else(|| {
+                c.when_then
+                    .iter()
+                    .find_map(|(w, t)| recurse!(w).or_else(|| recurse!(t)))
+            })
+            .or_else(|| c.else_expr.as_ref().and_then(|e| recurse!(e))),
+        LogicalExpr::ReduceExpr(r) => recurse!(&r.initial_value)
+            .or_else(|| recurse!(&r.list))
+            .or_else(|| recurse!(&r.expression)),
+        LogicalExpr::ArraySlicing { array, from, to } => recurse!(array)
+            .or_else(|| from.as_ref().and_then(|f| recurse!(f)))
+            .or_else(|| to.as_ref().and_then(|t| recurse!(t))),
         _ => None,
     }
 }
 
+/// Check all expressions in a slice of projection items.
+fn check_items(
+    items: &[ProjectionItem],
+    alias_types: &HashMap<String, NonIntegerKind>,
+) -> Option<String> {
+    items
+        .iter()
+        .find_map(|item| find_invalid_array_subscript(&item.expression, alias_types))
+}
+
 pub struct QueryValidation;
 
 impl AnalyzerPass for QueryValidation {
@@ -355,6 +387,11 @@ impl AnalyzerPass for QueryValidation {
                 graph_joins.rebuild_or_clone(child_tf, logical_plan.clone())
             }
             LogicalPlan::Filter(filter) => {
+                // Validate subscript indices in WHERE predicates.
+                let empty_map = HashMap::new();
+                if let Some(err) = find_invalid_array_subscript(&filter.predicate, &empty_map) {
+                    return Err(AnalyzerError::InvalidPlan(err));
+                }
                 let child_tf =
                     self.analyze_with_graph_schema(filter.input.clone(), plan_ctx, graph_schema)?;
                 filter.rebuild_or_clone(child_tf, logical_plan.clone())
@@ -434,6 +471,27 @@ impl AnalyzerPass for QueryValidation {
                 }
             }
             LogicalPlan::WithClause(with_clause) => {
+                // Validate subscript indices inside WITH projection expressions.
+                let empty_map = HashMap::new();
+                if let Some(err) = check_items(&with_clause.items, &empty_map) {
+                    return Err(AnalyzerError::InvalidPlan(err));
+                }
+                // Build alias→type map so WHERE/ORDER BY can resolve alias references.
+                let alias_types = build_non_integer_alias_map(&with_clause.items);
+                if let Some(ref wc) = with_clause.where_clause {
+                    if let Some(err) = find_invalid_array_subscript(wc, &alias_types) {
+                        return Err(AnalyzerError::InvalidPlan(err));
+                    }
+                }
+                if let Some(ref order_by) = with_clause.order_by {
+                    for item in order_by {
+                        if let Some(err) =
+                            find_invalid_array_subscript(&item.expression, &alias_types)
+                        {
+                            return Err(AnalyzerError::InvalidPlan(err));
+                        }
+                    }
+                }
                 let child_tf = self.analyze_with_graph_schema(
                     with_clause.input.clone(),
                     plan_ctx,