Code review. Remove quotation of floats for PostgreSQL.

Removed quotation of floating point values after code review by Nyall Dawson.
qgis · Jun 17, 2020 · 9469f24 · 9469f24
1 parent 6a85309
commit 9469f24
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 90 deletions.
diff --git a/src/providers/postgres/qgspostgresconn.cpp b/src/providers/postgres/qgspostgresconn.cpp
@@ -1212,6 +1212,7 @@ QString QgsPostgresConn::quotedValue( const QVariant &value )
   {
     case QVariant::Int:
     case QVariant::LongLong:
+    case QVariant::Double:
       return value.toString();
 
     case QVariant::DateTime:
@@ -1227,8 +1228,6 @@ QString QgsPostgresConn::quotedValue( const QVariant &value )
     case QVariant::List:
       return quotedList( value.toList() );
 
-    // we need to pass floating point values quoted to the DBMS
-    case QVariant::Double:
     case QVariant::String:
     default:
       return quotedString( value.toString() );

diff --git a/tests/src/providers/testqgspostgresprovider.cpp b/tests/src/providers/testqgspostgresprovider.cpp
@@ -252,7 +252,7 @@ void TestQgsPostgresProvider::testQuotedValueBigInt()
   sdata->clear();
   sdata->insertFid( 1LL, vlst );
 
-  QCOMPARE( QgsPostgresUtils::whereClause( 1LL, fields, NULL, QgsPostgresPrimaryKeyType::PktFidMap, pkAttrs, std::shared_ptr<QgsPostgresSharedData>( sdata ) ), QString( "\"fld_double\"='3.141592741'" ) );
+  QCOMPARE( QgsPostgresUtils::whereClause( 1LL, fields, NULL, QgsPostgresPrimaryKeyType::PktFidMap, pkAttrs, std::shared_ptr<QgsPostgresSharedData>( sdata ) ), QString( "\"fld_double\"=3.141592741" ) );
 
   // text
   f3.setName( "fld_text" );

diff --git a/tests/src/python/test_provider_postgres.py b/tests/src/python/test_provider_postgres.py
@@ -776,20 +776,16 @@ def testPktComposite(self):
         """
         Check that tables with PKs composed of many fields of different types are correctly read and written to
         """
-        vl = QgsVectorLayer('{} sslmode=disable srid=4326 key=\'"pk1","pk2","pk3"\' table="qgis_test"."tb_test_compound_pk" (geom)'.format(self.dbconn), "test_compound", "postgres")
+        vl = QgsVectorLayer('{} sslmode=disable srid=4326 key=\'"pk1","pk2"\' table="qgis_test"."tb_test_compound_pk" (geom)'.format(self.dbconn), "test_compound", "postgres")
         self.assertTrue(vl.isValid())
 
         fields = vl.fields()
 
-        # Only 6 decimals for PostgreSQL 11.
-        f = next(vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk3 = 3.14159')))
+        f = next(vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk1 = 1 AND pk2 = 2')))
         # first of all: we must be able to fetch a valid feature
         self.assertTrue(f.isValid())
         self.assertEqual(f['pk1'], 1)
         self.assertEqual(f['pk2'], 2)
-
-        # Only 6 decimals for PostgreSQL 11.
-        self.assertEqual(f['pk3'], 3.14159)
         self.assertEqual(f['value'], 'test 2')
 
         # can we edit a field?
@@ -798,23 +794,18 @@ def testPktComposite(self):
         self.assertTrue(vl.commitChanges())
 
         # Did we get it right? Let's create a new QgsVectorLayer and try to read back our changes:
-        vl2 = QgsVectorLayer('{} sslmode=disable srid=4326 table="qgis_test"."tb_test_compound_pk" (geom) key=\'"pk1","pk2","pk3"\' '.format(self.dbconn), "test_compound2", "postgres")
+        vl2 = QgsVectorLayer('{} sslmode=disable srid=4326 table="qgis_test"."tb_test_compound_pk" (geom) key=\'"pk1","pk2"\' '.format(self.dbconn), "test_compound2", "postgres")
         self.assertTrue(vl2.isValid())
-        f2 = next(vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk3 = 3.14159')))
+        f2 = next(vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk1 = 1 AND pk2 = 2')))
         self.assertTrue(f2.isValid())
 
-        # just making sure we have the correct feature
-        # Only 6 decimals for PostgreSQL 11.
-        self.assertEqual(f2['pk3'], 3.14159)
-
         # Then, making sure we really did change our value.
         self.assertEqual(f2['value'], 'Edited Test 2')
 
         # How about inserting a new field?
         f3 = QgsFeature(vl2.fields())
         f3['pk1'] = 4
         f3['pk2'] = -9223372036854775800
-        f3['pk3'] = 7.29154
         f3['value'] = 'other test'
         vl.startEditing()
         res, f3 = vl.dataProvider().addFeatures([f3])
@@ -825,79 +816,17 @@ def testPktComposite(self):
         f4 = next(vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk2 = -9223372036854775800')))
 
         self.assertTrue(f4.isValid())
-        expected_attrs = [4, -9223372036854775800, 7.29154, 'other test']
+        expected_attrs = [4, -9223372036854775800, 'other test']
         self.assertEqual(f4.attributes(), expected_attrs)
 
         # Finally, let's delete one of the features.
-        f5 = next(vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk3 = 7.29154')))
-        vl2.startEditing()
-        vl2.deleteFeatures([f5.id()])
-        self.assertTrue(vl2.commitChanges())
-
-        # did we really delete?
-        f_iterator = vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk3 = 7.29154'))
-        got_feature = True
-
-        try:
-            f6 = next(f_iterator)
-            got_feature = f6.isValid()
-        except StopIteration:
-            got_feature = False
-
-        self.assertFalse(got_feature)
-
-    def testPktFloat(self):
-        """
-        Verify that we handle tables with floating point PKs well.
-        """
-        vl = QgsVectorLayer('{} sslmode=disable srid=4326 key="pk" table="qgis_test"."tb_test_float_pk" (geom)'.format(self.dbconn), "test_floatpk", "postgres")
-        self.assertTrue(vl.isValid())
-        fields = vl.fields()
-        # floating point madness: PostgreSQL 11 only outputs 6 decimal digits for real type by default
-        # Thus, QGIS only stores 6 decimal digits in its attributes. For this reason,
-        # we must insert only 3.14159 instead of the more accurate 3.141592741 in the test table.
-        f = next(vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 3.14159')))
-        self.assertTrue(f.isValid())
-
-        self.assertEqual(f['pk'], 3.14159)
-        self.assertEqual(f['value'], 'first test')
-
-        # editing
-        vl.startEditing()
-        vl.changeAttributeValue(f.id(), fields.indexOf('value'), 'first check')
-        self.assertTrue(vl.commitChanges())
-
-        # checking out if we really wrote to the table
-        vl2 = QgsVectorLayer('{} sslmode=disable srid=4326 key="pk" table="qgis_test"."tb_test_float_pk" (geom)'.format(self.dbconn), "test_floatpk2", "postgres")
-        self.assertTrue(vl2.isValid())
-        f2 = next(vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 3.14159')))
-
-        self.assertEqual(f2['value'], 'first check')
-
-        # inserting new...
-        f3 = QgsFeature(vl2.fields())
-        f3['pk'] = 7.29154
-        f3['value'] = 'newly inserted'
-        vl2.startEditing()
-        res, f3 = vl.dataProvider().addFeatures([f3])
-        self.assertTrue(res)
-        self.assertTrue(vl2.commitChanges())
-
-        # checking if correctly inserted...
-        f4 = next(vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 7.29154')))
-        self.assertTrue(f4.isValid())
-        self.assertEqual(f4['value'], 'newly inserted')
-
-        # Checking deletion
-        # same as above: no more than 6 digits of Euler's number in the testdata for PostgreSQL 11.
-        f5 = next(vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 2.71828')))
-        self.assertTrue(f5.isValid())
+        f5 = next(vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk1 = 2 AND pk2 = 1')))
         vl2.startEditing()
         vl2.deleteFeatures([f5.id()])
         self.assertTrue(vl2.commitChanges())
 
-        # did we really delete?
-        f_iterator = vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 2.71828'))
+        # did we really delete? Let's try to get the deleted feature from the first layer.
+        f_iterator = vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk1 = 2 AND pk2 = 1'))
         got_feature = True
 
         try:

diff --git a/tests/testdata/provider/testdata_pg_bigint_pk.sql b/tests/testdata/provider/testdata_pg_bigint_pk.sql
@@ -108,17 +108,16 @@ CREATE TABLE qgis_test.tb_test_compound_pk
 (
     pk1 INTEGER,
     pk2 BIGINT,
-    pk3 REAL,
     value VARCHAR(16),
     geom geometry(Point, 4326),
-    PRIMARY KEY (pk1, pk2, pk3)
+    PRIMARY KEY (pk1, pk2)
 );
 
-INSERT INTO qgis_test.tb_test_compound_pk (pk1, pk2, pk3, value, geom) VALUES
-    (1, 1, 1.0, 'test 1', ST_SetSRID(ST_Point(-47.930, -15.818), 4326)),
-    (1, 2, 3.14159, 'test 2', ST_SetSRID(ST_Point(-47.887, -15.864), 4326)),
-    (2, 2, 2.718281828, 'test 3', ST_SetSRID(ST_Point(-47.902, -15.763), 4326)),
-    (2, 2, 1.0, 'test 4', ST_SetSRID(ST_Point(-47.952, -15.781), 4326));
+INSERT INTO qgis_test.tb_test_compound_pk (pk1, pk2, value, geom) VALUES
+    (1, 1, 'test 1', ST_SetSRID(ST_Point(-47.930, -15.818), 4326)),
+    (1, 2, 'test 2', ST_SetSRID(ST_Point(-47.887, -15.864), 4326)),
+    (2, 1, 'test 3', ST_SetSRID(ST_Point(-47.902, -15.763), 4326)),
+    (2, 2, 'test 4', ST_SetSRID(ST_Point(-47.952, -15.781), 4326));
 
 CREATE TABLE qgis_test.tb_test_float_pk
 (
@@ -128,5 +127,5 @@ CREATE TABLE qgis_test.tb_test_float_pk
 );
 
 INSERT INTO qgis_test.tb_test_float_pk (pk, value, geom) VALUES
-    (3.14159, 'first test', ST_SetSRID(ST_Point(-47.887, -15.864), 4326)),
+    (3.14159, 'first test',  ST_SetSRID(ST_Point(-47.887, -15.864), 4326)),
     (2.71828, 'second test', ST_SetSRID(ST_Point(-47.902, -15.763), 4326));