From e064743c736cb35a6b6819345963f29b446b0445 Mon Sep 17 00:00:00 2001
From: walking devel <104449470+walkingdevel@users.noreply.github.com>
Date: Sat, 28 Jan 2023 08:07:02 +0000
Subject: [PATCH] parser: recursively search undefined variables in the `where`
 parts of SQL statements (#17136)

---
 vlib/v/parser/sql.v                           | 79 +++++++++++++++----
 ...l_undefined_variables_in_complex_exprs.out |  7 ++
 ...ql_undefined_variables_in_complex_exprs.vv | 15 ++++
 3 files changed, 87 insertions(+), 14 deletions(-)
 create mode 100644 vlib/v/parser/tests/sql_undefined_variables_in_complex_exprs.out
 create mode 100644 vlib/v/parser/tests/sql_undefined_variables_in_complex_exprs.vv

diff --git a/vlib/v/parser/sql.v b/vlib/v/parser/sql.v
index 8f08ed5d19..177a72f4f0 100644
--- a/vlib/v/parser/sql.v
+++ b/vlib/v/parser/sql.v
@@ -31,20 +31,15 @@ fn (mut p Parser) sql_expr() ast.Expr {
 	if has_where {
 		p.next()
 		where_expr = p.expr(0)
-		// `id == x` means that a single object is returned
-		if !is_count && mut where_expr is ast.InfixExpr {
-			if where_expr.op == .eq && mut where_expr.left is ast.Ident {
-				if where_expr.left.name == 'id' {
-					query_one = true
-				}
-			}
-			if mut where_expr.right is ast.Ident {
-				if !p.scope.known_var(where_expr.right.name) {
-					p.check_undefined_variables([where_expr.left.str()], where_expr.right) or {
-						return p.error_with_pos(err.msg(), where_expr.right.pos)
-					}
-				}
-			}
+
+		where_check_result := p.check_sql_where_expr_has_no_undefined_variables(&where_expr,
+			[])
+		if where_check_result is ast.NodeError {
+			return where_check_result
+		}
+
+		if !is_count {
+			query_one = p.has_sql_where_expr_with_comparison_with_id(&where_expr)
 		}
 	}
 	mut has_limit := false
@@ -304,3 +299,59 @@ fn (mut p Parser) check_sql_keyword(name string) ?bool {
 	}
 	return true
 }
+
+// has_sql_where_expr_with_comparison_with_id tries to search comparison with the `id` field.
+// If `id` is found it means that a database "should" return one row,
+// but it is not true and depends on a database scheme.
+// For example, it will be hard to use V ORM in an existing database scheme which wrote before using V.
+fn (p &Parser) has_sql_where_expr_with_comparison_with_id(expr &ast.Expr) bool {
+	// This method will be removed in the future when we refuse it.
+	// A more difficult expression with `id` means a user tries to find more than one row or he is wrong.
+	// And there is no point to get one structure instead of an array.
+	// `id == x` means that a single object is returned.
+	if expr is ast.InfixExpr {
+		if expr.left is ast.Ident && expr.op == .eq {
+			return expr.left.name == 'id'
+		}
+	} else if expr is ast.ParExpr {
+		return p.has_sql_where_expr_with_comparison_with_id(&expr.expr)
+	}
+
+	return false
+}
+
+// check_sql_where_expr_has_no_undefined_variables recursively tries to find undefined variables in the right part of infix expressions.
+fn (mut p Parser) check_sql_where_expr_has_no_undefined_variables(expr &ast.Expr, unacceptable_variable_names []string) ast.Expr {
+	if expr is ast.Ident {
+		if !p.scope.known_var(expr.name) {
+			p.check_undefined_variables(unacceptable_variable_names, expr) or {
+				return p.error_with_pos(err.msg(), expr.pos)
+			}
+		}
+	} else if expr is ast.InfixExpr {
+		if expr.left is ast.Ident && expr.right is ast.Ident {
+			return p.check_sql_where_expr_has_no_undefined_variables(&expr.right, [
+				expr.left.str(),
+			])
+		}
+
+		left_check_result := p.check_sql_where_expr_has_no_undefined_variables(&expr.left,
+			[])
+
+		if left_check_result is ast.NodeError {
+			return left_check_result
+		}
+
+		variable_names := if expr.left is ast.Ident { [expr.left.str()] } else { []string{} }
+		right_check_result := p.check_sql_where_expr_has_no_undefined_variables(&expr.right,
+			variable_names)
+
+		if right_check_result is ast.NodeError {
+			return right_check_result
+		}
+	} else if expr is ast.ParExpr {
+		return p.check_sql_where_expr_has_no_undefined_variables(&expr.expr, [])
+	}
+
+	return ast.empty_expr
+}
diff --git a/vlib/v/parser/tests/sql_undefined_variables_in_complex_exprs.out b/vlib/v/parser/tests/sql_undefined_variables_in_complex_exprs.out
new file mode 100644
index 0000000000..2a55dfb327
--- /dev/null
+++ b/vlib/v/parser/tests/sql_undefined_variables_in_complex_exprs.out
@@ -0,0 +1,7 @@
+vlib/v/parser/tests/sql_undefined_variables_in_complex_exprs.vv:13:73: error: undefined variable: `username`
+   11 | fn main() {
+   12 |     _ := sql _ {
+   13 |         select from User where (age > 18) && (city == 'London' || username == username)
+      |                                                                               ~~~~~~~~
+   14 |     }
+   15 | }
diff --git a/vlib/v/parser/tests/sql_undefined_variables_in_complex_exprs.vv b/vlib/v/parser/tests/sql_undefined_variables_in_complex_exprs.vv
new file mode 100644
index 0000000000..574fca226a
--- /dev/null
+++ b/vlib/v/parser/tests/sql_undefined_variables_in_complex_exprs.vv
@@ -0,0 +1,15 @@
+module main
+
+struct User {
+mut:
+	id       int    [primary]
+	username string
+	age      string
+	city     string
+}
+
+fn main() {
+	_ := sql _ {
+		select from User where (age > 18) && (city == 'London' || username == username)
+	}
+}