[tarantool-patches] Re: [PATCH v9 1/7] sql: add column name to SQL change counter

[Mergen Imeev <imeevma@tarantool.org>]

Hi! Thank you for review. My answer, diff and new patch below.
Actually, I am not sure that this pragma will be useful after
box.sql.execute() will be deleted because INSERT, UPDATE, REPLACE
and DELETE returns rowcount after execution. This pragma do the
same thing, but only for INSERT, UPDATE and REPLACE. I think it
should work for DELETE, but it doesn't. The only difference is
the way the data shown: if pragma is on then the data returned as
rows with metadata, otherwise the data returned as info.


On Fri, Mar 22, 2019 at 06:42:00PM +0300, Konstantin Osipov wrote:
> * imeevma@tarantool.org <imeevma@tarantool.org> [19/03/22 13:57]:
> 
> The patch is OK to push.
> 
> > +cn:execute("INSERT INTO t1 VALUES (1), (2), (3);")
> > +---
> > +- metadata:
> > +  - name: rows inserted
> > +    type: INTEGER
> > +  rows:
> > +  - [3]
> > +...
> > +cn:execute("REPLACE INTO t1 VALUES (2), (3), (4), (5);")
> > +---
> > +- metadata:
> > +  - name: rows inserted
> 
> Why rows inserted, not rows replaced?
>
Fixed. 
> > +    type: INTEGER
> > +  rows:
> > +  - [4]
> > +...
> > +cn:execute("UPDATE t1 SET id = id + 100 WHERE id > 10;")
> > +---
> > +- metadata:
> > +  - name: rows updated
> > +    type: INTEGER
> > +  rows:
> > +  - [0]
> > +...
> 
> -- 
> Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
> http://tarantool.io - www.twitter.com/kostja_osipov
> 


Diff:

diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 654b282..1c6b7cf 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -786,8 +786,12 @@ sqlInsert(Parse * pParse,	/* Parser context */
 	    pParse->triggered_space == NULL) {
 		sqlVdbeAddOp2(v, OP_ResultRow, regRowCount, 1);
 		sqlVdbeSetNumCols(v, 1);
-		sqlVdbeSetColName(v, 0, COLNAME_NAME, "rows inserted",
-				      SQL_STATIC);
+		const char *column_name;
+		if (on_error == ON_CONFLICT_ACTION_REPLACE)
+			column_name = "rows replaced";
+		else
+			column_name = "rows inserted";
+		sqlVdbeSetColName(v, 0, COLNAME_NAME, column_name, SQL_STATIC);
 		sqlVdbeSetColName(v, 0, COLNAME_DECLTYPE, "INTEGER",
 				  SQL_STATIC);
 	}
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index c700a80..0944f61 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -960,7 +960,7 @@ cn:execute("INSERT INTO t1 VALUES (1), (2), (3);")
 cn:execute("REPLACE INTO t1 VALUES (2), (3), (4), (5);")
 ---
 - metadata:
-  - name: rows inserted
+  - name: rows replaced
     type: INTEGER
   rows:
   - [4]


New patch:

commit c830c5bd283daa2a7f34b7e6a4a3ffccef28ec52
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Sat Mar 16 17:20:00 2019 +0300

    sql: add column name to SQL change counter
    
    Currently, if the count_changes pragma is enabled, the INSERT,
    REPLACE and UPDATE statements will return the number of changes at
    execution time. This patch sets an INTEGER type for this result.
    
    Follow up #3832
    Needed from #3505

diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index f4d0334..eb1c8aa 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -422,6 +422,8 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 		sqlVdbeSetNumCols(v, 1);
 		sqlVdbeSetColName(v, 0, COLNAME_NAME, "rows deleted",
 				      SQL_STATIC);
+		sqlVdbeSetColName(v, 0, COLNAME_DECLTYPE, "INTEGER",
+				  SQL_STATIC);
 	}
 
  delete_from_cleanup:
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 6f7f020..1c6b7cf 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -786,8 +786,14 @@ sqlInsert(Parse * pParse,	/* Parser context */
 	    pParse->triggered_space == NULL) {
 		sqlVdbeAddOp2(v, OP_ResultRow, regRowCount, 1);
 		sqlVdbeSetNumCols(v, 1);
-		sqlVdbeSetColName(v, 0, COLNAME_NAME, "rows inserted",
-				      SQL_STATIC);
+		const char *column_name;
+		if (on_error == ON_CONFLICT_ACTION_REPLACE)
+			column_name = "rows replaced";
+		else
+			column_name = "rows inserted";
+		sqlVdbeSetColName(v, 0, COLNAME_NAME, column_name, SQL_STATIC);
+		sqlVdbeSetColName(v, 0, COLNAME_DECLTYPE, "INTEGER",
+				  SQL_STATIC);
 	}
 
  insert_cleanup:
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 05ceeb4..82de09c 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -530,6 +530,8 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 		sqlVdbeSetNumCols(v, 1);
 		sqlVdbeSetColName(v, 0, COLNAME_NAME, "rows updated",
 				      SQL_STATIC);
+		sqlVdbeSetColName(v, 0, COLNAME_DECLTYPE, "INTEGER",
+				  SQL_STATIC);
 	}
 
  update_cleanup:
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index 938aea9..0944f61 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -942,6 +942,37 @@ res.metadata
   - name: detail
     type: TEXT
 ...
+-- When pragma count_changes is on, statements INSERT, REPLACE and
+-- UPDATE returns number of changed columns. Make sure that this
+-- result has a column type.
+cn:execute("PRAGMA count_changes = 1;")
+---
+- rowcount: 0
+...
+cn:execute("INSERT INTO t1 VALUES (1), (2), (3);")
+---
+- metadata:
+  - name: rows inserted
+    type: INTEGER
+  rows:
+  - [3]
+...
+cn:execute("REPLACE INTO t1 VALUES (2), (3), (4), (5);")
+---
+- metadata:
+  - name: rows replaced
+    type: INTEGER
+  rows:
+  - [4]
+...
+cn:execute("UPDATE t1 SET id = id + 100 WHERE id > 10;")
+---
+- metadata:
+  - name: rows updated
+    type: INTEGER
+  rows:
+  - [0]
+...
 cn:close()
 ---
 ...
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index fbdc5a2..3b36cc3 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -289,6 +289,14 @@ res.metadata
 res = cn:execute("EXPLAIN QUERY PLAN SELECT COUNT(*) FROM t1")
 res.metadata
 
+-- When pragma count_changes is on, statements INSERT, REPLACE and
+-- UPDATE returns number of changed columns. Make sure that this
+-- result has a column type.
+cn:execute("PRAGMA count_changes = 1;")
+cn:execute("INSERT INTO t1 VALUES (1), (2), (3);")
+cn:execute("REPLACE INTO t1 VALUES (2), (3), (4), (5);")
+cn:execute("UPDATE t1 SET id = id + 100 WHERE id > 10;")
+
 cn:close()
 box.sql.execute('DROP TABLE t1')