Merge 'Improve changes() and total_changes() functions and add tests'…

… from Ben Li

#144
- The previous `changes()` function returns 1 no matter what
- Modified `changes()` and `total_changes()` to be closer to the sqlite
implementation
    - Store local `n_change` counter in the `Program` struct
    - Update `last_change` and `total_changes` values in `Connection`
struct on halt
    - Add `change_cnt_on` flag to `Program` struct that is `true` for
insert and delete (also need later on for update)
- Added TCL tests

Closes #749

COMPAT.md

@@ -112,7 +112,7 @@ Feature support of [sqlite expr syntax](https://www.sqlite.org/lang_expr.html).
 | Function                     | Status | Comment |
 |------------------------------|--------|---------|
 | abs(X)                       | Yes    |         |
-| changes()                    | No     |         |
+| changes()                    | Partial| Still need to support update statements and triggers |
 | char(X1,X2,...,XN)           | Yes    |         |
 | coalesce(X,Y,...)            | Yes    |         |
 | concat(X,...)                | Yes    |         |
@@ -158,7 +158,7 @@ Feature support of [sqlite expr syntax](https://www.sqlite.org/lang_expr.html).
 | substr(X,Y)                  | Yes    |         |
 | substring(X,Y,Z)             | Yes    |         |
 | substring(X,Y)               | Yes    |         |
-| total_changes()              | No     |         |
+| total_changes()              | Partial| Still need to support update statements and triggers |
 | trim(X)                      | Yes    |         |
 | trim(X,Y)                    | Yes    |         |
 | typeof(X)                    | Yes    |         |

core/lib.rs

@@ -422,6 +422,12 @@ impl Connection {
     fn update_last_rowid(&self, rowid: u64) {
         self.last_insert_rowid.set(rowid);
     }
+
+    pub fn set_changes(&self, nchange: i64) {
+        self.last_change.set(nchange);
+        let prev_total_changes = self.total_changes.get();
+        self.total_changes.set(prev_total_changes + nchange);
+    }
 }
 
 pub struct Statement {

core/translate/mod.rs

@@ -48,6 +48,7 @@ pub fn translate(
     syms: &SymbolTable,
 ) -> Result<Program> {
     let mut program = ProgramBuilder::new();
+    let mut change_cnt_on = false;
 
     match stmt {
         ast::Stmt::AlterTable(_, _) => bail_parse_error!("ALTER TABLE not supported yet"),
@@ -79,6 +80,7 @@ pub fn translate(
             limit,
             ..
         } => {
+            change_cnt_on = true;
             translate_delete(&mut program, schema, &tbl_name, where_clause, limit, syms)?;
         }
         ast::Stmt::Detach(_) => bail_parse_error!("DETACH not supported yet"),
@@ -106,6 +108,7 @@ pub fn translate(
             body,
             returning,
         } => {
+            change_cnt_on = true;
             translate_insert(
                 &mut program,
                 schema,
@@ -120,7 +123,7 @@ pub fn translate(
         }
     }
 
-    Ok(program.build(database_header, connection))
+    Ok(program.build(database_header, connection, change_cnt_on))
 }
 
 /* Example:

core/vdbe/builder.rs

@@ -1,5 +1,5 @@
 use std::{
-    cell::RefCell,
+    cell::{Cell, RefCell},
     collections::HashMap,
     rc::{Rc, Weak},
 };
@@ -327,6 +327,7 @@ impl ProgramBuilder {
         mut self,
         database_header: Rc<RefCell<DatabaseHeader>>,
         connection: Weak<Connection>,
+        change_cnt_on: bool,
     ) -> Program {
         self.resolve_labels();
         assert!(
@@ -343,6 +344,8 @@ impl ProgramBuilder {
             connection,
             auto_commit: true,
             parameters: self.parameters,
+            n_change: Cell::new(0),
+            change_cnt_on,
         }
     }
 }

core/vdbe/mod.rs

@@ -55,7 +55,7 @@ use rand::{thread_rng, Rng};
 use regex::{Regex, RegexBuilder};
 use sorter::Sorter;
 use std::borrow::BorrowMut;
-use std::cell::RefCell;
+use std::cell::{Cell, RefCell};
 use std::collections::{BTreeMap, HashMap};
 use std::num::NonZero;
 use std::rc::{Rc, Weak};
@@ -282,6 +282,8 @@ pub struct Program {
     pub parameters: crate::parameters::Parameters,
     pub connection: Weak<Connection>,
     pub auto_commit: bool,
+    pub n_change: Cell<i64>,
+    pub change_cnt_on: bool,
 }
 
 impl Program {
@@ -892,11 +894,23 @@ impl Program {
                     return if self.auto_commit {
                         match pager.end_tx() {
                             Ok(crate::storage::wal::CheckpointStatus::IO) => Ok(StepResult::IO),
-                            Ok(crate::storage::wal::CheckpointStatus::Done) => Ok(StepResult::Done),
+                            Ok(crate::storage::wal::CheckpointStatus::Done) => {
+                                if self.change_cnt_on {
+                                    if let Some(conn) = self.connection.upgrade() {
+                                        conn.set_changes(self.n_change.get());
+                                    }
+                                }
+                                Ok(StepResult::Done)
+                            }
                             Err(e) => Err(e),
                         }
                     } else {
-                        Ok(StepResult::Done)
+                        if self.change_cnt_on {
+                            if let Some(conn) = self.connection.upgrade() {
+                                conn.set_changes(self.n_change.get());
+                            }
+                        }
+                        return Ok(StepResult::Done);
                     };
                 }
                 Insn::Transaction { write } => {
@@ -2076,10 +2090,9 @@ impl Program {
                         if let Some(rowid) = cursor.rowid()? {
                             if let Some(conn) = self.connection.upgrade() {
                                 conn.update_last_rowid(rowid);
-                                let prev_total_changes = conn.total_changes.get();
-                                conn.last_change.set(1);
-                                conn.total_changes.set(prev_total_changes + 1);
                             }
+                            let prev_changes = self.n_change.get();
+                            self.n_change.set(prev_changes + 1);
                         }
                     }
                     state.pc += 1;
@@ -2092,6 +2105,8 @@ impl Program {
                 Insn::DeleteAwait { cursor_id } => {
                     let cursor = btree_table_cursors.get_mut(cursor_id).unwrap();
                     cursor.wait_for_completion()?;
+                    let prev_changes = self.n_change.get();
+                    self.n_change.set(prev_changes + 1);
                     state.pc += 1;
                 }
                 Insn::NewRowid {

testing/all.test

@@ -20,3 +20,5 @@ source $testdir/select.test
 source $testdir/subquery.test
 source $testdir/where.test
 source $testdir/compare.test
+source $testdir/changes.test
+source $testdir/total-changes.test

testing/changes.test

@@ -0,0 +1,23 @@
+#!/usr/bin/env tclsh
+
+set testdir [file dirname $argv0]
+source $testdir/tester.tcl
+
+do_execsql_test_on_specific_db {:memory:} changes-on-basic-insert {
+    create table temp (t1 integer, primary key (t1));
+    insert into temp values (1);
+    select changes();
+} {1}
+
+do_execsql_test_on_specific_db {:memory:} changes-on-multiple-row-insert {
+    create table temp (t1 integer, primary key (t1));
+    insert into temp values (1), (2), (3);
+    select changes();
+} {3}
+
+do_execsql_test_on_specific_db {:memory:} changes-shows-most-recent {
+    create table temp (t1 integer, primary key (t1));
+    insert into temp values (1), (2), (3);
+    insert into temp values (4), (5), (6), (7);
+    select changes();
+} {4}

testing/total-changes.test

@@ -0,0 +1,23 @@
+#!/usr/bin/env tclsh
+
+set testdir [file dirname $argv0]
+source $testdir/tester.tcl
+
+do_execsql_test_on_specific_db {:memory:} total-changes-on-basic-insert {
+    create table temp (t1 integer, primary key (t1));
+    insert into temp values (1);
+    select total_changes();
+} {1}
+
+do_execsql_test_on_specific_db {:memory:} total-changes-on-multiple-row-insert {
+    create table temp (t1 integer, primary key (t1));
+    insert into temp values (1), (2), (3);
+    select total_changes();
+} {3}
+
+do_execsql_test_on_specific_db {:memory:} total-changes-on-multiple-inserts {
+    create table temp (t1 integer, primary key (t1));
+    insert into temp values (1), (2), (3);
+    insert into temp values (4), (5), (6), (7);
+    select total_changes();
+} {7}