From e06e8b10cd5cf23ba37909bcec5a976e4c313ca2 Mon Sep 17 00:00:00 2001 From: Louis Schmieder Date: Fri, 22 Jan 2021 23:28:26 +0100 Subject: [PATCH] orm: fix empty struct and simplify (#8246) --- vlib/v/ast/ast.v | 6 +-- vlib/v/checker/checker.v | 15 +++---- vlib/v/checker/tests/orm_empty_struct.out | 7 ++++ vlib/v/checker/tests/orm_empty_struct.vv | 11 +++++ vlib/v/fmt/fmt.v | 13 +++--- vlib/v/gen/sql.v | 12 +++--- vlib/v/parser/sql.v | 49 ++++++++++------------- 7 files changed, 64 insertions(+), 49 deletions(-) create mode 100644 vlib/v/checker/tests/orm_empty_struct.out create mode 100644 vlib/v/checker/tests/orm_empty_struct.vv diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index 8fe1f1859a..413d16b1e2 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -1156,13 +1156,12 @@ pub: kind SqlStmtKind db_expr Expr // `db` in `sql db {` object_var_name string // `user` - table_type table.Type pos token.Position where_expr Expr updated_columns []string // for `update set x=y` update_exprs []Expr // for `update` pub mut: - table_name string + table_expr Type fields []table.Field } @@ -1179,12 +1178,11 @@ pub: order_expr Expr has_desc bool is_array bool - table_type table.Type pos token.Position has_limit bool limit_expr Expr pub mut: - table_name string + table_expr Type fields []table.Field } diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 495ed1a28b..187013c27b 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -5220,16 +5220,15 @@ fn (mut c Checker) sql_expr(mut node ast.SqlExpr) table.Type { defer { c.inside_sql = false } - sym := c.table.get_type_symbol(node.table_type) + sym := c.table.get_type_symbol(node.table_expr.typ) if sym.kind == .placeholder { c.error('orm: unknown type `$sym.name`', node.pos) return table.void_type } c.cur_orm_ts = sym info := sym.info as table.Struct - fields := c.fetch_and_verify_orm_fields(info, node.pos, node.table_name) + fields := c.fetch_and_verify_orm_fields(info, node.table_expr.pos, sym.name) node.fields = fields - node.table_name = sym.name if node.has_where { c.expr(node.where_expr) } @@ -5251,17 +5250,18 @@ fn (mut c Checker) sql_stmt(mut node ast.SqlStmt) table.Type { defer { c.inside_sql = false } - if node.table_type == 0 { - c.error('orm: unknown type `$node.table_name`', node.pos) + sym := c.table.get_type_symbol(node.table_expr.typ) + if node.table_expr.typ == 0 { + c.error('orm: unknown type `$sym.name`', node.pos) } - sym := c.table.get_type_symbol(node.table_type) if sym.kind == .placeholder { c.error('orm: unknown type `$sym.name`', node.pos) return table.void_type } c.cur_orm_ts = sym info := sym.info as table.Struct - fields := c.fetch_and_verify_orm_fields(info, node.pos, node.table_name) + table_sym := c.table.get_type_symbol(node.table_expr.typ) + fields := c.fetch_and_verify_orm_fields(info, node.table_expr.pos, table_sym.name) node.fields = fields c.expr(node.db_expr) if node.kind == .update { @@ -5278,6 +5278,7 @@ fn (mut c Checker) fetch_and_verify_orm_fields(info table.Struct, pos token.Posi [table.string_type, table.int_type, table.bool_type] && !it.attrs.contains('skip')) if fields.len == 0 { c.error('V orm: select: empty fields in `$table_name`', pos) + return []table.Field{} } if fields[0].name != 'id' { c.error('V orm: `id int` must be the first field in `$table_name`', pos) diff --git a/vlib/v/checker/tests/orm_empty_struct.out b/vlib/v/checker/tests/orm_empty_struct.out new file mode 100644 index 0000000000..26e5131310 --- /dev/null +++ b/vlib/v/checker/tests/orm_empty_struct.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/orm_empty_struct.vv:9:15: error: V orm: select: empty fields in `Person` + 7 | db := sqlite.connect(':memory:')? + 8 | _ := sql db { + 9 | select from Person + | ~~~~~~ + 10 | } + 11 | } diff --git a/vlib/v/checker/tests/orm_empty_struct.vv b/vlib/v/checker/tests/orm_empty_struct.vv new file mode 100644 index 0000000000..ba7b4e7872 --- /dev/null +++ b/vlib/v/checker/tests/orm_empty_struct.vv @@ -0,0 +1,11 @@ +import sqlite + +struct Person { +} + +fn main() { + db := sqlite.connect(':memory:')? + _ := sql db { + select from Person + } +} diff --git a/vlib/v/fmt/fmt.v b/vlib/v/fmt/fmt.v index 40dd163d11..1c27cea89c 100644 --- a/vlib/v/fmt/fmt.v +++ b/vlib/v/fmt/fmt.v @@ -1327,8 +1327,8 @@ pub fn (mut f Fmt) sql_expr(node ast.SqlExpr) { f.writeln(' {') f.write('\t') f.write('select ') - esym := f.table.get_type_symbol(node.table_type) - table_name := esym.name + esym := f.table.get_type_symbol(node.table_expr.typ) + table_name := util.strip_mod_name(esym.name) if node.is_count { f.write('count ') } else { @@ -1342,7 +1342,7 @@ pub fn (mut f Fmt) sql_expr(node ast.SqlExpr) { f.write(' ') } } - f.write('from ${util.strip_mod_name(table_name)}') + f.write('from $table_name') if node.has_where { f.write(' where ') f.expr(node.where_expr) @@ -2366,12 +2366,13 @@ pub fn (mut f Fmt) sql_stmt(node ast.SqlStmt) { f.write('sql ') f.expr(node.db_expr) f.writeln(' {') + table_name := util.strip_mod_name(f.table.get_type_symbol(node.table_expr.typ).name) match node.kind { .insert { - f.writeln('\tinsert $node.object_var_name into ${util.strip_mod_name(node.table_name)}') + f.writeln('\tinsert $node.object_var_name into $table_name') } .update { - f.write('\tupdate ${util.strip_mod_name(node.table_name)} set ') + f.write('\tupdate $table_name set ') for i, col in node.updated_columns { f.write('$col = ') f.expr(node.update_exprs[i]) @@ -2387,7 +2388,7 @@ pub fn (mut f Fmt) sql_stmt(node ast.SqlStmt) { f.writeln('') } .delete { - f.write('\tdelete from ${util.strip_mod_name(node.table_name)} where ') + f.write('\tdelete from $table_name where ') f.expr(node.where_expr) f.writeln('') } diff --git a/vlib/v/gen/sql.v b/vlib/v/gen/sql.v index d4ae60d8dd..0eba2568da 100644 --- a/vlib/v/gen/sql.v +++ b/vlib/v/gen/sql.v @@ -26,12 +26,13 @@ fn (mut g Gen) sql_stmt(node ast.SqlStmt) { g.expr(node.db_expr) g.writeln(';') g.write('sqlite3_stmt* $g.sql_stmt_name = ${dbtype}__DB_init_stmt($db_name, _SLIT("') + table_name := util.strip_mod_name(g.table.get_type_symbol(node.table_expr.typ).name) if node.kind == .insert { - g.write('INSERT INTO `${util.strip_mod_name(node.table_name)}` (') + g.write('INSERT INTO `$table_name` (') } else if node.kind == .update { - g.write('UPDATE `${util.strip_mod_name(node.table_name)}` SET ') + g.write('UPDATE `$table_name` SET ') } else if node.kind == .delete { - g.write('DELETE FROM `${util.strip_mod_name(node.table_name)}` ') + g.write('DELETE FROM `$table_name` ') } if node.kind == .insert { for i, field in node.fields { @@ -108,9 +109,10 @@ fn (mut g Gen) sql_select_expr(node ast.SqlExpr) { */ cur_line := g.go_before_stmt(0) mut sql_query := 'SELECT ' + table_name := util.strip_mod_name(g.table.get_type_symbol(node.table_expr.typ).name) if node.is_count { // `select count(*) from User` - sql_query += 'COUNT(*) FROM `${util.strip_mod_name(node.table_name)}` ' + sql_query += 'COUNT(*) FROM `$table_name` ' } else { // `select id, name, country from User` for i, field in node.fields { @@ -119,7 +121,7 @@ fn (mut g Gen) sql_select_expr(node ast.SqlExpr) { sql_query += ', ' } } - sql_query += ' FROM `${util.strip_mod_name(node.table_name)}`' + sql_query += ' FROM `$table_name`' } if node.has_where { sql_query += ' WHERE ' diff --git a/vlib/v/parser/sql.v b/vlib/v/parser/sql.v index 5e7ac1bc39..3dcd2645ee 100644 --- a/vlib/v/parser/sql.v +++ b/vlib/v/parser/sql.v @@ -20,6 +20,7 @@ fn (mut p Parser) sql_expr() ast.Expr { p.check_name() // from typ = table.int_type } + table_pos := p.tok.position() table_type := p.parse_type() // `User` mut where_expr := ast.Expr{} has_where := p.tok.kind == .name && p.tok.lit == 'where' @@ -89,7 +90,6 @@ fn (mut p Parser) sql_expr() ast.Expr { is_count: is_count typ: typ db_expr: db_expr - table_type: table_type where_expr: where_expr has_where: has_where has_limit: has_limit @@ -101,6 +101,10 @@ fn (mut p Parser) sql_expr() ast.Expr { has_desc: has_desc is_array: !query_one pos: pos + table_expr: ast.Type{ + typ: table_type + pos: table_pos + } } } @@ -127,18 +131,15 @@ fn (mut p Parser) sql_stmt() ast.SqlStmt { kind = .update } mut inserted_var_name := '' - mut table_name := '' + mut table_type := table.Type(0) if kind != .delete { - expr := p.expr(0) - match expr { - ast.Ident { - if kind == .insert { - inserted_var_name = expr.name - } else if kind == .update { - table_name = expr.name - } - } - else { + if kind == .update { + table_type = p.parse_type() + } else if kind == .insert { + expr := p.expr(0) + if expr is ast.Ident { + inserted_var_name = expr.name + } else { p.error('can only insert variables') return ast.SqlStmt{} } @@ -170,26 +171,18 @@ fn (mut p Parser) sql_stmt() ast.SqlStmt { p.error('expecting `from`') return ast.SqlStmt{} } - mut table_type := table.Type(0) + + mut table_pos := p.tok.position() mut where_expr := ast.Expr{} if kind == .insert { - table_type = p.parse_type() // `User` - sym := p.table.get_type_symbol(table_type) - // info := sym.info as table.Struct - // fields := info.fields.filter(it.typ in [table.string_type, table.int_type, table.bool_type]) - table_name = sym.name + table_pos = p.tok.position() + table_type = p.parse_type() } else if kind == .update { - if !p.pref.is_fmt { - // NB: in vfmt mode, v parses just a single file and table_name may not have been registered - idx := p.table.find_type_idx(p.prepend_mod(table_name)) - table_type = table.new_type(idx) - } p.check_sql_keyword('where') or { return ast.SqlStmt{} } where_expr = p.expr(0) } else if kind == .delete { + table_pos = p.tok.position() table_type = p.parse_type() - sym := p.table.get_type_symbol(table_type) - table_name = sym.name p.check_sql_keyword('where') or { return ast.SqlStmt{} } where_expr = p.expr(0) } @@ -197,8 +190,10 @@ fn (mut p Parser) sql_stmt() ast.SqlStmt { pos.last_line = p.prev_tok.line_nr return ast.SqlStmt{ db_expr: db_expr - table_name: table_name - table_type: table_type + table_expr: ast.Type{ + typ: table_type + pos: table_pos + } object_var_name: inserted_var_name pos: pos updated_columns: updated_columns