From 74e0df376a2c12093d3c9524cfa02752cda31481 Mon Sep 17 00:00:00 2001
From: Martin Kleusberg <mkleusberg@gmail.com>
Date: Thu, 1 Aug 2019 13:30:01 +0200
Subject: [PATCH] When updating a field try to respect the affinity data type
 if possible

When modifying the value of a field in the Browse Data tab, we used to
always hand over the new value to SQLite as text. This works most of the
time but can cause problems e.g. when a CHECK constraint checks the data
type of the value and expects something other than text. It is also a
pretty lazy approach in general.

This commit checks if the new value matches the affinity data type of
the column, and if it does hands over the new value as the correct data
type.

See issue #1952.
---
 src/sqlitedb.cpp         | 14 +++++++++-----
 src/sqlitedb.h           |  2 +-
 src/sqlitetablemodel.cpp | 21 ++++++++++++++++++++-
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/src/sqlitedb.cpp b/src/sqlitedb.cpp
index 4bd42fc8..e1fe1443 100644
--- a/src/sqlitedb.cpp
+++ b/src/sqlitedb.cpp
@@ -1398,7 +1398,7 @@ bool DBBrowserDB::deleteRecords(const sqlb::ObjectIdentifier& table, const QStri
 }
 
 bool DBBrowserDB::updateRecord(const sqlb::ObjectIdentifier& table, const std::string& column,
-                               const QString& rowid, const QByteArray& value, bool itsBlob, const sqlb::StringVector& pseudo_pk)
+                               const QString& rowid, const QByteArray& value, int force_type, const sqlb::StringVector& pseudo_pk)
 {
     waitForDbRelease();
     if (!isOpen()) return false;
@@ -1439,13 +1439,17 @@ bool DBBrowserDB::updateRecord(const sqlb::ObjectIdentifier& table, const std::s
     if(sqlite3_prepare_v2(_db, sql.toUtf8(), -1, &stmt, nullptr) != SQLITE_OK)
         success = 0;
     if(success == 1) {
-        if(itsBlob)
+        if(force_type == SQLITE_BLOB)
         {
             if(sqlite3_bind_blob(stmt, 1, rawValue, value.length(), SQLITE_STATIC))
                 success = -1;
-        }
-        else
-        {
+        } else if(force_type == SQLITE_INTEGER) {
+            if(sqlite3_bind_int(stmt, 1, value.toInt()))
+                success = -1;
+        } else if(force_type == SQLITE_FLOAT) {
+            if(sqlite3_bind_double(stmt, 1, value.toDouble()))
+                success = -1;
+        } else {
             if(sqlite3_bind_text(stmt, 1, rawValue, value.length(), SQLITE_STATIC))
                 success = -1;
         }
diff --git a/src/sqlitedb.h b/src/sqlitedb.h
index 4df1ab11..f6079456 100644
--- a/src/sqlitedb.h
+++ b/src/sqlitedb.h
@@ -173,7 +173,7 @@ private:
 public:
     QString addRecord(const sqlb::ObjectIdentifier& tablename);
     bool deleteRecords(const sqlb::ObjectIdentifier& table, const QStringList& rowids, const sqlb::StringVector& pseudo_pk = {});
-    bool updateRecord(const sqlb::ObjectIdentifier& table, const std::string& column, const QString& rowid, const QByteArray& value, bool itsBlob, const sqlb::StringVector& pseudo_pk = {});
+    bool updateRecord(const sqlb::ObjectIdentifier& table, const std::string& column, const QString& rowid, const QByteArray& value, int force_type = 0, const sqlb::StringVector& pseudo_pk = {});
 
     bool createTable(const sqlb::ObjectIdentifier& name, const sqlb::FieldVector& structure);
     bool renameTable(const std::string& schema, const std::string& from_table, const std::string& to_table);
diff --git a/src/sqlitetablemodel.cpp b/src/sqlitetablemodel.cpp
index 1502da55..86bc7203 100644
--- a/src/sqlitetablemodel.cpp
+++ b/src/sqlitetablemodel.cpp
@@ -466,7 +466,26 @@ bool SqliteTableModel::setTypedData(const QModelIndex& index, bool isBlob, const
         if(oldValue == newValue && oldValue.isNull() == newValue.isNull())
             return true;
 
-        if(m_db.updateRecord(m_query.table(), m_headers.at(column), cached_row.at(0), newValue, isBlob, m_query.rowIdColumns()))
+        // Determine type. If the BLOB flag is set, it's always BLOB. If the affinity data type of the modified column is something numeric,
+        // we check if the new value is also numeric. In that case we can safely set the data type to INTEGER or FLOAT. In all other cases we
+        // default to TEXT.
+        int type = SQLITE_TEXT;
+        if(isBlob)
+        {
+            type = SQLITE_BLOB;
+        } else if(m_vDataTypes.at(column) == SQLITE_INTEGER) {
+            bool ok;
+            newValue.toInt(&ok);
+            if(ok)
+                type = SQLITE_INTEGER;
+        } else if(m_vDataTypes.at(column) == SQLITE_FLOAT) {
+            bool ok;
+            newValue.toFloat(&ok);
+            if(ok)
+                type = SQLITE_FLOAT;
+        }
+
+        if(m_db.updateRecord(m_query.table(), m_headers.at(column), cached_row.at(0), newValue, type, m_query.rowIdColumns()))
         {
             cached_row[column] = newValue;