Allow custom display formats (#1720)

Allow user to define their own display formats. The SQL code is editable
and the combo box changes automatically to custom if the user edits one
of the predefined formats.

The display format (either custom or predefined) is validated with a 
case-insensitive regex so at least it contains a function applied to the column
name. 

Added a new callback for executeSQL following the model of sqlite3_exec.
Using this, the number of columns is got from a checking query execution. If
it is not one, the custom display format is also rejected.

Note that these validations might still be fooled in some unforeseen way, but
the users should know what they're doing.

See issue #573

src/ColumnDisplayFormatDialog.cpp

@@ -1,11 +1,16 @@
+#include <QMessageBox>
+
 #include "ColumnDisplayFormatDialog.h"
 #include "ui_ColumnDisplayFormatDialog.h"
 #include "sql/sqlitetypes.h"
+#include "sqlitedb.h"
 
-ColumnDisplayFormatDialog::ColumnDisplayFormatDialog(const QString& colname, QString current_format, QWidget* parent)
+ColumnDisplayFormatDialog::ColumnDisplayFormatDialog(DBBrowserDB& db, const sqlb::ObjectIdentifier& tableName, const QString& colname, QString current_format, QWidget* parent)
     : QDialog(parent),
       ui(new Ui::ColumnDisplayFormatDialog),
-      column_name(colname)
+      column_name(colname),
+      pdb(db),
+      curTable(tableName)
 {
     // Create UI
     ui->setupUi(this);
@@ -28,6 +33,9 @@ ColumnDisplayFormatDialog::ColumnDisplayFormatDialog(const QString& colname, QSt
     ui->comboDisplayFormat->insertSeparator(ui->comboDisplayFormat->count());
     ui->comboDisplayFormat->addItem(tr("Lower case"), "lower");
     ui->comboDisplayFormat->addItem(tr("Upper case"), "upper");
+    ui->comboDisplayFormat->insertSeparator(ui->comboDisplayFormat->count());
+    ui->comboDisplayFormat->addItem(tr("Custom"), "custom");
+
     ui->labelDisplayFormat->setText(ui->labelDisplayFormat->text().arg(column_name));
 
     formatFunctions["decimal"] = "printf('%d', " + sqlb::escapeIdentifier(column_name) + ")";
@@ -53,19 +61,14 @@ ColumnDisplayFormatDialog::ColumnDisplayFormatDialog(const QString& colname, QSt
         ui->comboDisplayFormat->setCurrentIndex(0);
         updateSqlCode();
     } else {
-        QString formatName;
+        // When it doesn't match any predefined format, it is considered custom
+        QString formatName = "custom";
         for(auto& formatKey : formatFunctions.keys()) {
             if(current_format == formatFunctions.value(formatKey)) {
                 formatName = formatKey;
                 break;
             }
         }
-
-        if(formatName.isEmpty()) {
-            ui->comboDisplayFormat->insertSeparator(ui->comboDisplayFormat->count());
-            ui->comboDisplayFormat->addItem(tr("Custom"), "custom");
-            formatName = "custom";
-        }
         ui->comboDisplayFormat->setCurrentIndex(ui->comboDisplayFormat->findData(formatName));
         ui->editDisplayFormat->setText(current_format);
     }
@@ -90,7 +93,46 @@ void ColumnDisplayFormatDialog::updateSqlCode()
 
     if(format == "default")
         ui->editDisplayFormat->setText(sqlb::escapeIdentifier(column_name));
-    else
+    else if(format != "custom")
         ui->editDisplayFormat->setText(formatFunctions.value(format));
-
+}
+
+void ColumnDisplayFormatDialog::accept()
+{
+    QString errorMessage;
+
+    // Accept the SQL code if it's the column name (default), it contains a function invocation applied to the column name and it can be
+    // executed without errors returning only one column.
+    // Users could still devise a way to break this, but this is considered good enough for letting them know about simple incorrect
+    // cases.
+    if(!(ui->editDisplayFormat->text() == sqlb::escapeIdentifier(column_name) ||
+         ui->editDisplayFormat->text().contains(QRegExp("[a-z]+[a-z_0-9]* *\\(.*" + QRegExp::escape(sqlb::escapeIdentifier(column_name)) + ".*\\)", Qt::CaseInsensitive))))
+        errorMessage = tr("Custom display format must contain a function call applied to %1").arg(sqlb::escapeIdentifier(column_name));
+    else {
+        // Execute a query using the display format and check that it only returns one column.
+        int customNumberColumns = 0;
+
+        DBBrowserDB::execCallback callback = [&customNumberColumns](int numberColumns, QStringList, QStringList) -> bool {
+            customNumberColumns = numberColumns;
+            // Return false so the query is not aborted and no error is reported.
+            return false;
+        };
+        if(!pdb.executeSQL(QString("SELECT %1 FROM %2 LIMIT 1").arg(ui->editDisplayFormat->text(), curTable.toString()),
+                           false, true, callback))
+            errorMessage = tr("Error in custom display format. Message from database engine:\n\n%1").arg(pdb.lastError());
+        else if(customNumberColumns != 1)
+            errorMessage = tr("Custom display format must return only one column but it returned %1.").arg(customNumberColumns);
+
+    }
+    if(!errorMessage.isEmpty())
+        QMessageBox::warning(this, QApplication::applicationName(), errorMessage);
+    else
+        QDialog::accept();
+}
+
+void ColumnDisplayFormatDialog::setCustom(bool modified)
+{
+    // If the SQL code is modified by user, select the custom value in the combo-box
+    if(modified && ui->editDisplayFormat->hasFocus())
+        ui->comboDisplayFormat->setCurrentIndex(ui->comboDisplayFormat->findData("custom"));
 }

src/ColumnDisplayFormatDialog.h

@@ -5,6 +5,10 @@
 #include <QString>
 #include <QMap>
 
+#include "sql/sqlitetypes.h"
+
+class DBBrowserDB;
+
 namespace Ui {
 class ColumnDisplayFormatDialog;
 }
@@ -14,18 +18,22 @@ class ColumnDisplayFormatDialog : public QDialog
     Q_OBJECT
 
 public:
-    explicit ColumnDisplayFormatDialog(const QString& colname, QString current_format, QWidget* parent = nullptr);
+    explicit ColumnDisplayFormatDialog(DBBrowserDB& db, const sqlb::ObjectIdentifier& tableName, const QString& colname, QString current_format, QWidget* parent = nullptr);
     ~ColumnDisplayFormatDialog() override;
 
     QString selectedDisplayFormat() const;
 
 private slots:
     void updateSqlCode();
+    void accept() override;
+    void setCustom(bool modified);
 
 private:
     Ui::ColumnDisplayFormatDialog* ui;
     QString column_name;
     QMap<QString, QString> formatFunctions;
+    DBBrowserDB& pdb;
+    sqlb::ObjectIdentifier curTable;
 };
 
 #endif

src/ColumnDisplayFormatDialog.ui

@@ -31,11 +31,7 @@
        <widget class="QComboBox" name="comboDisplayFormat"/>
       </item>
       <item>
-       <widget class="SqlTextEdit" name="editDisplayFormat">
-        <property name="readOnly">
-         <bool>true</bool>
-        </property>
-       </widget>
+       <widget class="SqlTextEdit" name="editDisplayFormat"/>
       </item>
      </layout>
     </widget>
@@ -114,8 +110,25 @@
     </hint>
    </hints>
   </connection>
+  <connection>
+   <sender>editDisplayFormat</sender>
+   <signal>modificationChanged(bool)</signal>
+   <receiver>ColumnDisplayFormatDialog</receiver>
+   <slot>setCustom(bool)</slot>
+   <hints>
+    <hint type="sourcelabel">
+     <x>125</x>
+     <y>69</y>
+    </hint>
+    <hint type="destinationlabel">
+     <x>244</x>
+     <y>4</y>
+    </hint>
+   </hints>
+  </connection>
  </connections>
  <slots>
   <slot>updateSqlCode()</slot>
+  <slot>setCustom()</slot>
  </slots>
 </ui>

src/MainWindow.cpp

@@ -3348,7 +3348,7 @@ void MainWindow::editDataColumnDisplayFormat()
     QString current_displayformat = browseTableSettings[current_table].displayFormats[field_number];
 
     // Open the dialog
-    ColumnDisplayFormatDialog dialog(field_name, current_displayformat, this);
+    ColumnDisplayFormatDialog dialog(db, current_table, field_name, current_displayformat, this);
     if(dialog.exec())
     {
         // Set the newly selected display format

src/sqlitedb.cpp

@@ -890,7 +890,23 @@ bool DBBrowserDB::dump(const QString& filePath,
     return false;
 }
 
-bool DBBrowserDB::executeSQL(QString statement, bool dirtyDB, bool logsql)
+// Callback for sqlite3_exec. It receives the user callback in the first parameter. Converts parameters
+// to C++ classes and calls user callback.
+int DBBrowserDB::callbackWrapper (void* callback, int numberColumns, char** values, char** columnNames)
+{
+    QStringList valuesList;
+    QStringList namesList;
+
+    for (int i=0; i<numberColumns; i++) {
+        valuesList << QString(values[i]);
+        namesList << QString(columnNames[i]);
+    }
+
+    execCallback userCallback = *(static_cast<execCallback*>(callback));
+    return userCallback(numberColumns, valuesList, namesList);
+}
+
+bool DBBrowserDB::executeSQL(QString statement, bool dirtyDB, bool logsql, execCallback callback)
 {
     waitForDbRelease();
     if(!_db)
@@ -905,7 +921,7 @@ bool DBBrowserDB::executeSQL(QString statement, bool dirtyDB, bool logsql)
     if (dirtyDB) setSavepoint();
 
     char* errmsg;
-    if (SQLITE_OK == sqlite3_exec(_db, statement.toUtf8(), nullptr, nullptr, &errmsg))
+    if (SQLITE_OK == sqlite3_exec(_db, statement.toUtf8(), callback ? callbackWrapper : nullptr, &callback, &errmsg))
     {
         // Update DB structure after executing an SQL statement. But try to avoid doing unnecessary updates.
         if(!dontCheckForStructureUpdates && (statement.startsWith("ALTER", Qt::CaseInsensitive) ||
@@ -919,6 +935,7 @@ bool DBBrowserDB::executeSQL(QString statement, bool dirtyDB, bool logsql)
         lastErrorMessage = QString("%1 (%2)").arg(QString::fromUtf8(errmsg)).arg(statement);
         qWarning() << "executeSQL: " << statement << "->" << errmsg;
         sqlite3_free(errmsg);
+
         return false;
     }
 }

src/sqlitedb.h

@@ -6,6 +6,7 @@
 #include <condition_variable>
 #include <memory>
 #include <mutex>
+#include <functional>
 
 #include <QByteArray>
 #include <QMultiMap>
@@ -54,6 +55,7 @@ private:
     };
 
 public:
+
     explicit DBBrowserDB () : _db(nullptr), db_used(false), isEncrypted(false), isReadOnly(false), dontCheckForStructureUpdates(false) {}
     ~DBBrowserDB () override {}
 
@@ -104,8 +106,17 @@ public:
         Wait,
         CancelOther
     };
-
-    bool executeSQL(QString statement, bool dirtyDB = true, bool logsql = true);
+    // Callback to get results from executeSQL(). It is invoked for
+    // each result row coming out of the evaluated SQL statements. If
+    // a callback returns true (abort), the executeSQL() method
+    // returns false (error) without invoking the callback again and
+    // without running any subsequent SQL statements. The 1st argument
+    // is the number of columns in the result. The 2nd argument to the
+    // callback is the text representation of the values, one for each
+    // column. The 3rd argument is a list of strings where each entry
+    // represents the name of corresponding result column.
+    typedef std::function<bool(int, QStringList, QStringList)> execCallback;
+    bool executeSQL(QString statement, bool dirtyDB = true, bool logsql = true, execCallback callback = nullptr);
     bool executeMultiSQL(QByteArray query, bool dirty = true, bool log = false);
     QByteArray querySingleValueFromDb(const QString& sql, bool log = true, ChoiceOnUse choice = Ask);
 
@@ -136,6 +147,8 @@ private:
      */
     QString max(const sqlb::ObjectIdentifier& tableName, const sqlb::Field& field) const;
 
+    static int callbackWrapper (void* callback, int numberColumns, char** values, char** columnNames);
+
 public:
     void updateSchema();