From 6ac09e605e721fe87e9e9148d01d74e11418f496 Mon Sep 17 00:00:00 2001
From: Mark aka walkingdevel <104449470+walkingdevel@users.noreply.github.com>
Date: Mon, 8 May 2023 21:21:42 +0000
Subject: [PATCH] orm: allow structs without the id field, more flexible
 primary keys (#18140)

---
 vlib/orm/orm.v                           | 40 +++++++---
 vlib/orm/orm_create_and_drop_test.v      | 49 +++++++++++-
 vlib/orm/orm_insert_test.v               | 95 ++++++++++++++++++++++++
 vlib/v/checker/orm.v                     | 19 ++++-
 vlib/v/gen/c/orm.v                       | 17 ++---
 vlib/v/tests/orm_sub_array_struct_test.v | 19 ++---
 6 files changed, 199 insertions(+), 40 deletions(-)

diff --git a/vlib/orm/orm.v b/vlib/orm/orm.v
index 57997fd03d..555a27a9ad 100644
--- a/vlib/orm/orm.v
+++ b/vlib/orm/orm.v
@@ -116,12 +116,13 @@ fn (kind OrderType) to_str() string {
 // parentheses defines which fields will be inside ()
 pub struct QueryData {
 pub:
-	fields      []string
-	data        []Primitive
-	types       []int
-	parentheses [][]int
-	kinds       []OperationKind
-	is_and      []bool
+	fields              []string
+	data                []Primitive
+	types               []int
+	parentheses         [][]int
+	kinds               []OperationKind
+	primary_column_name string
+	is_and              []bool
 }
 
 pub struct InfixType {
@@ -202,7 +203,18 @@ pub fn orm_stmt_gen(sql_dialect SQLDialect, table string, q string, kind StmtKin
 			mut select_fields := []string{}
 
 			for i in 0 .. data.fields.len {
+				column_name := data.fields[i]
+				is_primary_column := column_name == data.primary_column_name
+
 				if data.data.len > 0 {
+					// Allow the database to insert an automatically generated primary key
+					// under the hood if it is not passed by the user.
+					if is_primary_column && data.data[i].type_idx() in orm.nums {
+						if (data.data[i] as int) == 0 {
+							continue
+						}
+					}
+
 					match data.data[i].type_name() {
 						'string' {
 							if (data.data[i] as string).len == 0 {
@@ -218,9 +230,9 @@ pub fn orm_stmt_gen(sql_dialect SQLDialect, table string, q string, kind StmtKin
 					}
 					data_data << data.data[i]
 				}
-				select_fields << '${q}${data.fields[i]}${q}'
+				select_fields << '${q}${column_name}${q}'
 				values << factory_insert_qm_value(num, qm, c)
-				data_fields << data.fields[i]
+				data_fields << column_name
 				c++
 			}
 
@@ -313,6 +325,7 @@ pub fn orm_stmt_gen(sql_dialect SQLDialect, table string, q string, kind StmtKin
 	$if trace_orm ? {
 		eprintln('> orm: ${str}')
 	}
+
 	return str, QueryData{
 		fields: data_fields
 		data: data_data
@@ -519,9 +532,7 @@ pub fn orm_table_gen(table string, q string, defaults bool, def_unique_len int,
 		}
 		fs << stmt
 	}
-	if primary == '' {
-		return error('A primary key is required for ${table}')
-	}
+
 	if unique.len > 0 {
 		for k, v in unique {
 			mut tmp := []string{}
@@ -531,7 +542,11 @@ pub fn orm_table_gen(table string, q string, defaults bool, def_unique_len int,
 			fs << '/* ${k} */UNIQUE(${tmp.join(', ')})'
 		}
 	}
-	fs << 'PRIMARY KEY(${q}${primary}${q})'
+
+	if primary != '' {
+		fs << 'PRIMARY KEY(${q}${primary}${q})'
+	}
+
 	fs << unique_fields
 	str += fs.join(', ')
 	str += ');'
@@ -541,6 +556,7 @@ pub fn orm_table_gen(table string, q string, defaults bool, def_unique_len int,
 	$if trace_orm ? {
 		eprintln('> orm: ${str}')
 	}
+
 	return str
 }
 
diff --git a/vlib/orm/orm_create_and_drop_test.v b/vlib/orm/orm_create_and_drop_test.v
index a163b3bd60..a49a6e5675 100644
--- a/vlib/orm/orm_create_and_drop_test.v
+++ b/vlib/orm/orm_create_and_drop_test.v
@@ -18,8 +18,53 @@ mut:
 	owner_id int
 }
 
+struct Entity {
+	name        string [primary]
+	description string
+}
+
+fn test_create_without_id_field() {
+	db := sqlite.connect(':memory:')!
+
+	sql db {
+		create table Entity
+	}!
+
+	first := Entity{
+		name: 'First'
+		description: 'Such wow! No `id` field'
+	}
+	second := Entity{
+		name: 'Second'
+		description: 'Such wow! No `id` field again'
+	}
+
+	sql db {
+		insert first into Entity
+		insert second into Entity
+	}!
+
+	entities := sql db {
+		select from Entity
+	}!
+
+	assert entities.len == 2
+
+	first_entity := sql db {
+		select from Entity where name == 'First'
+	}!
+
+	assert first_entity.first().name == 'First'
+
+	second_entity := sql db {
+		select from Entity where name == 'Second'
+	}!
+
+	assert second_entity.first().name == 'Second'
+}
+
 fn test_create_only_one_table() {
-	mut db := sqlite.connect(':memory:') or { panic(err) }
+	mut db := sqlite.connect(':memory:')!
 
 	sql db {
 		create table Parent
@@ -47,7 +92,7 @@ fn test_create_only_one_table() {
 }
 
 fn test_drop_only_one_table() {
-	mut db := sqlite.connect(':memory:') or { panic(err) }
+	mut db := sqlite.connect(':memory:')!
 
 	sql db {
 		create table Parent
diff --git a/vlib/orm/orm_insert_test.v b/vlib/orm/orm_insert_test.v
index e4a43d7cbd..f2fdd44aec 100644
--- a/vlib/orm/orm_insert_test.v
+++ b/vlib/orm/orm_insert_test.v
@@ -1,4 +1,5 @@
 import db.sqlite
+import rand
 
 struct Parent {
 	id       int     [primary; sql: serial]
@@ -37,12 +38,106 @@ pub mut:
 	username string [unique]
 }
 
+struct Entity {
+	uuid        string [primary]
+	description string
+}
+
+struct EntityWithFloatPrimary {
+	id   f64    [primary]
+	name string
+}
+
 pub fn insert_parent(db sqlite.DB, mut parent Parent) ! {
 	sql db {
 		insert parent into Parent
 	}!
 }
 
+fn test_set_primary_value() {
+	// The primary key is an constraint that ensures each record in a table is unique.
+	// Primary keys must contain unique values and cannot contain `NULL` values.
+	// However, this statement does not imply that a value cannot be inserted by the user.
+	// Therefore, let's allow this.
+	db := sqlite.connect(':memory:')!
+
+	sql db {
+		create table Child
+	}!
+
+	child := Child{
+		id: 10
+		parent_id: 20
+	}
+
+	sql db {
+		insert child into Child
+	}!
+
+	children := sql db {
+		select from Child
+	}!
+
+	assert children.first() == child
+}
+
+fn test_uuid_primary_key() {
+	db := sqlite.connect(':memory:')!
+	uuid := rand.uuid_v4()
+
+	sql db {
+		create table Entity
+	}!
+
+	entity := Entity{
+		uuid: uuid
+		description: 'Test'
+	}
+
+	sql db {
+		insert entity into Entity
+	}!
+
+	entities := sql db {
+		select from Entity where uuid == uuid
+	}!
+
+	mut is_duplicate_inserted := true
+
+	sql db {
+		insert entity into Entity
+	} or { is_duplicate_inserted = false }
+
+	assert entities.len == 1
+	assert entities.first() == entity
+	assert is_duplicate_inserted == false
+}
+
+fn test_float_primary_key() {
+	db := sqlite.connect(':memory:')!
+	id := 3.14
+
+	sql db {
+		create table EntityWithFloatPrimary
+	}!
+
+	entity := EntityWithFloatPrimary{
+		id: id
+		name: 'Test'
+	}
+
+	sql db {
+		insert entity into EntityWithFloatPrimary
+	}!
+
+	entities := sql db {
+		select from EntityWithFloatPrimary where id == id
+	}!
+
+	assert entities.len == 1
+	assert entities.first() == entity
+}
+
 fn test_does_not_insert_uninitialized_field() {
 	db := sqlite.connect(':memory:')!
 
diff --git a/vlib/v/checker/orm.v b/vlib/v/checker/orm.v
index ad952942f7..3513ad57dc 100644
--- a/vlib/v/checker/orm.v
+++ b/vlib/v/checker/orm.v
@@ -212,6 +212,7 @@ fn (mut c Checker) sql_stmt_line(mut node ast.SqlStmtLine) ast.Type {
 	info := table_sym.info as ast.Struct
 	mut fields := c.fetch_and_verify_orm_fields(info, node.table_expr.pos, table_sym.name)
 	mut sub_structs := map[int]ast.SqlStmtLine{}
+
 	for f in fields.filter((c.table.type_symbols[int(it.typ)].kind == .struct_
 		|| (c.table.sym(it.typ).kind == .array
 		&& c.table.sym(c.table.sym(it.typ).array_info().elem_type).kind == .struct_))
@@ -332,9 +333,7 @@ fn (mut c Checker) fetch_and_verify_orm_fields(info ast.Struct, pos token.Pos, t
 		c.orm_error('select: empty fields in `${table_name}`', pos)
 		return []ast.StructField{}
 	}
-	if fields[0].name != 'id' {
-		c.orm_error('`id int` must be the first field in `${table_name}`', pos)
-	}
+
 	return fields
 }
 
@@ -537,6 +536,20 @@ fn (mut c Checker) check_db_expr(db_expr &ast.Expr) bool {
 	return true
 }
 
+// walkingdevel: Now I don't think it's a good solution
+// because it only checks structure initialization,
+// but structure fields may be updated later before inserting.
+// For example,
+// ```v
+// mut package := Package{
+// 	name: 'xml'
+// }
+//
+// package.author = User{
+// 	username: 'walkingdevel'
+// }
+// ```
+// TODO: rewrite it, move to runtime.
 fn (_ &Checker) check_field_of_inserting_struct_is_uninitialized(node &ast.SqlStmtLine, field_name string) bool {
 	struct_scope := node.scope.find_var(node.object_var_name) or { return false }
 
diff --git a/vlib/v/gen/c/orm.v b/vlib/v/gen/c/orm.v
index 846e07eaaf..ffba121244 100644
--- a/vlib/v/gen/c/orm.v
+++ b/vlib/v/gen/c/orm.v
@@ -302,6 +302,7 @@ fn (mut g Gen) write_orm_insert_with_last_ids(node ast.SqlStmtLine, connection_v
 	}
 
 	fields := node.fields.filter(g.table.sym(it.typ).kind != .array)
+	primary_field_name := g.get_orm_struct_primary_field_name(fields) or { '' }
 
 	for sub in subs {
 		g.sql_stmt_line(sub, connection_var_name, or_expr)
@@ -373,6 +374,7 @@ fn (mut g Gen) write_orm_insert_with_last_ids(node ast.SqlStmtLine, connection_v
 	g.indent--
 	g.writeln('),')
 	g.writeln('.types = __new_array_with_default_noscan(0, 0, sizeof(int), 0),')
+	g.writeln('.primary_column_name = _SLIT("${primary_field_name}"),')
 	g.writeln('.kinds = __new_array_with_default_noscan(0, 0, sizeof(orm__OperationKind), 0),')
 	g.writeln('.is_and = __new_array_with_default_noscan(0, 0, sizeof(bool), 0),')
 	g.indent--
@@ -396,11 +398,7 @@ fn (mut g Gen) write_orm_insert_with_last_ids(node ast.SqlStmtLine, connection_v
 			mut fff := []ast.StructField{}
 			for f in arr.fields {
 				mut skip := false
-				mut primary := false
 				for attr in f.attrs {
-					if attr.name == 'primary' {
-						primary = true
-					}
 					if attr.name == 'skip' {
 						skip = true
 					}
@@ -408,7 +406,7 @@ fn (mut g Gen) write_orm_insert_with_last_ids(node ast.SqlStmtLine, connection_v
 						skip = true
 					}
 				}
-				if !skip && !primary {
+				if !skip {
 					fff << f
 				}
 			}
@@ -986,19 +984,14 @@ fn (mut g Gen) write_orm_select(node ast.SqlExpr, connection_var_name string, le
 }
 
 // filter_struct_fields_by_orm_attrs filters struct fields taking into its attributes.
-// Used by non-create queries for skipping fields
-// if it has a `skip` attribute or `primary` that inserts automatically.
+// Used by non-create queries for skipping fields.
 fn (_ &Gen) filter_struct_fields_by_orm_attrs(fields []ast.StructField) []ast.StructField {
 	mut result := []ast.StructField{}
 
 	for field in fields {
 		mut skip := false
-		mut primary := false
 
 		for attr in field.attrs {
-			if attr.name == 'primary' {
-				primary = true
-			}
 			if attr.name == 'skip' {
 				skip = true
 			}
@@ -1007,7 +1000,7 @@ fn (_ &Gen) filter_struct_fields_by_orm_attrs(fields []ast.StructField) []ast.St
 			}
 		}
 
-		if !skip && !primary {
+		if !skip {
 			result << field
 		}
 	}
diff --git a/vlib/v/tests/orm_sub_array_struct_test.v b/vlib/v/tests/orm_sub_array_struct_test.v
index 15c78b3d88..904e12b5e6 100644
--- a/vlib/v/tests/orm_sub_array_struct_test.v
+++ b/vlib/v/tests/orm_sub_array_struct_test.v
@@ -22,7 +22,7 @@ fn test_orm_array() {
 		create table Child
 	}!
 
-	par := Parent{
+	new_parent := Parent{
 		name: 'test'
 		children: [
 			Child{
@@ -35,7 +35,7 @@ fn test_orm_array() {
 	}
 
 	sql db {
-		insert par into Parent
+		insert new_parent into Parent
 	}!
 
 	parents := sql db {
@@ -47,8 +47,8 @@ fn test_orm_array() {
 	}!
 
 	parent := parents.first()
-	assert parent.name == par.name
-	assert parent.children.len == par.children.len
+	assert parent.name == new_parent.name
+	assert parent.children.len == new_parent.children.len
 	assert parent.children[0].name == 'abc'
 	assert parent.children[1].name == 'def'
 }
@@ -57,8 +57,6 @@ fn test_orm_relationship() {
 	mut db := sqlite.connect(':memory:') or { panic(err) }
 	sql db {
 		create table Parent
-	}!
-	sql db {
 		create table Child
 	}!
 
@@ -66,13 +64,12 @@ fn test_orm_relationship() {
 		name: 'abc'
 	}
 
-	par := Parent{
+	new_parent := Parent{
 		name: 'test'
 		children: []
 	}
-
 	sql db {
-		insert par into Parent
+		insert new_parent into Parent
 	}!
 
 	mut parents := sql db {
@@ -93,7 +90,7 @@ fn test_orm_relationship() {
 		insert child into Child
 	}!
 
-	assert parent.name == par.name
+	assert parent.name == new_parent.name
 	assert parent.children.len == 0
 
 	parents = sql db {
@@ -101,7 +98,7 @@ fn test_orm_relationship() {
 	}!
 
 	parent = parents.first()
-	assert parent.name == par.name
+	assert parent.name == new_parent.name
 	assert parent.children.len == 2
 	assert parent.children[0].name == 'atum'
 	assert parent.children[1].name == 'bacon'