Skip to content

Commit e064743

Browse files
author
walking devel
authored
parser: recursively search undefined variables in the where parts of SQL statements (#17136)
1 parent 1d4fd53 commit e064743

File tree

3 files changed

+87
-14
lines changed

3 files changed

+87
-14
lines changed

vlib/v/parser/sql.v

Lines changed: 65 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,15 @@ fn (mut p Parser) sql_expr() ast.Expr {
3131
if has_where {
3232
p.next()
3333
where_expr = p.expr(0)
34-
// `id == x` means that a single object is returned
35-
if !is_count && mut where_expr is ast.InfixExpr {
36-
if where_expr.op == .eq && mut where_expr.left is ast.Ident {
37-
if where_expr.left.name == 'id' {
38-
query_one = true
39-
}
40-
}
41-
if mut where_expr.right is ast.Ident {
42-
if !p.scope.known_var(where_expr.right.name) {
43-
p.check_undefined_variables([where_expr.left.str()], where_expr.right) or {
44-
return p.error_with_pos(err.msg(), where_expr.right.pos)
45-
}
46-
}
47-
}
34+
35+
where_check_result := p.check_sql_where_expr_has_no_undefined_variables(&where_expr,
36+
[])
37+
if where_check_result is ast.NodeError {
38+
return where_check_result
39+
}
40+
41+
if !is_count {
42+
query_one = p.has_sql_where_expr_with_comparison_with_id(&where_expr)
4843
}
4944
}
5045
mut has_limit := false
@@ -304,3 +299,59 @@ fn (mut p Parser) check_sql_keyword(name string) ?bool {
304299
}
305300
return true
306301
}
302+
303+
// has_sql_where_expr_with_comparison_with_id tries to search comparison with the `id` field.
304+
// If `id` is found it means that a database "should" return one row,
305+
// but it is not true and depends on a database scheme.
306+
// For example, it will be hard to use V ORM in an existing database scheme which wrote before using V.
307+
fn (p &Parser) has_sql_where_expr_with_comparison_with_id(expr &ast.Expr) bool {
308+
// This method will be removed in the future when we refuse it.
309+
// A more difficult expression with `id` means a user tries to find more than one row or he is wrong.
310+
// And there is no point to get one structure instead of an array.
311+
// `id == x` means that a single object is returned.
312+
if expr is ast.InfixExpr {
313+
if expr.left is ast.Ident && expr.op == .eq {
314+
return expr.left.name == 'id'
315+
}
316+
} else if expr is ast.ParExpr {
317+
return p.has_sql_where_expr_with_comparison_with_id(&expr.expr)
318+
}
319+
320+
return false
321+
}
322+
323+
// check_sql_where_expr_has_no_undefined_variables recursively tries to find undefined variables in the right part of infix expressions.
324+
fn (mut p Parser) check_sql_where_expr_has_no_undefined_variables(expr &ast.Expr, unacceptable_variable_names []string) ast.Expr {
325+
if expr is ast.Ident {
326+
if !p.scope.known_var(expr.name) {
327+
p.check_undefined_variables(unacceptable_variable_names, expr) or {
328+
return p.error_with_pos(err.msg(), expr.pos)
329+
}
330+
}
331+
} else if expr is ast.InfixExpr {
332+
if expr.left is ast.Ident && expr.right is ast.Ident {
333+
return p.check_sql_where_expr_has_no_undefined_variables(&expr.right, [
334+
expr.left.str(),
335+
])
336+
}
337+
338+
left_check_result := p.check_sql_where_expr_has_no_undefined_variables(&expr.left,
339+
[])
340+
341+
if left_check_result is ast.NodeError {
342+
return left_check_result
343+
}
344+
345+
variable_names := if expr.left is ast.Ident { [expr.left.str()] } else { []string{} }
346+
right_check_result := p.check_sql_where_expr_has_no_undefined_variables(&expr.right,
347+
variable_names)
348+
349+
if right_check_result is ast.NodeError {
350+
return right_check_result
351+
}
352+
} else if expr is ast.ParExpr {
353+
return p.check_sql_where_expr_has_no_undefined_variables(&expr.expr, [])
354+
}
355+
356+
return ast.empty_expr
357+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
vlib/v/parser/tests/sql_undefined_variables_in_complex_exprs.vv:13:73: error: undefined variable: `username`
2+
11 | fn main() {
3+
12 | _ := sql _ {
4+
13 | select from User where (age > 18) && (city == 'London' || username == username)
5+
| ~~~~~~~~
6+
14 | }
7+
15 | }
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
module main
2+
3+
struct User {
4+
mut:
5+
id int [primary]
6+
username string
7+
age string
8+
city string
9+
}
10+
11+
fn main() {
12+
_ := sql _ {
13+
select from User where (age > 18) && (city == 'London' || username == username)
14+
}
15+
}

0 commit comments

Comments
 (0)