From a9a04bba55742f333db18ce39e00dda4c6e4c55f Mon Sep 17 00:00:00 2001 From: walking devel <104449470+walkingdevel@users.noreply.github.com> Date: Thu, 26 Jan 2023 20:36:30 +0000 Subject: [PATCH] orm: support fn calls in `where` (#17127) --- cmd/tools/vtest-self.v | 2 + vlib/orm/orm_fn_calls_test.v | 48 ++++++++++++++++ vlib/v/checker/orm.v | 43 ++++++++++++++ .../orm_fn_call_with_wrong_return_type.out | 7 +++ .../orm_fn_call_with_wrong_return_type.vv | 56 +++++++++++++++++++ vlib/v/gen/c/sql.v | 9 +++ 6 files changed, 165 insertions(+) create mode 100644 vlib/orm/orm_fn_calls_test.v create mode 100644 vlib/v/checker/tests/orm_fn_call_with_wrong_return_type.out create mode 100644 vlib/v/checker/tests/orm_fn_call_with_wrong_return_type.vv diff --git a/cmd/tools/vtest-self.v b/cmd/tools/vtest-self.v index f64f1d77e5..87b6dab890 100644 --- a/cmd/tools/vtest-self.v +++ b/cmd/tools/vtest-self.v @@ -125,6 +125,7 @@ const ( 'vlib/orm/orm_sql_or_blocks_test.v', 'vlib/orm/orm_create_and_drop_test.v', 'vlib/orm/orm_insert_test.v', + 'vlib/orm/orm_fn_calls_test.v', 'vlib/db/sqlite/sqlite_test.v', 'vlib/db/sqlite/sqlite_orm_test.v', 'vlib/db/sqlite/sqlite_vfs_lowlevel_test.v', @@ -192,6 +193,7 @@ const ( 'vlib/orm/orm_sql_or_blocks_test.v', 'vlib/orm/orm_create_and_drop_test.v', 'vlib/orm/orm_insert_test.v', + 'vlib/orm/orm_fn_calls_test.v', 'vlib/v/tests/orm_sub_struct_test.v', 'vlib/v/tests/orm_sub_array_struct_test.v', 'vlib/v/tests/orm_joined_tables_select_test.v', diff --git a/vlib/orm/orm_fn_calls_test.v b/vlib/orm/orm_fn_calls_test.v new file mode 100644 index 0000000000..3994bd4733 --- /dev/null +++ b/vlib/orm/orm_fn_calls_test.v @@ -0,0 +1,48 @@ +import db.sqlite + +struct User { + id int [primary; sql: serial] + name string + age int +} + +fn get_acceptable_age() int { + return 21 +} + +fn test_fn_calls() { + mut db := sqlite.connect(':memory:') or { panic(err) } + + sql db { + create table User + } + + first_user := User{ + name: 'first' + age: 25 + } + + second_user := User{ + name: 'second' + age: 14 + } + + sql db { + insert first_user into User + insert second_user into User + } + + users_with_acceptable_age := sql db { + select from User where age >= get_acceptable_age() + } + + assert users_with_acceptable_age.len == 1 + assert users_with_acceptable_age.first().name == 'first' + + users_with_non_acceptable_age := sql db { + select from User where age < get_acceptable_age() + } + + assert users_with_non_acceptable_age.len == 1 + assert users_with_non_acceptable_age.first().name == 'second' +} diff --git a/vlib/v/checker/orm.v b/vlib/v/checker/orm.v index 79cd57bc1b..7d670e1ffe 100644 --- a/vlib/v/checker/orm.v +++ b/vlib/v/checker/orm.v @@ -96,6 +96,7 @@ fn (mut c Checker) sql_expr(mut node ast.SqlExpr) ast.Type { node.sub_structs = sub_structs.move() if node.has_where { c.expr(node.where_expr) + c.check_expr_has_no_fn_calls_with_non_orm_return_type(&node.where_expr) } if node.has_offset { c.expr(node.offset_expr) @@ -273,3 +274,45 @@ fn (mut c Checker) fetch_and_verify_orm_fields(info ast.Struct, pos token.Pos, t } return fields } + +// check_expr_has_no_fn_calls_with_non_orm_return_type checks that an expression has no function calls +// that return complex types which can't be transformed into SQL. +fn (mut c Checker) check_expr_has_no_fn_calls_with_non_orm_return_type(expr &ast.Expr) { + if expr is ast.CallExpr { + // `expr.return_type` may be empty. For example, a user call function incorrectly without passing all required arguments. + // This error will be handled in another place. Otherwise, `c.table.sym` below does panic. + // + // fn test(flag bool) {} + // test() + // ~~~~~~ expected 1 arguments, but got 0 + if expr.return_type == 0 { + return + } + + type_symbol := c.table.sym(expr.return_type) + is_result_type := expr.return_type.has_flag(.result) + is_option_type := expr.return_type.has_flag(.option) + is_time := type_symbol.cname == 'time__Time' + is_not_pointer := !type_symbol.is_pointer() + is_error_type := is_result_type || is_option_type + is_acceptable_type := (type_symbol.is_primitive() || is_time) && is_not_pointer + && !is_error_type + + if !is_acceptable_type { + error_type_symbol := if is_result_type { + '!' + } else if is_option_type { + '?' + } else { + '' + } + c.error('V ORM: function calls must return only primitive types and time.Time, but `${expr.name}` returns `${error_type_symbol}${type_symbol.name}`', + expr.pos) + } + } else if expr is ast.ParExpr { + c.check_expr_has_no_fn_calls_with_non_orm_return_type(&expr.expr) + } else if expr is ast.InfixExpr { + c.check_expr_has_no_fn_calls_with_non_orm_return_type(&expr.left) + c.check_expr_has_no_fn_calls_with_non_orm_return_type(&expr.right) + } +} diff --git a/vlib/v/checker/tests/orm_fn_call_with_wrong_return_type.out b/vlib/v/checker/tests/orm_fn_call_with_wrong_return_type.out new file mode 100644 index 0000000000..2bdefe2bee --- /dev/null +++ b/vlib/v/checker/tests/orm_fn_call_with_wrong_return_type.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/orm_fn_call_with_wrong_return_type.vv:52:37: error: V ORM: function calls must return only primitive types and time.Time, but `get_second_child` returns `Child` + 50 | + 51 | steve := sql db { + 52 | select from Parent where child == get_second_child() + | ~~~~~~~~~~~~~~~~~~ + 53 | } + 54 | diff --git a/vlib/v/checker/tests/orm_fn_call_with_wrong_return_type.vv b/vlib/v/checker/tests/orm_fn_call_with_wrong_return_type.vv new file mode 100644 index 0000000000..2adec6c34d --- /dev/null +++ b/vlib/v/checker/tests/orm_fn_call_with_wrong_return_type.vv @@ -0,0 +1,56 @@ +import db.sqlite + +struct Parent { + id int [primary; sql: serial] + name string + child Child [fkey: 'parent_id'] +} + +struct Child { +mut: + id int [primary; sql: serial] + parent_id int + name string +} + +fn get_first_child() Child { + return Child{ + name: 'Lisa' + } +} + +fn get_second_child() Child { + return Child{ + name: 'Steve' + } +} + +fn main() { + mut db := sqlite.connect(':memory:') or { panic(err) } + + sql db { + create table Parent + create table Child + } + + new_parent := Parent{ + name: 'test' + child: get_second_child() + } + + sql db { + insert new_parent into Parent + } + + first_child := get_first_child() + + sql db { + insert first_child into Child + } + + steve := sql db { + select from Parent where child == get_second_child() + } + + dump(steve) +} diff --git a/vlib/v/gen/c/sql.v b/vlib/v/gen/c/sql.v index 647ece359d..624143f368 100644 --- a/vlib/v/gen/c/sql.v +++ b/vlib/v/gen/c/sql.v @@ -299,6 +299,9 @@ fn (mut g Gen) sql_expr_to_orm_primitive(expr ast.Expr) { ast.SelectorExpr { g.sql_write_orm_primitive(expr.typ, expr) } + ast.CallExpr { + g.sql_write_orm_primitive(expr.return_type, expr) + } else { eprintln(expr) verror('Unknown expr') @@ -320,6 +323,7 @@ fn (mut g Gen) sql_write_orm_primitive(t ast.Type, expr ast.Expr) { if typ == 'orm__InfixType' { typ = 'infix' } + g.write('orm__${typ}_to_primitive(') if expr is ast.InfixExpr { g.write('(orm__InfixType){') @@ -345,6 +349,8 @@ fn (mut g Gen) sql_write_orm_primitive(t ast.Type, expr ast.Expr) { g.write('.right = ') g.sql_expr_to_orm_primitive(expr.right) g.write('}') + } else if expr is ast.CallExpr { + g.call_expr(expr) } else { g.expr(expr) } @@ -422,6 +428,9 @@ fn (mut g Gen) sql_where_data(expr ast.Expr, mut fields []string, mut parenthese ast.BoolLiteral { data << expr } + ast.CallExpr { + data << expr + } else {} } }