SQLResultSet.rowsAffected not cleared
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Oct 2012 12:37:57 +0000 (12:37 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Oct 2012 12:37:57 +0000 (12:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=46070

Patch by Christophe Dumez <christophe.dumez@intel.com> on 2012-10-10
Reviewed by Kenneth Rohde Christiansen.

Source/WebCore:

SQLResultSet.rowsAffected is supposed to return the number
of rows that were changed by the statement. For "SELECT"
statements, it should return 0.

However, our implementation currently relies on sqlite3_changes()
to compute this value. sqlite3_changes() returns the number of
direct row changes in the most recent INSERT, UPDATE, or DELETE
statement within the same trigger context. Unfortunately, the
most recent INSERT, UPDATE, or DELETE statement may not be the
last statement. As a consequence, if you INSERT 1 row, then
do a SELECT, SQLResultSet.rowsAffected will be 1 for both the
INSERT and the SELECT statements.

The proposed solution is to use sqlite3_total_changes() instead
of sqlite3_changes(). sqlite3_total_changes() returns the number
of row changes caused by INSERT, UPDATE or DELETE statements since
the database connection was opened. We now store the value
returned by sqlite3_total_changes() before each statement in
order to return the count difference in
SQLiteDatabase::lastChanges().

Test: storage/websql/execute-sql-rowsAffected.html

* platform/sql/SQLiteDatabase.cpp:
(WebCore::SQLiteDatabase::SQLiteDatabase):
(WebCore::SQLiteDatabase::updateLastChangesCount):
(WebCore):
(WebCore::SQLiteDatabase::lastChanges):
* platform/sql/SQLiteDatabase.h:
(SQLiteDatabase):
* platform/sql/SQLiteStatement.cpp:
(WebCore::SQLiteStatement::step):

LayoutTests:

Add layout test to check that SQLResultSet.rowsAffected is
correct in executeSql() success callback.

* storage/websql/execute-sql-rowsAffected-expected.txt: Added.
* storage/websql/execute-sql-rowsAffected.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@130891 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/storage/websql/execute-sql-rowsAffected-expected.txt [new file with mode: 0644]
LayoutTests/storage/websql/execute-sql-rowsAffected.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/sql/SQLiteDatabase.cpp
Source/WebCore/platform/sql/SQLiteDatabase.h
Source/WebCore/platform/sql/SQLiteStatement.cpp

index 1d2bb03..6cfe19d 100644 (file)
@@ -1,3 +1,16 @@
+2012-10-10  Christophe Dumez  <christophe.dumez@intel.com>
+
+        SQLResultSet.rowsAffected not cleared
+        https://bugs.webkit.org/show_bug.cgi?id=46070
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        Add layout test to check that SQLResultSet.rowsAffected is
+        correct in executeSql() success callback.
+
+        * storage/websql/execute-sql-rowsAffected-expected.txt: Added.
+        * storage/websql/execute-sql-rowsAffected.html: Added.
+
 2012-10-10  KwangYong Choi  <ky0.choi@samsung.com>
 
         [EFL] Rebaseline after r129972 which enabled plugin feature
diff --git a/LayoutTests/storage/websql/execute-sql-rowsAffected-expected.txt b/LayoutTests/storage/websql/execute-sql-rowsAffected-expected.txt
new file mode 100644 (file)
index 0000000..630b089
--- /dev/null
@@ -0,0 +1,16 @@
+This test tests that SQLResultSet.rowsAffected attribute is correct in success callback for executeSql().
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS rowsAffected is 1
+PASS rowsAffected is 1
+PASS rowsAffected is 2
+PASS rowsAffected is 0
+PASS rowsAffected is 2
+PASS rowsAffected is 0
+PASS rowsAffected is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/websql/execute-sql-rowsAffected.html b/LayoutTests/storage/websql/execute-sql-rowsAffected.html
new file mode 100644 (file)
index 0000000..24df946
--- /dev/null
@@ -0,0 +1,45 @@
+<html>
+<head>
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<script>
+description("This test tests that SQLResultSet.rowsAffected attribute is correct in success callback for executeSql().");
+
+function errorCallback(transaction, error)
+{
+    testFailed("Database error code: " + error.code + ", message: " + error.message);
+}
+
+function rowsAffectedShouldBe(resultSet, expected)
+{
+    rowsAffected = resultSet.rowsAffected;
+    shouldBe("rowsAffected", expected);
+}
+
+function runTest()
+{
+    if (window.testRunner)
+        testRunner.clearAllDatabases();
+
+    db = openDatabase("RowsAffectedTest", "1.0", "", 1);
+    db.transaction(function (t) {
+        t.executeSql("CREATE TABLE IF NOT EXISTS RowsAffectedTest (Foo INT, text TEXT)");
+        t.executeSql("INSERT INTO RowsAffectedTest VALUES (1, 'a')", null, function(t, r) { rowsAffectedShouldBe(r, "1"); }, errorCallback);
+        t.executeSql("INSERT INTO RowsAffectedTest VALUES (2, 'b')", null, function(t, r) { rowsAffectedShouldBe(r, "1"); }, errorCallback);
+        t.executeSql("UPDATE RowsAffectedTest SET text = 'c'", null, function(t, r) { rowsAffectedShouldBe(r, "2"); }, errorCallback);
+        t.executeSql("SELECT * FROM RowsAffectedTest", null, function(t, r) { rowsAffectedShouldBe(r, "0"); }, errorCallback);
+        t.executeSql("DELETE FROM RowsAffectedTest", null, function(t, r) { rowsAffectedShouldBe(r, "2"); }, errorCallback);
+        t.executeSql("DELETE FROM RowsAffectedTest", null, function(t, r) { rowsAffectedShouldBe(r, "0"); }, errorCallback);
+        t.executeSql("SELECT * FROM RowsAffectedTest", null, function(t, r) { rowsAffectedShouldBe(r, "0"); }, errorCallback);
+    }, function(error) { errorCallback(null, error); finishJSTest(); }, finishJSTest);
+}
+
+runTest();
+window.jsTestIsAsync = true;
+</script>
+
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</body>
+</html>
index c68cebf..3ddcf3e 100644 (file)
@@ -1,3 +1,43 @@
+2012-10-10  Christophe Dumez  <christophe.dumez@intel.com>
+
+        SQLResultSet.rowsAffected not cleared
+        https://bugs.webkit.org/show_bug.cgi?id=46070
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        SQLResultSet.rowsAffected is supposed to return the number
+        of rows that were changed by the statement. For "SELECT"
+        statements, it should return 0.
+
+        However, our implementation currently relies on sqlite3_changes()
+        to compute this value. sqlite3_changes() returns the number of
+        direct row changes in the most recent INSERT, UPDATE, or DELETE
+        statement within the same trigger context. Unfortunately, the
+        most recent INSERT, UPDATE, or DELETE statement may not be the
+        last statement. As a consequence, if you INSERT 1 row, then
+        do a SELECT, SQLResultSet.rowsAffected will be 1 for both the
+        INSERT and the SELECT statements.
+
+        The proposed solution is to use sqlite3_total_changes() instead
+        of sqlite3_changes(). sqlite3_total_changes() returns the number
+        of row changes caused by INSERT, UPDATE or DELETE statements since
+        the database connection was opened. We now store the value
+        returned by sqlite3_total_changes() before each statement in
+        order to return the count difference in
+        SQLiteDatabase::lastChanges().
+
+        Test: storage/websql/execute-sql-rowsAffected.html
+
+        * platform/sql/SQLiteDatabase.cpp:
+        (WebCore::SQLiteDatabase::SQLiteDatabase):
+        (WebCore::SQLiteDatabase::updateLastChangesCount):
+        (WebCore):
+        (WebCore::SQLiteDatabase::lastChanges):
+        * platform/sql/SQLiteDatabase.h:
+        (SQLiteDatabase):
+        * platform/sql/SQLiteStatement.cpp:
+        (WebCore::SQLiteStatement::step):
+
 2012-10-10  Keishi Hattori  <keishi@webkit.org>
 
         REGRESSION (r129738): Calendar picker is too wide when the input is rtl
index 68d124f..2418fc1 100644 (file)
@@ -58,6 +58,7 @@ SQLiteDatabase::SQLiteDatabase()
     , m_interrupted(false)
     , m_openError(SQLITE_ERROR)
     , m_openErrorMessage()
+    , m_lastChangesCount(0)
 {
 }
 
@@ -321,11 +322,20 @@ int64_t SQLiteDatabase::lastInsertRowID()
     return sqlite3_last_insert_rowid(m_db);
 }
 
+void SQLiteDatabase::updateLastChangesCount()
+{
+    if (!m_db)
+        return;
+
+    m_lastChangesCount = sqlite3_total_changes(m_db);
+}
+
 int SQLiteDatabase::lastChanges()
 {
     if (!m_db)
         return 0;
-    return sqlite3_changes(m_db);
+
+    return sqlite3_total_changes(m_db) - m_lastChangesCount;
 }
 
 int SQLiteDatabase::lastError()
index 5a66d49..f15508c 100644 (file)
@@ -65,6 +65,8 @@ public:
     void interrupt();
     bool isInterrupted();
 
+    void updateLastChangesCount();
+
     bool executeCommand(const String&);
     bool returnsAtLeastOneResult(const String&);
     
@@ -158,6 +160,8 @@ private:
 
     int m_openError;
     CString m_openErrorMessage;
+
+    int m_lastChangesCount;
 };
 
 } // namespace WebCore
index d308872..3c7e949 100644 (file)
@@ -101,6 +101,11 @@ int SQLiteStatement::step()
 
     if (!m_statement)
         return SQLITE_OK;
+
+    // The database needs to update its last changes count before each statement
+    // in order to compute properly the lastChanges() return value.
+    m_database.updateLastChangesCount();
+
     LOG(SQLDatabase, "SQL - step - %s", m_query.ascii().data());
     int error = sqlite3_step(m_statement);
     if (error != SQLITE_DONE && error != SQLITE_ROW) {