Skip to content

Commit 1ce09b8

Browse files
committedOct 14, 2016
Fix PostgreSQL import of layers with multi-column or quoted-column keys
Fixes #15226 (drag & drop of postgresql views) Includes test
1 parent 0ff8740 commit 1ce09b8

File tree

3 files changed

+171
-85
lines changed

3 files changed

+171
-85
lines changed
 

‎src/providers/postgres/qgspostgresprovider.cpp

Lines changed: 128 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,56 +1377,73 @@ bool QgsPostgresProvider::determinePrimaryKey()
13771377
return mValid;
13781378
}
13791379

1380-
void QgsPostgresProvider::determinePrimaryKeyFromUriKeyColumn()
1380+
/* static */
1381+
QStringList QgsPostgresProvider::parseUriKey( const QString& key )
13811382
{
1382-
QString primaryKey = mUri.keyColumn();
1383-
mPrimaryKeyType = pktUnknown;
1383+
if ( key.isEmpty() ) return QStringList();
13841384

1385-
if ( !primaryKey.isEmpty() )
1386-
{
1387-
QStringList cols;
1385+
QStringList cols;
13881386

1389-
// remove quotes from key list
1390-
if ( primaryKey.startsWith( '"' ) && primaryKey.endsWith( '"' ) )
1387+
// remove quotes from key list
1388+
if ( key.startsWith( '"' ) && key.endsWith( '"' ) )
1389+
{
1390+
int i = 1;
1391+
QString col;
1392+
while ( i < key.size() )
13911393
{
1392-
int i = 1;
1393-
QString col;
1394-
while ( i < primaryKey.size() )
1394+
if ( key[i] == '"' )
13951395
{
1396-
if ( primaryKey[i] == '"' )
1396+
if ( i + 1 < key.size() && key[i+1] == '"' )
13971397
{
1398-
if ( i + 1 < primaryKey.size() && primaryKey[i+1] == '"' )
1399-
{
1400-
i++;
1401-
}
1402-
else
1403-
{
1404-
cols << col;
1405-
col = "";
1398+
i++;
1399+
}
1400+
else
1401+
{
1402+
cols << col;
1403+
col = "";
14061404

1407-
if ( ++i == primaryKey.size() )
1408-
break;
1405+
if ( ++i == key.size() )
1406+
break;
14091407

1410-
Q_ASSERT( primaryKey[i] == ',' );
1411-
i++;
1412-
Q_ASSERT( primaryKey[i] == '"' );
1413-
i++;
1414-
col = "";
1415-
continue;
1416-
}
1408+
Q_ASSERT( key[i] == ',' );
1409+
i++;
1410+
Q_ASSERT( key[i] == '"' );
1411+
i++;
1412+
col = "";
1413+
continue;
14171414
}
1418-
1419-
col += primaryKey[i++];
14201415
}
1416+
1417+
col += key[i++];
14211418
}
1422-
else if ( primaryKey.contains( ',' ) )
1423-
{
1424-
cols = primaryKey.split( ',' );
1425-
}
1426-
else
1419+
}
1420+
else if ( key.contains( ',' ) )
1421+
{
1422+
cols = key.split( ',' );
1423+
}
1424+
else
1425+
{
1426+
cols << key;
1427+
}
1428+
1429+
return cols;
1430+
}
1431+
1432+
void QgsPostgresProvider::determinePrimaryKeyFromUriKeyColumn()
1433+
{
1434+
QString primaryKey = mUri.keyColumn();
1435+
mPrimaryKeyType = pktUnknown;
1436+
1437+
if ( !primaryKey.isEmpty() )
1438+
{
1439+
QStringList cols = parseUriKey( primaryKey );
1440+
1441+
primaryKey = "";
1442+
QString del = "";
1443+
Q_FOREACH ( const QString& col, cols )
14271444
{
1428-
cols << primaryKey;
1429-
primaryKey = quotedIdentifier( primaryKey );
1445+
primaryKey += del + quotedIdentifier( col );
1446+
del = ",";
14301447
}
14311448

14321449
Q_FOREACH ( const QString& col, cols )
@@ -3483,6 +3500,9 @@ QgsVectorLayerImport::ImportError QgsPostgresProvider::createEmptyLayer(
34833500
QString primaryKey = dsUri.keyColumn();
34843501
QString primaryKeyType;
34853502

3503+
QStringList pkList;
3504+
QStringList pkType;
3505+
34863506
QString schemaTableName = "";
34873507
if ( !schemaName.isEmpty() )
34883508
{
@@ -3523,45 +3543,39 @@ QgsVectorLayerImport::ImportError QgsPostgresProvider::createEmptyLayer(
35233543
}
35243544
else
35253545
{
3526-
// search for the passed field
3527-
for ( int fldIdx = 0; fldIdx < fields.count(); ++fldIdx )
3546+
pkList = parseUriKey( primaryKey );
3547+
Q_FOREACH ( const QString& col, pkList )
35283548
{
3529-
if ( fields[fldIdx].name() == primaryKey )
3549+
// search for the passed field
3550+
QString type;
3551+
for ( int fldIdx = 0; fldIdx < fields.count(); ++fldIdx )
35303552
{
3531-
// found, get the field type
3532-
QgsField fld = fields[fldIdx];
3533-
if ( convertField( fld, options ) )
3553+
if ( fields[fldIdx].name() == col )
35343554
{
3535-
primaryKeyType = fld.typeName();
3555+
// found, get the field type
3556+
QgsField fld = fields[fldIdx];
3557+
if ( convertField( fld, options ) )
3558+
{
3559+
type = fld.typeName();
3560+
break;
3561+
}
35363562
}
35373563
}
3538-
}
3539-
}
3540-
3541-
// if the pk field doesn't exist yet, create a serial pk field
3542-
// as it's autoincremental
3543-
if ( primaryKeyType.isEmpty() )
3544-
{
3545-
primaryKeyType = "serial";
3546-
#if 0
3547-
// TODO: check the feature count to choose if create a serial8 pk field
3548-
if ( layer->featureCount() > 0xffffffff )
3549-
{
3550-
primaryKeyType = "serial8";
3551-
}
3552-
#endif
3553-
}
3554-
else
3555-
{
3556-
// if the pk field's type is one of the postgres integer types,
3557-
// use the equivalent autoincremental type (serialN)
3558-
if ( primaryKeyType == "int2" || primaryKeyType == "int4" )
3559-
{
3560-
primaryKeyType = "serial";
3561-
}
3562-
else if ( primaryKeyType == "int8" )
3563-
{
3564-
primaryKeyType = "serial8";
3564+
if ( type.isEmpty() ) type = "serial";
3565+
else
3566+
{
3567+
// if the pk field's type is one of the postgres integer types,
3568+
// use the equivalent autoincremental type (serialN)
3569+
if ( primaryKeyType == "int2" || primaryKeyType == "int4" )
3570+
{
3571+
primaryKeyType = "serial";
3572+
}
3573+
else if ( primaryKeyType == "int8" )
3574+
{
3575+
primaryKeyType = "serial8";
3576+
}
3577+
}
3578+
pkType << type;
35653579
}
35663580
}
35673581

@@ -3597,17 +3611,32 @@ QgsVectorLayerImport::ImportError QgsPostgresProvider::createEmptyLayer(
35973611
throw PGException( result );
35983612
}
35993613

3600-
if ( options && options->value( "lowercaseFieldNames", false ).toBool() )
3614+
sql = QString( "CREATE TABLE %1(" ) .arg( schemaTableName );
3615+
QString pk;
3616+
for ( int i = 0; i < pkList.size(); ++i )
36013617
{
3602-
//convert primary key name to lowercase
3603-
//this must happen after determining the field type of the primary key
3604-
primaryKey = primaryKey.toLower();
3605-
}
3618+
QString col = pkList[i];
3619+
const QString& type = pkType[i];
36063620

3607-
sql = QString( "CREATE TABLE %1(%2 %3 PRIMARY KEY)" )
3608-
.arg( schemaTableName,
3609-
quotedIdentifier( primaryKey ),
3610-
primaryKeyType );
3621+
if ( options && options->value( "lowercaseFieldNames", false ).toBool() )
3622+
{
3623+
col = col.toLower();
3624+
}
3625+
else
3626+
{
3627+
col = quotedIdentifier( col ); // no need to quote lowercase field
3628+
}
3629+
3630+
if ( i )
3631+
{
3632+
pk += ",";
3633+
sql += ",";
3634+
}
3635+
3636+
pk += col;
3637+
sql += col + " " + type;
3638+
}
3639+
sql += QString( ", PRIMARY KEY (%1) )" ) .arg( pk );
36113640

36123641
result = conn->PQexec( sql );
36133642
if ( result.PQresultStatus() != PGRES_COMMAND_OK )
@@ -3698,9 +3727,25 @@ QgsVectorLayerImport::ImportError QgsPostgresProvider::createEmptyLayer(
36983727
fld.setName( fld.name().toLower() );
36993728
}
37003729

3701-
if ( fld.name() == primaryKey )
3730+
int pkIdx = -1;
3731+
for ( int i = 0; i < pkList.size(); ++i )
3732+
{
3733+
QString col = pkList[i];
3734+
if ( options && options->value( "lowercaseFieldNames", false ).toBool() )
3735+
{
3736+
//convert field name to lowercase (TODO: avoid doing this
3737+
//over and over)
3738+
col = col.toLower();
3739+
}
3740+
if ( fld.name() == col )
3741+
{
3742+
pkIdx = i;
3743+
break;
3744+
}
3745+
}
3746+
if ( pkIdx >= 0 )
37023747
{
3703-
oldToNewAttrIdxMap->insert( fldIdx, 0 );
3748+
oldToNewAttrIdxMap->insert( fldIdx, pkIdx );
37043749
continue;
37053750
}
37063751

‎src/providers/postgres/qgspostgresprovider.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,11 @@ class QgsPostgresProvider : public QgsVectorDataProvider
118118
*/
119119
static QString endianString();
120120

121+
/**
122+
* Returns a list of unquoted column names from an uri key
123+
*/
124+
static QStringList parseUriKey( const QString& key );
125+
121126
/**
122127
* Changes the stored extent for this layer to the supplied extent.
123128
* For example, this is called when the extent worker thread has a result.
@@ -132,11 +137,16 @@ class QgsPostgresProvider : public QgsVectorDataProvider
132137
*/
133138
virtual void updateExtents() override;
134139

135-
/** Determine the fields making up the primary key
140+
/**
141+
* Determine the fields making up the primary key
136142
*/
137143
bool determinePrimaryKey();
138144

139-
/** Determine the fields making up the primary key from the uri attribute keyColumn
145+
/**
146+
* Determine the fields making up the primary key from the uri attribute keyColumn
147+
*
148+
* Fills mPrimaryKeyType and mPrimaryKeyAttrs
149+
* from mUri
140150
*/
141151
void determinePrimaryKeyFromUriKeyColumn();
142152

‎tests/src/python/test_provider_postgres.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,37 @@ def testNumericPrecision(self):
349349
self.assertEqual(f['f2'], 123.456)
350350
self.assertEqual(f['f3'], '12345678.90123456789')
351351

352+
# See http://hub.qgis.org/issues/15226
353+
def testImportKey(self):
354+
uri = 'point?field=f1:int'
355+
uri += '&field=F2:double(6,4)'
356+
uri += '&field=f3:string(20)'
357+
lyr = QgsVectorLayer(uri, "x", "memory")
358+
self.assertTrue(lyr.isValid())
359+
360+
def testKey(lyr, key, kfnames):
361+
self.execSQLCommand('DROP TABLE IF EXISTS qgis_test.import_test')
362+
uri = '%s table="qgis_test"."import_test" (g) key=\'%s\'' % (self.dbconn, key)
363+
err = QgsVectorLayerImport.importLayer(lyr, uri, "postgres", lyr.crs())
364+
self.assertEqual(err[0], QgsVectorLayerImport.NoError,
365+
'unexpected import error {0}'.format(err))
366+
olyr = QgsVectorLayer(uri, "y", "postgres")
367+
self.assertTrue(olyr.isValid())
368+
flds = lyr.fields()
369+
oflds = olyr.fields()
370+
self.assertEquals(oflds.size(), flds.size())
371+
for i in range(0, oflds.size()):
372+
self.assertEqual(oflds[i].name(), flds[i].name())
373+
pks = olyr.pkAttributeList()
374+
self.assertEquals(len(pks), len(kfnames))
375+
for i in range(0, len(kfnames)):
376+
self.assertEqual(oflds[pks[i]].name(), kfnames[i])
377+
378+
testKey(lyr, 'f1', ['f1'])
379+
testKey(lyr, '"f1"', ['f1'])
380+
testKey(lyr, '"f1","F2"', ['f1', 'F2'])
381+
testKey(lyr, '"f1","F2","f3"', ['f1', 'F2', 'f3'])
382+
352383

353384
if __name__ == '__main__':
354385
unittest.main()

0 commit comments

Comments
 (0)
Please sign in to comment.