Skip to content

Commit ec62a0b

Browse files
cdumezcdumez
committed
Drop unnecessary SQLiteDatabase::updateLastChangesCount()
https://bugs.webkit.org/show_bug.cgi?id=225885 Reviewed by Alex Christensen. Drop unnecessary SQLiteDatabase::updateLastChangesCount() and have SQLiteDatabase::lastChanges() rely on sqlite3_changes() instead of sqlite3_total_changes(). We started using updateLastChangesCount() + sqlite3_total_changes() in https://commits.webkit.org/r130891 to address an issue in WebSQL SQLResultSet.rowsAffected would not be 0 for SELECT statement. This patch reverts r130891 and instead sets SQLResultSet.rowsAffected when the statement is not a read-only statement. This is covered by storage/websql/execute-sql-rowsAffected.html which is still passing after this change. * Modules/webdatabase/SQLStatement.cpp: (WebCore::SQLStatement::execute): * platform/sql/SQLiteDatabase.cpp: (WebCore::SQLiteDatabase::lastChanges): (WebCore::SQLiteDatabase::updateLastChangesCount): Deleted. * platform/sql/SQLiteDatabase.h: * platform/sql/SQLiteStatement.cpp: (WebCore::SQLiteStatement::step): (WebCore::SQLiteStatement::isReadOnly): * platform/sql/SQLiteStatement.h: Canonical link: https://commits.webkit.org/237833@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@277618 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 93226f6 commit ec62a0b

6 files changed

Lines changed: 41 additions & 21 deletions

File tree

‎Source/WebCore/ChangeLog‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,30 @@
1+
2021-05-17 Chris Dumez <cdumez@apple.com>
2+
3+
Drop unnecessary SQLiteDatabase::updateLastChangesCount()
4+
https://bugs.webkit.org/show_bug.cgi?id=225885
5+
6+
Reviewed by Alex Christensen.
7+
8+
Drop unnecessary SQLiteDatabase::updateLastChangesCount() and have SQLiteDatabase::lastChanges()
9+
rely on sqlite3_changes() instead of sqlite3_total_changes(). We started using updateLastChangesCount()
10+
+ sqlite3_total_changes() in https://commits.webkit.org/r130891 to address an issue in WebSQL
11+
SQLResultSet.rowsAffected would not be 0 for SELECT statement. This patch reverts r130891 and instead
12+
sets SQLResultSet.rowsAffected when the statement is not a read-only statement.
13+
14+
This is covered by storage/websql/execute-sql-rowsAffected.html which is still passing after
15+
this change.
16+
17+
* Modules/webdatabase/SQLStatement.cpp:
18+
(WebCore::SQLStatement::execute):
19+
* platform/sql/SQLiteDatabase.cpp:
20+
(WebCore::SQLiteDatabase::lastChanges):
21+
(WebCore::SQLiteDatabase::updateLastChangesCount): Deleted.
22+
* platform/sql/SQLiteDatabase.h:
23+
* platform/sql/SQLiteStatement.cpp:
24+
(WebCore::SQLiteStatement::step):
25+
(WebCore::SQLiteStatement::isReadOnly):
26+
* platform/sql/SQLiteStatement.h:
27+
128
2021-05-17 Alex Christensen <achristensen@webkit.org>
229

330
Fix Windows debug build.

‎Source/WebCore/Modules/webdatabase/SQLStatement.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,10 +187,10 @@ bool SQLStatement::execute(Database& db)
187187
return false;
188188
}
189189

190-
// FIXME: If the spec allows triggers, and we want to be "accurate" in a different way, we'd use
191-
// sqlite3_total_changes() here instead of sqlite3_changed, because that includes rows modified from within a trigger
192-
// For now, this seems sufficient
193-
resultSet->setRowsAffected(database.lastChanges());
190+
// rowsAffected should be 0 for read only statements (e.g. SELECT statement). However, SQLiteDatabase::lastChanges() returns
191+
// the number of changes made by the most recent INSERT, UPDATE or DELETE statement.
192+
if (!statement->isReadOnly())
193+
resultSet->setRowsAffected(database.lastChanges());
194194

195195
m_resultSet = WTFMove(resultSet);
196196
return true;

‎Source/WebCore/platform/sql/SQLiteDatabase.cpp‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -445,20 +445,12 @@ int64_t SQLiteDatabase::lastInsertRowID()
445445
return sqlite3_last_insert_rowid(m_db);
446446
}
447447

448-
void SQLiteDatabase::updateLastChangesCount()
449-
{
450-
if (!m_db)
451-
return;
452-
453-
m_lastChangesCount = sqlite3_total_changes(m_db);
454-
}
455-
456448
int SQLiteDatabase::lastChanges()
457449
{
458450
if (!m_db)
459451
return 0;
460452

461-
return sqlite3_total_changes(m_db) - m_lastChangesCount;
453+
return sqlite3_changes(m_db);
462454
}
463455

464456
int SQLiteDatabase::lastError()

‎Source/WebCore/platform/sql/SQLiteDatabase.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@ class SQLiteDatabase {
6060
bool isOpen() const { return m_db; }
6161
WEBCORE_EXPORT void close();
6262

63-
WEBCORE_EXPORT void updateLastChangesCount();
64-
6563
WEBCORE_EXPORT bool executeCommandSlow(const String&);
6664
WEBCORE_EXPORT bool executeCommand(ASCIILiteral);
6765

@@ -81,6 +79,8 @@ class SQLiteDatabase {
8179
WEBCORE_EXPORT void interrupt();
8280

8381
int64_t lastInsertRowID();
82+
83+
// This function returns the number of rows modified, inserted or deleted by the most recently completed INSERT, UPDATE or DELETE statement.
8484
WEBCORE_EXPORT int lastChanges();
8585

8686
void setBusyTimeout(int ms);
@@ -185,8 +185,6 @@ class SQLiteDatabase {
185185

186186
int m_openError { SQLITE_ERROR };
187187
CString m_openErrorMessage;
188-
189-
int m_lastChangesCount { 0 };
190188
};
191189

192190
} // namespace WebCore

‎Source/WebCore/platform/sql/SQLiteStatement.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,6 @@ int SQLiteStatement::step()
6363
{
6464
LockHolder databaseLock(m_database.databaseMutex());
6565

66-
// The database needs to update its last changes count before each statement
67-
// in order to compute properly the lastChanges() return value.
68-
m_database.updateLastChangesCount();
69-
7066
int error = sqlite3_step(m_statement);
7167
if (error != SQLITE_DONE && error != SQLITE_ROW)
7268
LOG(SQLDatabase, "sqlite3_step failed (%i)\nError - %s", error, sqlite3_errmsg(m_database.sqlite3Handle()));
@@ -431,4 +427,9 @@ bool SQLiteStatement::hasStartedStepping()
431427
return sqlite3_stmt_busy(m_statement);
432428
}
433429

430+
bool SQLiteStatement::isReadOnly()
431+
{
432+
return sqlite3_stmt_readonly(m_statement);
433+
}
434+
434435
} // namespace WebCore

‎Source/WebCore/platform/sql/SQLiteStatement.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ class SQLiteStatement {
6464
// Returns -1 on last-step failing. Otherwise, returns number of rows
6565
// returned in the last step()
6666
int columnCount();
67+
68+
bool isReadOnly();
6769

6870
WEBCORE_EXPORT bool isColumnNull(int col);
6971
WEBCORE_EXPORT bool isColumnDeclaredAsBlob(int col);

0 commit comments

Comments
 (0)