From d0e78b1da6d49d3b57453a0addf635e8fd27486c Mon Sep 17 00:00:00 2001
From: walking devel <104449470+walkingdevel@users.noreply.github.com>
Date: Wed, 22 Mar 2023 07:48:01 +0000
Subject: [PATCH] parser: breaking change, let V ORM queries return arrays for
 *all* non-count queries, including `limit = 1` (#17719)

---
 .../v_apps_and_modules_compile_ci.yml         | 34 +++----
 doc/docs.md                                   |  9 +-
 examples/vweb_fullstack/src/auth_services.v   |  5 +-
 examples/vweb_fullstack/src/user_services.v   |  2 +-
 examples/vweb_orm_jwt/src/auth_services.v     |  8 +-
 examples/vweb_orm_jwt/src/user_services.v     |  8 +-
 vlib/db/sqlite/sqlite_orm_test.v              |  3 +-
 vlib/orm/orm_insert_test.v                    |  3 +-
 vlib/orm/orm_sql_or_blocks_test.v             | 12 +--
 vlib/orm/orm_test.v                           | 90 ++++++++++---------
 vlib/v/parser/sql.v                           | 54 +++--------
 vlib/v/tests/orm_sub_array_struct_test.v      |  9 +-
 vlib/v/tests/orm_sub_struct_test.v            |  4 +-
 .../tests/sql_statement_inside_fn_call_test.v |  4 +-
 14 files changed, 117 insertions(+), 128 deletions(-)

diff --git a/.github/workflows/v_apps_and_modules_compile_ci.yml b/.github/workflows/v_apps_and_modules_compile_ci.yml
index 204b2b71c0..a57fcbc392 100644
--- a/.github/workflows/v_apps_and_modules_compile_ci.yml
+++ b/.github/workflows/v_apps_and_modules_compile_ci.yml
@@ -67,23 +67,23 @@ jobs:
           echo "Build Coreutils"
           cd /tmp/coreutils; make
 
-      - name: Build vlang/gitly
-        run: |
-          echo "Install markdown"
-          v install markdown
-          echo "Install pcre"
-          v install pcre
-          echo "Clone Gitly"
-          git clone https://github.com/vlang/gitly /tmp/gitly
-          echo "Build Gitly"
-          v /tmp/gitly
-          ## echo "Build Gitly with -autofree"
-          ## v -autofree /tmp/gitly
-          echo "Compile gitly.css from gitly.scss"
-          sassc /tmp/gitly/src/static/css/gitly.scss > /tmp/gitly/src/static/css/gitly.css
-          echo "Run first_run.v"
-          v run /tmp/gitly/tests/first_run.v
-          # /tmp/gitly/gitly -ci_run
+#      - name: Build vlang/gitly
+#        run: |
+#          echo "Install markdown"
+#          v install markdown
+#          echo "Install pcre"
+#          v install pcre
+#          echo "Clone Gitly"
+#          git clone https://github.com/vlang/gitly /tmp/gitly
+#          echo "Build Gitly"
+#          v /tmp/gitly
+#          ## echo "Build Gitly with -autofree"
+#          ## v -autofree /tmp/gitly
+#          echo "Compile gitly.css from gitly.scss"
+#          sassc /tmp/gitly/src/static/css/gitly.scss > /tmp/gitly/src/static/css/gitly.css
+#          echo "Run first_run.v"
+#          v run /tmp/gitly/tests/first_run.v
+#          # /tmp/gitly/gitly -ci_run
 
       - name: Build V Language Server (VLS) vlang/vls
         run: |
diff --git a/doc/docs.md b/doc/docs.md
index 5b5d43225c..9bfdf68126 100644
--- a/doc/docs.md
+++ b/doc/docs.md
@@ -4836,7 +4836,7 @@ struct Customer {
 	country   string [nonull]
 }
 
-db := sqlite.connect('customers.db')?
+db := sqlite.connect('customers.db')!
 
 // you can create tables:
 // CREATE TABLE IF NOT EXISTS `Customer` (
@@ -4854,6 +4854,7 @@ nr_customers := sql db {
 	select count from Customer
 }
 println('number of all customers: ${nr_customers}')
+
 // V syntax can be used to build queries
 uk_customers := sql db {
 	select from Customer where country == 'uk' && nr_orders > 0
@@ -4862,11 +4863,7 @@ println(uk_customers.len)
 for customer in uk_customers {
 	println('${customer.id} - ${customer.name}')
 }
-// by adding `limit 1` we tell V that there will be only one object
-customer := sql db {
-	select from Customer where id == 1 limit 1
-}
-println('${customer.id} - ${customer.name}')
+
 // insert a new customer
 new_customer := Customer{
 	name: 'Bob'
diff --git a/examples/vweb_fullstack/src/auth_services.v b/examples/vweb_fullstack/src/auth_services.v
index 28861f2f03..db3da637bc 100644
--- a/examples/vweb_fullstack/src/auth_services.v
+++ b/examples/vweb_fullstack/src/auth_services.v
@@ -34,9 +34,10 @@ fn (mut app App) service_auth(username string, password string) !string {
 		db.close() or { panic(err) }
 	}
 
-	user := sql db {
-		select from User where username == username limit 1
+	users := sql db {
+		select from User where username == username
 	}
+	user := users.first()
 	if user.username != username {
 		return error('user not found')
 	}
diff --git a/examples/vweb_fullstack/src/user_services.v b/examples/vweb_fullstack/src/user_services.v
index 30e675f3ca..222c3d62f7 100644
--- a/examples/vweb_fullstack/src/user_services.v
+++ b/examples/vweb_fullstack/src/user_services.v
@@ -61,5 +61,5 @@ fn (mut app App) service_get_user(id int) !User {
 		select from User where id == id
 	}
 
-	return results
+	return results.first()
 }
diff --git a/examples/vweb_orm_jwt/src/auth_services.v b/examples/vweb_orm_jwt/src/auth_services.v
index f56d0cfaa9..82e2c15ac5 100644
--- a/examples/vweb_orm_jwt/src/auth_services.v
+++ b/examples/vweb_orm_jwt/src/auth_services.v
@@ -31,13 +31,15 @@ fn (mut app App) service_auth(username string, password string) !string {
 		panic(err)
 	}
 
-	user := sql db {
-		select from User where username == username limit 1
+	users := sql db {
+		select from User where username == username
 	}
-	if user.username != username {
+
+	if users.len == 0 {
 		return error('user not found')
 	}
 
+	user := users.first()
 	if !user.active {
 		return error('user is not active')
 	}
diff --git a/examples/vweb_orm_jwt/src/user_services.v b/examples/vweb_orm_jwt/src/user_services.v
index d2006c193f..a579b9c1f5 100644
--- a/examples/vweb_orm_jwt/src/user_services.v
+++ b/examples/vweb_orm_jwt/src/user_services.v
@@ -31,11 +31,11 @@ fn (mut app App) service_add_user(username string, password string) !User {
 		return err
 	}
 
-	result := sql db {
+	users := sql db {
 		select from User where username == username limit 1
 	}
 
-	return result
+	return users.first()
 }
 
 fn (mut app App) service_get_user_by_id(user_id int) !User {
@@ -48,11 +48,11 @@ fn (mut app App) service_get_user_by_id(user_id int) !User {
 		db.close() or { panic(err) }
 	}
 
-	results := sql db {
+	users := sql db {
 		select from User where id == user_id
 	}
 
-	return results
+	return users.first()
 }
 
 fn (mut app App) service_get_all_user() ![]User {
diff --git a/vlib/db/sqlite/sqlite_orm_test.v b/vlib/db/sqlite/sqlite_orm_test.v
index 82527668c5..94f780cff2 100644
--- a/vlib/db/sqlite/sqlite_orm_test.v
+++ b/vlib/db/sqlite/sqlite_orm_test.v
@@ -149,10 +149,11 @@ fn test_sqlite_orm() {
 		insert test_default_atribute into TestDefaultAtribute
 	}
 
-	result_test_default_atribute := sql db {
+	test_default_atributes := sql db {
 		select from TestDefaultAtribute limit 1
 	}
 
+	result_test_default_atribute := test_default_atributes.first()
 	assert result_test_default_atribute.name == 'Hitalo'
 	assert test_default_atribute.created_at.len == 0
 	assert test_default_atribute.created_at1.len == 0
diff --git a/vlib/orm/orm_insert_test.v b/vlib/orm/orm_insert_test.v
index dcbc1fc04e..0833d8c52e 100644
--- a/vlib/orm/orm_insert_test.v
+++ b/vlib/orm/orm_insert_test.v
@@ -106,10 +106,11 @@ fn test_orm_insert_with_multiple_child_elements() {
 		insert new_parent into Parent
 	}
 
-	parent := sql db {
+	parents := sql db {
 		select from Parent where id == 1
 	}
 
+	parent := parents.first()
 	assert parent.children.len == new_parent.children.len
 	assert parent.notes.len == new_parent.notes.len
 
diff --git a/vlib/orm/orm_sql_or_blocks_test.v b/vlib/orm/orm_sql_or_blocks_test.v
index ac56f923da..906f45b9ea 100644
--- a/vlib/orm/orm_sql_or_blocks_test.v
+++ b/vlib/orm/orm_sql_or_blocks_test.v
@@ -60,26 +60,26 @@ fn test_sql_or_block_for_select() {
 	mut db := sqlite.connect(db_path)!
 
 	eprintln('> selecting user with id 1...')
-	single := sql db {
+	mut users := sql db {
 		select from User where id == 1
 	} or {
 		eprintln('could not select user, err: ${err}')
-		User{0, ''}
+		[]User{}
 	}
 	eprintln('LINE: ${@LINE}')
 
+	single := users.first()
 	assert single.id == 1
 
-	failed := sql db {
+	users = sql db {
 		select from User where id == 0
 	} or {
 		eprintln('could not select user, err: ${err}')
-		User{0, ''}
+		[]User{}
 	}
 	eprintln('LINE: ${@LINE}')
 
-	assert failed.id == 0
-	assert failed.name == ''
+	assert users.len == 0
 	eprintln('LINE: ${@LINE}')
 
 	eprintln('> selecting users...')
diff --git a/vlib/orm/orm_test.v b/vlib/orm/orm_test.v
index a1033cc42a..2d738d9e1a 100644
--- a/vlib/orm/orm_test.v
+++ b/vlib/orm/orm_test.v
@@ -104,19 +104,16 @@ fn test_orm() {
 		select count from User
 	}
 	assert nr_all_users == 3
-	println('nr_all_users=${nr_all_users}')
 
 	nr_users1 := sql db {
 		select count from User where id == 1
 	}
 	assert nr_users1 == 1
-	println('nr_users1=${nr_users1}')
 
 	nr_peters := sql db {
 		select count from User where id == 2 && name == 'Peter'
 	}
 	assert nr_peters == 1
-	println('nr_peters=${nr_peters}')
 
 	nr_peters2 := sql db {
 		select count from User where id == 2 && name == name
@@ -134,24 +131,26 @@ fn test_orm() {
 	assert peters.len == 1
 	assert peters[0].name == 'Peter'
 
-	one_peter := sql db {
+	mut users := sql db {
 		select from User where name == name limit 1
 	}
+
+	one_peter := users.first()
 	assert one_peter.name == 'Peter'
 	assert one_peter.id == 2
 
-	user := sql db {
+	users = sql db {
 		select from User where id == 1
 	}
-	println(user)
+
+	user := users.first()
 	assert user.name == 'Sam'
 	assert user.id == 1
 	assert user.age == 29
 
-	users := sql db {
+	users = sql db {
 		select from User where id > 0
 	}
-	println(users)
 	assert users.len == 3
 	assert users[0].name == 'Sam'
 	assert users[1].name == 'Peter'
@@ -160,23 +159,16 @@ fn test_orm() {
 	users2 := sql db {
 		select from User where id < 0
 	}
-	println(users2)
 	assert users2.len == 0
 
 	users3 := sql db {
 		select from User where age == 29 || age == 31
 	}
-	println(users3)
+
 	assert users3.len == 2
 	assert users3[0].age == 29
 	assert users3[1].age == 31
 
-	missing_user := sql db {
-		select from User where id == 8777
-	}
-	println('missing_user:')
-	println(missing_user) // zero struct
-
 	new_user := User{
 		name: 'New user'
 		age: 30
@@ -185,22 +177,27 @@ fn test_orm() {
 		insert new_user into User
 	}
 
-	x := sql db {
+	users = sql db {
 		select from User where id == 4
 	}
-	println(x)
+
+	x := users.first()
 	assert x.age == 30
 	assert x.id == 4
 	assert x.name == 'New user'
 
-	kate := sql db {
+	users = sql db {
 		select from User where id == 3
 	}
+
+	kate := users.first()
 	assert kate.is_customer == true
 
-	customer := sql db {
+	users = sql db {
 		select from User where is_customer == true limit 1
 	}
+
+	customer := users.first()
 	assert customer.is_customer == true
 	assert customer.name == 'Kate'
 
@@ -208,9 +205,10 @@ fn test_orm() {
 		update User set age = 31 where name == 'Kate'
 	}
 
-	kate2 := sql db {
+	users = sql db {
 		select from User where id == 3
 	}
+	kate2 := users.first()
 	assert kate2.age == 31
 	assert kate2.name == 'Kate'
 
@@ -218,9 +216,10 @@ fn test_orm() {
 		update User set age = 32, name = 'Kate N' where name == 'Kate'
 	}
 
-	mut kate3 := sql db {
+	users = sql db {
 		select from User where id == 3
 	}
+	mut kate3 := users.first()
 	assert kate3.age == 32
 	assert kate3.name == 'Kate N'
 
@@ -229,9 +228,11 @@ fn test_orm() {
 		update User set age = new_age, name = 'Kate N' where id == 3
 	}
 
-	kate3 = sql db {
+	users = sql db {
 		select from User where id == 3
 	}
+
+	kate3 = users.first()
 	assert kate3.age == 33
 	assert kate3.name == 'Kate N'
 
@@ -240,17 +241,18 @@ fn test_orm() {
 		update User set age = foo.age, name = 'Kate N' where id == 3
 	}
 
-	kate3 = sql db {
+	users = sql db {
 		select from User where id == 3
 	}
+	kate3 = users.first()
 	assert kate3.age == 34
 	assert kate3.name == 'Kate N'
 
 	no_user := sql db {
 		select from User where id == 30
 	}
-	assert no_user.name == '' // TODO optional
-	assert no_user.age == 0
+
+	assert no_user.len == 0
 
 	two_users := sql db {
 		select from User limit 2
@@ -270,35 +272,41 @@ fn test_orm() {
 	assert z.len == 2
 	assert z[0].id == 3
 
-	oldest := sql db {
+	users = sql db {
 		select from User order by age desc limit 1
 	}
+
+	oldest := users.first()
 	assert oldest.age == 34
 
 	offs := 1
-	second_oldest := sql db {
+	users = sql db {
 		select from User order by age desc limit 1 offset offs
 	}
+
+	second_oldest := users.first()
 	assert second_oldest.age == 31
 	sql db {
 		delete from User where age == 34
 	}
 
-	updated_oldest := sql db {
+	users = sql db {
 		select from User order by age desc limit 1
 	}
+	updated_oldest := users.first()
 	assert updated_oldest.age == 31
 
 	// Remove this when pg is used
 	// db.exec('insert into User (name, age) values (NULL, 31)')
-	null_user := sql db {
+	users = sql db {
 		select from User where id == 5
 	}
-	assert null_user.name == ''
+	assert users.len == 0
 
-	age_test := sql db {
+	users = sql db {
 		select from User where id == 1
 	}
+	age_test := users.first()
 
 	assert age_test.age == 29
 
@@ -306,20 +314,22 @@ fn test_orm() {
 		update User set age = age + 1 where id == 1
 	}
 
-	mut first := sql db {
+	users = sql db {
 		select from User where id == 1
 	}
 
+	mut first := users.first()
 	assert first.age == 30
 
 	sql db {
 		update User set age = age * 2 where id == 1
 	}
 
-	first = sql db {
+	users = sql db {
 		select from User where id == 1
 	}
 
+	first = users.first()
 	assert first.age == 60
 
 	sql db {
@@ -353,31 +363,31 @@ fn test_orm() {
 		update Module set test_id = 11 where id == 1
 	}
 
-	test_id_mod := sql db {
+	mut modules := sql db {
 		select from Module where id == 1
 	}
 
-	assert test_id_mod.test_id == 11
+	assert modules.first().test_id == 11
 
 	t := time.now()
 	sql db {
 		update Module set created = t where id == 1
 	}
 
-	updated_time_mod := sql db {
+	modules = sql db {
 		select from Module where id == 1
 	}
 
 	// Note: usually updated_time_mod.created != t, because t has
 	// its microseconds set, while the value retrieved from the DB
 	// has them zeroed, because the db field resolution is seconds.
-	assert updated_time_mod.created.format_ss() == t.format_ss()
+	assert modules.first().created.format_ss() == t.format_ss()
 
-	para_select := sql db {
+	users = sql db {
 		select from User where (name == 'Sam' && is_customer == true) || id == 1
 	}
 
-	assert para_select[0] == first
+	assert users.first() == first
 
 	sql db {
 		drop table Module
diff --git a/vlib/v/parser/sql.v b/vlib/v/parser/sql.v
index 21afb16404..862b554c78 100644
--- a/vlib/v/parser/sql.v
+++ b/vlib/v/parser/sql.v
@@ -16,18 +16,19 @@ fn (mut p Parser) sql_expr() ast.Expr {
 	}
 	p.check(.lcbr)
 	p.check(.key_select)
-	n := p.check_name()
-	is_count := n == 'count'
+	is_count := p.check_name() == 'count'
 	mut typ := ast.void_type
+
 	if is_count {
 		p.check_name() // from
-		typ = ast.int_type
 	}
+
 	table_pos := p.tok.pos()
 	table_type := p.parse_type() // `User`
+
 	mut where_expr := ast.empty_expr
 	has_where := p.tok.kind == .name && p.tok.lit == 'where'
-	mut query_one := false // one object is returned, not an array
+
 	if has_where {
 		p.next()
 		where_expr = p.expr(0)
@@ -37,11 +38,8 @@ fn (mut p Parser) sql_expr() ast.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
 	mut limit_expr := ast.empty_expr
 	mut has_offset := false
@@ -64,29 +62,25 @@ fn (mut p Parser) sql_expr() ast.Expr {
 			has_desc = true
 		}
 	}
+
 	if p.tok.kind == .name && p.tok.lit == 'limit' {
-		// `limit 1` means that a single object is returned
 		p.check_name() // `limit`
-		if p.tok.kind == .number && p.tok.lit == '1' {
-			query_one = true
-		}
 		has_limit = true
 		limit_expr = p.expr(0)
 	}
+
 	if p.tok.kind == .name && p.tok.lit == 'offset' {
 		p.check_name() // `offset`
 		has_offset = true
 		offset_expr = p.expr(0)
 	}
-	if !query_one && !is_count {
-		// return an array
+
+	if is_count {
+		typ = ast.int_type
+	} else {
 		typ = ast.new_type(p.table.find_or_register_array(table_type))
-	} else if !is_count {
-		// return a single object
-		// TODO optional
-		// typ = table_type.set_flag(.optional)
-		typ = table_type
 	}
+
 	p.check(.rcbr)
 	p.inside_match = false
 	or_expr := p.parse_sql_or_block()
@@ -105,7 +99,7 @@ fn (mut p Parser) sql_expr() ast.Expr {
 		has_order: has_order
 		order_expr: order_expr
 		has_desc: has_desc
-		is_array: !query_one
+		is_array: if is_count { false } else { true }
 		pos: pos.extend(p.prev_tok.pos())
 		table_expr: ast.TypeNode{
 			typ: table_type
@@ -304,26 +298,6 @@ 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 {
diff --git a/vlib/v/tests/orm_sub_array_struct_test.v b/vlib/v/tests/orm_sub_array_struct_test.v
index 365fd8af32..b547d3329a 100644
--- a/vlib/v/tests/orm_sub_array_struct_test.v
+++ b/vlib/v/tests/orm_sub_array_struct_test.v
@@ -38,7 +38,7 @@ fn test_orm_array() {
 		insert par into Parent
 	}
 
-	parent := sql db {
+	parents := sql db {
 		select from Parent where id == 1
 	}
 
@@ -46,6 +46,7 @@ fn test_orm_array() {
 		drop table Parent
 	}
 
+	parent := parents.first()
 	assert parent.name == par.name
 	assert parent.children.len == par.children.len
 	assert parent.children[0].name == 'abc'
@@ -74,10 +75,11 @@ fn test_orm_relationship() {
 		insert par into Parent
 	}
 
-	mut parent := sql db {
+	mut parents := sql db {
 		select from Parent where id == 1
 	}
 
+	mut parent := parents.first()
 	child.parent_id = parent.id
 	child.name = 'atum'
 
@@ -94,10 +96,11 @@ fn test_orm_relationship() {
 	assert parent.name == par.name
 	assert parent.children.len == 0
 
-	parent = sql db {
+	parents = sql db {
 		select from Parent where id == 1
 	}
 
+	parent = parents.first()
 	assert parent.name == par.name
 	assert parent.children.len == 2
 	assert parent.children[0].name == 'atum'
diff --git a/vlib/v/tests/orm_sub_struct_test.v b/vlib/v/tests/orm_sub_struct_test.v
index 46909fac40..2110b76a9e 100644
--- a/vlib/v/tests/orm_sub_struct_test.v
+++ b/vlib/v/tests/orm_sub_struct_test.v
@@ -29,9 +29,9 @@ fn test_orm_sub_structs() {
 		insert upper_1 into Upper
 	}
 
-	upper_s := sql db {
+	uppers := sql db {
 		select from Upper where id == 1
 	}
 
-	assert upper_s.sub.name == upper_1.sub.name
+	assert uppers.first().sub.name == upper_1.sub.name
 }
diff --git a/vlib/v/tests/sql_statement_inside_fn_call_test.v b/vlib/v/tests/sql_statement_inside_fn_call_test.v
index ba5410bfbf..88eebe00db 100644
--- a/vlib/v/tests/sql_statement_inside_fn_call_test.v
+++ b/vlib/v/tests/sql_statement_inside_fn_call_test.v
@@ -18,7 +18,7 @@ fn test_sql_statement_inside_fn_call() {
 	sql db {
 		insert m into Movie
 	}
-	dump(x(sql db {
+	dump(x((sql db {
 		select from Movie where id == 1
-	}))
+	}).first()))
 }