Skip to content

Commit

Permalink
Fix PostgreSQL import of layers with multi-column or quoted-column keys
Browse files Browse the repository at this point in the history
Fixes #15226 (drag & drop of postgresql views)
Includes test
  • Loading branch information
strk committed Oct 14, 2016
1 parent 0ff8740 commit 1ce09b8
Show file tree
Hide file tree
Showing 3 changed files with 171 additions and 85 deletions.
211 changes: 128 additions & 83 deletions src/providers/postgres/qgspostgresprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1377,56 +1377,73 @@ bool QgsPostgresProvider::determinePrimaryKey()
return mValid;
}

void QgsPostgresProvider::determinePrimaryKeyFromUriKeyColumn()
/* static */
QStringList QgsPostgresProvider::parseUriKey( const QString& key )
{
QString primaryKey = mUri.keyColumn();
mPrimaryKeyType = pktUnknown;
if ( key.isEmpty() ) return QStringList();

if ( !primaryKey.isEmpty() )
{
QStringList cols;
QStringList cols;

// remove quotes from key list
if ( primaryKey.startsWith( '"' ) && primaryKey.endsWith( '"' ) )
// remove quotes from key list
if ( key.startsWith( '"' ) && key.endsWith( '"' ) )
{
int i = 1;
QString col;
while ( i < key.size() )
{
int i = 1;
QString col;
while ( i < primaryKey.size() )
if ( key[i] == '"' )
{
if ( primaryKey[i] == '"' )
if ( i + 1 < key.size() && key[i+1] == '"' )
{
if ( i + 1 < primaryKey.size() && primaryKey[i+1] == '"' )
{
i++;
}
else
{
cols << col;
col = "";
i++;
}
else
{
cols << col;
col = "";

if ( ++i == primaryKey.size() )
break;
if ( ++i == key.size() )
break;

Q_ASSERT( primaryKey[i] == ',' );
i++;
Q_ASSERT( primaryKey[i] == '"' );
i++;
col = "";
continue;
}
Q_ASSERT( key[i] == ',' );
i++;
Q_ASSERT( key[i] == '"' );
i++;
col = "";
continue;
}

col += primaryKey[i++];
}

col += key[i++];
}
else if ( primaryKey.contains( ',' ) )
{
cols = primaryKey.split( ',' );
}
else
}
else if ( key.contains( ',' ) )
{
cols = key.split( ',' );
}
else
{
cols << key;
}

return cols;
}

void QgsPostgresProvider::determinePrimaryKeyFromUriKeyColumn()
{
QString primaryKey = mUri.keyColumn();
mPrimaryKeyType = pktUnknown;

if ( !primaryKey.isEmpty() )
{
QStringList cols = parseUriKey( primaryKey );

primaryKey = "";
QString del = "";
Q_FOREACH ( const QString& col, cols )
{
cols << primaryKey;
primaryKey = quotedIdentifier( primaryKey );
primaryKey += del + quotedIdentifier( col );
del = ",";
}

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

QStringList pkList;
QStringList pkType;

QString schemaTableName = "";
if ( !schemaName.isEmpty() )
{
Expand Down Expand Up @@ -3523,45 +3543,39 @@ QgsVectorLayerImport::ImportError QgsPostgresProvider::createEmptyLayer(
}
else
{
// search for the passed field
for ( int fldIdx = 0; fldIdx < fields.count(); ++fldIdx )
pkList = parseUriKey( primaryKey );
Q_FOREACH ( const QString& col, pkList )
{
if ( fields[fldIdx].name() == primaryKey )
// search for the passed field
QString type;
for ( int fldIdx = 0; fldIdx < fields.count(); ++fldIdx )
{
// found, get the field type
QgsField fld = fields[fldIdx];
if ( convertField( fld, options ) )
if ( fields[fldIdx].name() == col )
{
primaryKeyType = fld.typeName();
// found, get the field type
QgsField fld = fields[fldIdx];
if ( convertField( fld, options ) )
{
type = fld.typeName();
break;
}
}
}
}
}

// if the pk field doesn't exist yet, create a serial pk field
// as it's autoincremental
if ( primaryKeyType.isEmpty() )
{
primaryKeyType = "serial";
#if 0
// TODO: check the feature count to choose if create a serial8 pk field
if ( layer->featureCount() > 0xffffffff )
{
primaryKeyType = "serial8";
}
#endif
}
else
{
// if the pk field's type is one of the postgres integer types,
// use the equivalent autoincremental type (serialN)
if ( primaryKeyType == "int2" || primaryKeyType == "int4" )
{
primaryKeyType = "serial";
}
else if ( primaryKeyType == "int8" )
{
primaryKeyType = "serial8";
if ( type.isEmpty() ) type = "serial";
else
{
// if the pk field's type is one of the postgres integer types,
// use the equivalent autoincremental type (serialN)
if ( primaryKeyType == "int2" || primaryKeyType == "int4" )
{
primaryKeyType = "serial";
}
else if ( primaryKeyType == "int8" )
{
primaryKeyType = "serial8";
}
}
pkType << type;
}
}

Expand Down Expand Up @@ -3597,17 +3611,32 @@ QgsVectorLayerImport::ImportError QgsPostgresProvider::createEmptyLayer(
throw PGException( result );
}

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

sql = QString( "CREATE TABLE %1(%2 %3 PRIMARY KEY)" )
.arg( schemaTableName,
quotedIdentifier( primaryKey ),
primaryKeyType );
if ( options && options->value( "lowercaseFieldNames", false ).toBool() )
{
col = col.toLower();
}
else
{
col = quotedIdentifier( col ); // no need to quote lowercase field
}

if ( i )
{
pk += ",";
sql += ",";
}

pk += col;
sql += col + " " + type;
}
sql += QString( ", PRIMARY KEY (%1) )" ) .arg( pk );

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

if ( fld.name() == primaryKey )
int pkIdx = -1;
for ( int i = 0; i < pkList.size(); ++i )
{
QString col = pkList[i];
if ( options && options->value( "lowercaseFieldNames", false ).toBool() )
{
//convert field name to lowercase (TODO: avoid doing this
//over and over)
col = col.toLower();
}
if ( fld.name() == col )
{
pkIdx = i;
break;
}
}
if ( pkIdx >= 0 )
{
oldToNewAttrIdxMap->insert( fldIdx, 0 );
oldToNewAttrIdxMap->insert( fldIdx, pkIdx );
continue;
}

Expand Down
14 changes: 12 additions & 2 deletions src/providers/postgres/qgspostgresprovider.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ class QgsPostgresProvider : public QgsVectorDataProvider
*/
static QString endianString();

/**
* Returns a list of unquoted column names from an uri key
*/
static QStringList parseUriKey( const QString& key );

/**
* Changes the stored extent for this layer to the supplied extent.
* For example, this is called when the extent worker thread has a result.
Expand All @@ -132,11 +137,16 @@ class QgsPostgresProvider : public QgsVectorDataProvider
*/
virtual void updateExtents() override;

/** Determine the fields making up the primary key
/**
* Determine the fields making up the primary key
*/
bool determinePrimaryKey();

/** Determine the fields making up the primary key from the uri attribute keyColumn
/**
* Determine the fields making up the primary key from the uri attribute keyColumn
*
* Fills mPrimaryKeyType and mPrimaryKeyAttrs
* from mUri
*/
void determinePrimaryKeyFromUriKeyColumn();

Expand Down
31 changes: 31 additions & 0 deletions tests/src/python/test_provider_postgres.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,37 @@ def testNumericPrecision(self):
self.assertEqual(f['f2'], 123.456)
self.assertEqual(f['f3'], '12345678.90123456789')

# See http://hub.qgis.org/issues/15226
def testImportKey(self):
uri = 'point?field=f1:int'
uri += '&field=F2:double(6,4)'
uri += '&field=f3:string(20)'
lyr = QgsVectorLayer(uri, "x", "memory")
self.assertTrue(lyr.isValid())

def testKey(lyr, key, kfnames):
self.execSQLCommand('DROP TABLE IF EXISTS qgis_test.import_test')
uri = '%s table="qgis_test"."import_test" (g) key=\'%s\'' % (self.dbconn, key)
err = QgsVectorLayerImport.importLayer(lyr, uri, "postgres", lyr.crs())
self.assertEqual(err[0], QgsVectorLayerImport.NoError,
'unexpected import error {0}'.format(err))
olyr = QgsVectorLayer(uri, "y", "postgres")
self.assertTrue(olyr.isValid())
flds = lyr.fields()
oflds = olyr.fields()
self.assertEquals(oflds.size(), flds.size())
for i in range(0, oflds.size()):
self.assertEqual(oflds[i].name(), flds[i].name())
pks = olyr.pkAttributeList()
self.assertEquals(len(pks), len(kfnames))
for i in range(0, len(kfnames)):
self.assertEqual(oflds[pks[i]].name(), kfnames[i])

testKey(lyr, 'f1', ['f1'])
testKey(lyr, '"f1"', ['f1'])
testKey(lyr, '"f1","F2"', ['f1', 'F2'])
testKey(lyr, '"f1","F2","f3"', ['f1', 'F2', 'f3'])


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

0 comments on commit 1ce09b8

Please sign in to comment.