[postgres] correct timestamp comparison in where clause

fix #24163
qgis · Jun 10, 2020 · 7120e5d · 7120e5d
1 parent f1e3f82
commit 7120e5d
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 3 deletions.
diff --git a/src/providers/postgres/qgspostgresconn.cpp b/src/providers/postgres/qgspostgresconn.cpp
@@ -1215,6 +1215,9 @@ QString QgsPostgresConn::quotedValue( const QVariant &value )
     case QVariant::Double:
       return value.toString();
 
+    case QVariant::DateTime:
+      return quotedString( value.toDateTime().toString( Qt::ISODateWithMs ) );
+
     case QVariant::Bool:
       return value.toBool() ? "TRUE" : "FALSE";
 
@@ -1598,6 +1601,42 @@ qint64 QgsPostgresConn::getBinaryInt( QgsPostgresResult &queryResult, int row, i
   return oid;
 }
 
+QString QgsPostgresConn::fieldExpressionForWhereClause( const QgsField &fld, QVariant::Type valueType, QString expr )
+{
+  QString out;
+  const QString &type = fld.typeName();
+
+  if ( type == QLatin1String( "timestamp" ) || type == QLatin1String( "time" ) || type == QLatin1String( "date" ) )
+  {
+    out = expr.arg( quotedIdentifier( fld.name() ) );
+    // if field and value havev incompatible types, rollback to text cast
+    if ( valueType !=  QVariant::LastType && valueType != QVariant::DateTime && valueType != QVariant::Date && valueType != QVariant::Time )
+    {
+      out = out + "::text";
+    }
+  }
+
+  else if ( type == QLatin1String( "int8" ) || type == QLatin1String( "serial8" ) //
+            || type == QLatin1String( "int2" ) || type == QLatin1String( "int4" ) || type == QLatin1String( "oid" ) || type == QLatin1String( "serial" ) //
+            || type == QLatin1String( "real" ) || type == QLatin1String( "double precision" ) || type == QLatin1String( "float4" ) || type == QLatin1String( "float8" ) //
+            || type == QLatin1String( "numeric" ) )
+  {
+    out = expr.arg( quotedIdentifier( fld.name() ) );
+    // if field and value havev incompatible types, rollback to text cast
+    if ( valueType !=  QVariant::LastType && valueType != QVariant::Int && valueType != QVariant::LongLong && valueType != QVariant::Double )
+    {
+      out = out + "::text";
+    }
+  }
+
+  else
+  {
+    out = fieldExpression( fld, expr ); // same as fieldExpression by default
+  }
+
+  return out;
+}
+
 QString QgsPostgresConn::fieldExpression( const QgsField &fld, QString expr )
 {
   const QString &type = fld.typeName();

diff --git a/src/providers/postgres/qgspostgresconn.h b/src/providers/postgres/qgspostgresconn.h
@@ -339,6 +339,8 @@ class QgsPostgresConn : public QObject
 
     qint64 getBinaryInt( QgsPostgresResult &queryResult, int row, int col );
 
+    QString fieldExpressionForWhereClause( const QgsField &fld, QVariant::Type valueType = QVariant::LastType, QString expr = "%1" );
+
     QString fieldExpression( const QgsField &fld, QString expr = "%1" );
 
     QString connInfo() const { return mConnInfo; }

diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp
@@ -469,7 +469,7 @@ QString QgsPostgresProvider::pkParamWhereClause( int offset, const char *alias )
         int idx = mPrimaryKeyAttrs[i];
         QgsField fld = field( idx );
 
-        whereClause += delim + QStringLiteral( "%3%1=$%2" ).arg( connectionRO()->fieldExpression( fld ) ).arg( offset++ ).arg( aliased );
+        whereClause += delim + QStringLiteral( "%3%1=$%2" ).arg( connectionRO()->fieldExpressionForWhereClause( fld ) ).arg( offset++ ).arg( aliased );
         delim = QStringLiteral( " AND " );
       }
     }
@@ -604,11 +604,11 @@ QString QgsPostgresUtils::whereClause( QgsFeatureId featureId, const QgsFields &
           int idx = pkAttrs[i];
           QgsField fld = fields.at( idx );
 
-          whereClause += delim + conn->fieldExpression( fld );
+          whereClause += delim + conn->fieldExpressionForWhereClause( fld, pkVals[i].type() );
           if ( pkVals[i].isNull() )
             whereClause += QLatin1String( " IS NULL" );
           else
-            whereClause += '=' + QgsPostgresConn::quotedValue( pkVals[i].toString() );
+            whereClause += '=' + QgsPostgresConn::quotedValue( pkVals[i] ); // remove toString as it must be handled by quotedValue function
 
           delim = QStringLiteral( " AND " );
         }

diff --git a/tests/src/providers/testqgspostgresconn.cpp b/tests/src/providers/testqgspostgresconn.cpp
@@ -16,6 +16,8 @@
 #include <QObject>
 
 #include <qgspostgresconn.h>
+#include <qgsfields.h>
+#include <qgspostgresprovider.h>
 
 class TestQgsPostgresConn: public QObject
 {
@@ -38,6 +40,35 @@ class TestQgsPostgresConn: public QObject
       QCOMPARE( QgsPostgresConn::quotedValue( "b \"c' \\x" ), QString( "E'b \"c'' \\\\x'" ) );
     }
 
+    void quotedValueDatetime()
+    {
+      QCOMPARE( QgsPostgresConn::quotedValue( QDateTime::fromString( "2020-05-07 17:56:00", "yyyy-MM-dd HH:mm:ss" ) ), QString( "'2020-05-07T17:56:00.000'" ) );
+
+      QgsFields fields;
+      QgsField f;
+      f.setName( "ts" );
+      f.setType( QVariant::DateTime );
+      f.setTypeName( "timestamp" );
+      fields.append( f );
+      QgsField f2;
+      f2.setName( "pk" );
+      f2.setType( QVariant::LongLong );
+      f2.setTypeName( "serial8" );
+      fields.append( f2 );
+
+      QList<int> pkAttrs;
+      pkAttrs.append( 0 );
+      pkAttrs.append( 1 );
+
+      QgsPostgresSharedData *shared = new QgsPostgresSharedData;
+      QVariantList qvlist;
+      qvlist.append( QDateTime::fromString( "2020-05-07 17:56:00", "yyyy-MM-dd HH:mm:ss" ) );
+      qvlist.append( 123LL );
+      shared->insertFid( 1LL, qvlist );
+
+      QCOMPARE( QgsPostgresUtils::whereClause( 1LL, fields, NULL, QgsPostgresPrimaryKeyType::PktFidMap, pkAttrs, std::shared_ptr<QgsPostgresSharedData>( shared ) ), QString( "\"ts\"='2020-05-07T17:56:00.000' AND \"pk\"=123" ) );
+    }
+
     void quotedValueStringArray()
     {
       QStringList list;