QSqlQuery: make it a move only type

QSqlQuery is a broken value class. Copying one object would mean
copying database state (the result set, the cursor position, etc.)
which isn't generally available for all database drivers.
For that reason, the current implementation does not honor value
semantics -- modifying a QSqlQuery object has visible side effects
on its existing copies (!).

The correct solution is to accept that QSqlQuery is a move only
type, not a value type. Add move semantics to it, and deprecate
its copies.

(We can't just *remove* copies in Qt 6 due to SC/BC constraints).

[ChangeLog][QtSql][QSqlQuery] QSqlQuery copy operations have
been deprecated. QSqlQuery copy semantics cannot be implemented
correctly, as it's not generally possible to copy a result set
of a query when copying the corresponding QSqlQuery object. This
resulted in modifications on a QSqlQuery having visible (and
unintended) side effects on its copies. Instead, treat QSqlQuery
as a move-only type.

Fixes: QTBUG-91766
Change-Id: Iabd3aa605332a5c15c524303418bf17a21ed520b
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Andy Shaw <andy.shaw@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

2 files changed, 63 insertions, 14 deletions

diff --git a/src/sql/kernel/qsqlquery.cpp b/src/sql/kernel/qsqlquery.cpp
index d51d596d14..b78566e828 100644
--- a/src/sql/kernel/qsqlquery.cpp
+++ b/src/sql/kernel/qsqlquery.cpp
@@ -243,12 +243,18 @@ QSqlQuery::QSqlQuery(QSqlResult *result)
 
 QSqlQuery::~QSqlQuery()
 {
-    if (!d->ref.deref())
+    if (d && !d->ref.deref())
         delete d;
 }
 
+#if QT_DEPRECATED_SINCE(6, 2)
 /*!
     Constructs a copy of \a other.
+
+    \obsolete QSqlQuery cannot be meaningfully copied. Prepared
+    statements, bound values and so on will not work correctly, depending
+    on your database driver (for instance, changing the copy will affect
+    the original). Treat QSqlQuery as a move-only type instead.
 */
 
 QSqlQuery::QSqlQuery(const QSqlQuery& other)
@@ -258,6 +264,41 @@ QSqlQuery::QSqlQuery(const QSqlQuery& other)
 }
 
 /*!
+    Assigns \a other to this object.
+
+    \obsolete QSqlQuery cannot be meaningfully copied. Prepared
+    statements, bound values and so on will not work correctly, depending
+    on your database driver (for instance, changing the copy will affect
+    the original). Treat QSqlQuery as a move-only type instead.
+*/
+
+QSqlQuery& QSqlQuery::operator=(const QSqlQuery& other)
+{
+    qAtomicAssign(d, other.d);
+    return *this;
+}
+#endif
+
+/*!
+    \fn QSqlQuery::QSqlQuery(QSqlQuery &&other) noexcept
+    \since 6.2
+    Move-constructs a QSqlQuery from \a other.
+*/
+
+/*!
+    \fn QSqlQuery &QSqlQuery::operator=(QSqlQuery &&other) noexcept
+    \since 6.2
+    Move-assigns \a other to this object.
+*/
+
+/*!
+    \fn void QSqlQuery::swap(QSqlQuery &other) noexcept
+    \since 6.2
+    Swaps \a other to this object. This operation is very
+    fast and never fails.
+*/
+
+/*!
     \internal
 */
 static void qInit(QSqlQuery *q, const QString& query, const QSqlDatabase &db)
@@ -299,17 +340,6 @@ QSqlQuery::QSqlQuery(const QSqlDatabase &db)
     qInit(this, QString(), db);
 }
 
-
-/*!
-    Assigns \a other to this object.
-*/
-
-QSqlQuery& QSqlQuery::operator=(const QSqlQuery& other)
-{
-    qAtomicAssign(d, other.d);
-    return *this;
-}
-
 /*!
     Returns \c true if the query is not \l{isActive()}{active},
     the query is not positioned on a valid record,
diff --git a/src/sql/kernel/qsqlquery.h b/src/sql/kernel/qsqlquery.h
index b270441273..b34452c302 100644
--- a/src/sql/kernel/qsqlquery.h
+++ b/src/sql/kernel/qsqlquery.h
@@ -61,10 +61,29 @@ public:
     explicit QSqlQuery(QSqlResult *r);
     explicit QSqlQuery(const QString& query = QString(), const QSqlDatabase &db = QSqlDatabase());
     explicit QSqlQuery(const QSqlDatabase &db);
-    QSqlQuery(const QSqlQuery& other);
-    QSqlQuery& operator=(const QSqlQuery& other);
+
+#if QT_DEPRECATED_SINCE(6, 2)
+    QT_DEPRECATED_VERSION_X_6_2("QSqlQuery is not meant to be copied. Use move construction instead.")
+    QSqlQuery(const QSqlQuery &other);
+    QT_DEPRECATED_VERSION_X_6_2("QSqlQuery is not meant to be copied. Use move assignment instead.")
+    QSqlQuery& operator=(const QSqlQuery &other);
+#else
+    QSqlQuery(const QSqlQuery &other) = delete;
+    QSqlQuery& operator=(const QSqlQuery &other) = delete;
+#endif
+
+    QSqlQuery(QSqlQuery &&other) noexcept
+        : d(std::exchange(other.d, nullptr))
+    {}
+    QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_MOVE_AND_SWAP(QSqlQuery)
+
     ~QSqlQuery();
 
+    void swap(QSqlQuery &other) noexcept
+    {
+        qSwap(d, other.d);
+    }
+
     bool isValid() const;
     bool isActive() const;
     bool isNull(int field) const;