Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Quoting floating point values on PostgreSQL provider.
This quotes floating point values on the PostgreSQL data provider, which
allows such values to be used either as stand-alone primary keys or as
components of composite primary keys on underlying tables. The
PostgreSQL session parameter extra_float_digits is explicitly set to 3
(the default on PostgreSQL 11 and older; in PostgreSQL 12 this defaults
to 1), to keep compatibility with all database versions and to avoid
round-trip errors.

In this PR, NUMERIC/DECIMAL values are no longer cast to
QVariant::Double in order to avoid loss of precision.
  • Loading branch information
espinafre committed Jun 23, 2020
1 parent e4718d7 commit 0da7184
Show file tree
Hide file tree
Showing 5 changed files with 295 additions and 16 deletions.
3 changes: 2 additions & 1 deletion src/providers/postgres/qgspostgresconn.cpp
Expand Up @@ -361,6 +361,7 @@ QgsPostgresConn::QgsPostgresConn( const QString &conninfo, bool readOnly, bool s
if ( mPostgresqlVersion >= 90000 )
{
PQexecNR( QStringLiteral( "SET application_name='QGIS'" ) );
PQexecNR( QStringLiteral( "SET extra_float_digits=3" ) );
}

PQsetNoticeProcessor( mConn, noticeProcessor, nullptr );
Expand Down Expand Up @@ -1212,7 +1213,6 @@ QString QgsPostgresConn::quotedValue( const QVariant &value )
{
case QVariant::Int:
case QVariant::LongLong:
case QVariant::Double:
return value.toString();

case QVariant::DateTime:
Expand All @@ -1228,6 +1228,7 @@ QString QgsPostgresConn::quotedValue( const QVariant &value )
case QVariant::List:
return quotedList( value.toList() );

case QVariant::Double:
case QVariant::String:
default:
return quotedString( value.toString() );
Expand Down
12 changes: 2 additions & 10 deletions src/providers/postgres/qgspostgresprovider.cpp
Expand Up @@ -994,7 +994,7 @@ bool QgsPostgresProvider::loadFields()
}
else if ( fieldTypeName == QLatin1String( "numeric" ) )
{
fieldType = QVariant::Double;
fieldType = QVariant::String;

if ( formattedFieldType == QLatin1String( "numeric" ) || formattedFieldType.isEmpty() )
{
Expand Down Expand Up @@ -4134,15 +4134,7 @@ bool QgsPostgresProvider::convertField( QgsField &field, const QMap<QString, QVa
}

case QVariant::Double:
if ( fieldSize > 18 )
{
fieldType = QStringLiteral( "numeric" );
fieldSize = -1;
}
else
{
fieldType = QStringLiteral( "float8" );
}
fieldType = QStringLiteral( "float8" );
fieldPrec = -1;
break;

Expand Down
2 changes: 1 addition & 1 deletion tests/src/providers/testqgspostgresprovider.cpp
Expand Up @@ -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" );
Expand Down
243 changes: 242 additions & 1 deletion tests/src/python/test_provider_postgres.py
Expand Up @@ -837,6 +837,246 @@ def testPktComposite(self):

self.assertFalse(got_feature)

def testPktCompositeFloat(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_composite_float_pk" (geom)'.format(self.dbconn), "test_composite_float", "postgres")
self.assertTrue(vl.isValid())

fields = vl.fields()

f = next(vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk3 = 3.14159274')))
# 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)

self.assertEqual(round(f['pk3'], 6), round(3.14159274, 6))
self.assertEqual(f['value'], 'test 2')

# can we edit a field?
vl.startEditing()
vl.changeAttributeValue(f.id(), fields.indexOf('value'), 'Edited Test 2')
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 key=\'"pk1","pk2","pk3"\' table="qgis_test"."tb_test_composite_float_pk" (geom)'.format(self.dbconn), "test_composite_float2", "postgres")
self.assertTrue(vl2.isValid())
f2 = next(vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk3 = 3.14159274')))
self.assertTrue(f2.isValid())

# just making sure we have the correct feature
# Only 6 decimals for PostgreSQL 11.
self.assertEqual(round(f2['pk3'], 6), round(3.14159274, 6))

# 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])
self.assertTrue(res)
self.assertTrue(vl.commitChanges())

# can we catch it on another layer?
f4 = next(vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk2 = -9223372036854775800')))

self.assertTrue(f4.isValid())
expected_attrs = [4, -9223372036854775800, 7.29154, 'other test']
gotten_attrs = [f4['pk1'], f4['pk2'], round(f4['pk3'], 5), f4['value']]
self.assertEqual(gotten_attrs, 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 testPktFloatingPoint(self):
"""
Check if we can handle floating point/numeric primary keys correctly
"""
# 1. 32 bit float (PostgreSQL "REAL" type)
vl = QgsVectorLayer(self.dbconn + ' sslmode=disable srid=4326 key="pk" table="qgis_test"."tb_test_float_pk" (geom)', "test_float_pk", "postgres")
self.assertTrue(vl.isValid())

# 1.1. Retrieving
f = next(vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 3.141592653589793238462643383279502884197169')))
self.assertTrue(f.isValid())
self.assertEqual(f['value'], 'first teste')
# 1.2. Editing
self.assertTrue(vl.startEditing())
vl.changeAttributeValue(f.id(), vl.fields().indexOf('value'), 'Changed first')
self.assertTrue(vl.commitChanges())
# 1.2.1. Checking edit from another vector layer
vl2 = QgsVectorLayer(self.dbconn + ' sslmode=disable srid=4326 key="pk1" table="qgis_test"."tb_test_float_pk" (geom)', "test_float_pk2", "postgres")
self.assertTrue(vl2.isValid())
f2 = next(vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 3.141592653589793238462643383279502884197169')))
self.assertTrue(f2.isValid())
self.assertEqual(f2['value'], 'Changed first')
# 1.3. Deleting
f = next(vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 2.718281828459045235360287471352662497757247')))
vl.startEditing()
vl.deleteFeatures([f.id()])
self.assertTrue(vl.commitChanges())
# 1.3.1. Checking deletion
f_iterator = vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 2.718281828459045235360287471352662497757247'))
got_feature = True

try:
f2 = next(f_iterator)
got_feature = f2.isValid()
except StopIteration:
got_feature = False
self.assertFalse(got_feature)
# 1.4. Inserting new feature
newpointwkt = 'Point(-47.751 -15.644)'
f = QgsFeature(vl.fields())
f['pk'] = 0.22222222222222222222222
f['value'] = 'newly inserted'
f.setGeometry(QgsGeometry.fromWkt(newpointwkt))
vl.startEditing()
res, f = vl.dataProvider().addFeatures([f])
self.assertTrue(res)
self.assertTrue(vl.commitChanges())
# 1.4.1. Checking insertion
f2 = next(vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 0.22222222222222222222222')))
self.assertTrue(f2.isValid())
self.assertEqual(round(f2['pk'], 6), round(0.2222222222222222, 6))
self.assertEqual(f2['value'], 'newly inserted')
assert compareWkt(f2.geometry().asWkt(), newpointwkt), "Geometry mismatch. Expected: {} Got: {} \n".format(f2.geometry().asWkt(), newpointwkt)
# One more check: can we retrieve the same row with the value that we got from this layer?
floatpk = f2['pk']
f3 = next(vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk = {}'.format(floatpk) )))
self.assertTrue(f3.isValid())
self.assertEqual(f3['value'], 'newly inserted')
self.assertEqual(f3['pk'], floatpk)

# 2. 64 bit float (PostgreSQL "DOUBLE PRECISION" type)
vl = QgsVectorLayer(self.dbconn + ' sslmode=disable srid=4326 key="pk" table="qgis_test"."tb_test_double_pk" (geom)', "test_double_pk", "postgres")
self.assertTrue(vl.isValid())

# 2.1. Retrieving
f = next(vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 3.141592653589793238462643383279502884197169')))
self.assertTrue(f.isValid())
self.assertEqual(f['value'], 'first teste')
# 2.2. Editing
self.assertTrue(vl.startEditing())
vl.changeAttributeValue(f.id(), vl.fields().indexOf('value'), 'Changed first')
self.assertTrue(vl.commitChanges())
# 2.2.1. Checking edit from another vector layer
vl2 = QgsVectorLayer(self.dbconn + ' sslmode=disable srid=4326 key="pk" table="qgis_test"."tb_test_double_pk" (geom)', "test_double_pk2", "postgres")
self.assertTrue(vl2.isValid())
f2 = next(vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 3.141592653589793238462643383279502884197169')))
self.assertTrue(f2.isValid())
self.assertEqual(f2['value'], 'Changed first')
# 2.3. Deleting
f = next(vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 2.718281828459045235360287471352662497757247')))
vl.startEditing()
vl.deleteFeatures([f.id()])
self.assertTrue(vl.commitChanges())
# 2.3.1. Checking deletion
f_iterator = vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 2.718281828459045235360287471352662497757247'))
got_feature = True

try:
f2 = next(f_iterator)
got_feature = f2.isValid()
except StopIteration:
got_feature = False
self.assertFalse(got_feature)
# 2.4. Inserting new feature
newpointwkt = 'Point(-47.751 -15.644)'
f = QgsFeature(vl.fields())
f['pk'] = 0.22222222222222222222222
f['value'] = 'newly inserted'
f.setGeometry(QgsGeometry.fromWkt(newpointwkt))
vl.startEditing()
res, f = vl.dataProvider().addFeatures([f])
self.assertTrue(res)
self.assertTrue(vl.commitChanges())
# 2.4.1. Checking insertion
f2 = next(vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 0.22222222222222222222222')))
self.assertTrue(f2.isValid())
self.assertEqual(round(f2['pk'], 15), round(0.2222222222222222, 15))
self.assertEqual(f2['value'], 'newly inserted')
assert compareWkt(f2.geometry().asWkt(), newpointwkt), "Geometry mismatch. Expected: {} Got: {} \n".format(f2.geometry().asWkt(), newpointwkt)
# One more check: can we retrieve the same row with the value that we got from this layer?
doublepk = f2['pk']
f3 = next(vl.getFeatures(QgsFeatureRequest().setFilterExpression('pk = {}'.format(doublepk) )))
self.assertTrue(f3.isValid())
self.assertEqual(f3['value'], 'newly inserted')
self.assertEqual(f3['pk'], doublepk)

# 3. Arbitrary precision (exact real types). PostgreSQL NUMERIC/DECIMAL type.
vl = QgsVectorLayer(self.dbconn + ' sslmode=disable srid=4326 key="pk" table="qgis_test"."tb_test_numeric_pk" (geom)', "test_numeric_pk", "postgres")
self.assertTrue(vl.isValid())

# 3.1. Retrieving
f = next(vl.getFeatures(QgsFeatureRequest().setFilterExpression("pk = '3.1415926535897932384626433832795028841972'")))
self.assertTrue(f.isValid())
self.assertEqual(f['value'], 'first teste')
# 3.2. Editing
self.assertTrue(vl.startEditing())
vl.changeAttributeValue(f.id(), vl.fields().indexOf('value'), 'Changed first')
self.assertTrue(vl.commitChanges())
# 3.2.1. Checking edit from another vector layer
vl2 = QgsVectorLayer(self.dbconn + ' sslmode=disable srid=4326 key="pk" table="qgis_test"."tb_test_numeric_pk" (geom)', "test_numeric_pk2", "postgres")
self.assertTrue(vl2.isValid())
f2 = next(vl2.getFeatures(QgsFeatureRequest().setFilterExpression("pk = '3.1415926535897932384626433832795028841972'")))
self.assertTrue(f2.isValid())
self.assertEqual(f2['value'], 'Changed first')
# 3.3. Deleting
f = next(vl.getFeatures(QgsFeatureRequest().setFilterExpression("pk = '2.7182818284590452353602874713526624977572'")))
vl.startEditing()
vl.deleteFeatures([f.id()])
self.assertTrue(vl.commitChanges())
# 3.3.1. Checking deletion
f_iterator = vl2.getFeatures(QgsFeatureRequest().setFilterExpression("pk = '2.7182818284590452353602874713526624977572'"))
got_feature = True

try:
f2 = next(f_iterator)
got_feature = f2.isValid()
except StopIteration:
got_feature = False
self.assertFalse(got_feature)
# 3.4. Inserting new feature
newpointwkt = 'Point(-47.751 -15.644)'
f = QgsFeature(vl.fields())
f['pk'] = 0.22222222222222222222222
f['value'] = 'newly inserted'
f.setGeometry(QgsGeometry.fromWkt(newpointwkt))
vl.startEditing()
res, f = vl.dataProvider().addFeatures([f])
self.assertTrue(res)
self.assertTrue(vl.commitChanges())
# 3.4.1. Checking insertion
f2 = next(vl2.getFeatures(QgsFeatureRequest().setFilterExpression('pk = 0.22222222222222222222222')))
self.assertTrue(f2.isValid())
# PostgreSQL adds trailing zeros to NUMERIC values on output to pad the field up to the scale length
self.assertEqual(f2['pk'], '0.2222222222222222000000000000000000000000')
self.assertEqual(f2['value'], 'newly inserted')
assert compareWkt(f2.geometry().asWkt(), newpointwkt), "Geometry mismatch. Expected: {} Got: {} \n".format(f2.geometry().asWkt(), newpointwkt)

def testPktMapInsert(self):
vl = QgsVectorLayer('{} table="qgis_test"."{}" key="obj_id" sql='.format(self.dbconn, 'oid_serial_table'),
"oid_serial", "postgres")
Expand Down Expand Up @@ -1060,7 +1300,8 @@ def testDomainTypes(self):
'type': QVariant.String, 'typeName': 'qgis_test.char_domain_6', 'length': 6}
expected['fld_text_domain'] = {
'type': QVariant.String, 'typeName': 'qgis_test.text_domain', 'length': -1}
expected['fld_numeric_domain'] = {'type': QVariant.Double, 'typeName': 'qgis_test.numeric_domain', 'length': 10,
# Treating arbitrary precision PostgreSQL numbers (types numeric and decimal) as strings
expected['fld_numeric_domain'] = {'type': QVariant.String, 'typeName': 'qgis_test.numeric_domain', 'length': 10,
'precision': 4}

for f, e in list(expected.items()):
Expand Down
51 changes: 48 additions & 3 deletions tests/testdata/provider/testdata_pg_bigint_pk.sql
Expand Up @@ -119,13 +119,58 @@ INSERT INTO qgis_test.tb_test_compound_pk (pk1, pk2, value, geom) VALUES
(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_composite_float_pk
(
pk1 INTEGER,
pk2 BIGINT,
pk3 REAL,
value VARCHAR(16),
geom geometry(Point, 4326),
PRIMARY KEY (pk1, pk2, pk3)
);

INSERT INTO qgis_test.tb_test_composite_float_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.141592741, '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));

CREATE TABLE qgis_test.tb_test_float_pk
(
pk REAL PRIMARY KEY,
value VARCHAR(16),
geom geometry(Point, 4326)
);

-- those values (pi, Euler's, and a third) will be truncated/rounded to fit
-- PostgreSQL's internal type size. REAL is IEEE-754 4 bytes (32 bit).
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)),
(2.71828, 'second test', ST_SetSRID(ST_Point(-47.902, -15.763), 4326));
(3.141592653589793238462643383279502884197169399375105820974944592307816406286, 'first teste', ST_SetSRID(ST_Point(-47.887, -15.864), 4326)),
(2.718281828459045235360287471352662497757247093699959574966967627724076630353, 'second test', ST_SetSRID(ST_Point(-47.902, -15.763), 4326)),
(1.333333333333333333333333333333333333333333333333333333333333333333333333333, 'third teste', ST_SetSRID(ST_Point(-47.751, -15.644), 4326));

CREATE TABLE qgis_test.tb_test_double_pk
(
pk DOUBLE PRECISION PRIMARY KEY,
value VARCHAR(16),
geom geometry(Point, 4326)
);
-- those values (pi, Euler's, and a third) will be truncated/rounded to fit
-- PostgreSQL's internal type size. DOUBLE PRECISION is IEEE-754 8 bytes (64 bit).
INSERT INTO qgis_test.tb_test_double_pk (pk, value, geom) VALUES
(3.141592653589793238462643383279502884197169399375105820974944592307816406286, 'first teste', ST_SetSRID(ST_Point(-47.887, -15.864), 4326)),
(2.718281828459045235360287471352662497757247093699959574966967627724076630353, 'second test', ST_SetSRID(ST_Point(-47.902, -15.763), 4326)),
(1.333333333333333333333333333333333333333333333333333333333333333333333333333, 'third teste', ST_SetSRID(ST_Point(-47.751, -15.644), 4326));

CREATE TABLE qgis_test.tb_test_numeric_pk
(
pk NUMERIC(41, 40) PRIMARY KEY,
value VARCHAR(16),
geom geometry(Point, 4326)
);
-- those values (pi, Euler's, and a third) will be truncated to fit
-- PostgreSQL's internal type size. The last digit will be rounded.
INSERT INTO qgis_test.tb_test_numeric_pk (pk, value, geom) VALUES
(3.1415926535897932384626433832795028841972, 'first teste', ST_SetSRID(ST_Point(-47.887, -15.864), 4326)),
(2.7182818284590452353602874713526624977572, 'second test', ST_SetSRID(ST_Point(-47.902, -15.763), 4326)),
(1.3333333333333333333333333333333333333333, 'third teste', ST_SetSRID(ST_Point(-47.751, -15.644), 4326));

0 comments on commit 0da7184

Please sign in to comment.