From 1822b76c1a69418e44d7a1186e16f54d670267ad Mon Sep 17 00:00:00 2001
From: Even Rouault <even.rouault@spatialys.com>
Date: Fri, 20 Oct 2017 22:50:29 +0200
Subject: [PATCH] [OGR provider/askUserForOGRSublayers] Use layername= instead
 of layerid= when no ambiguity (#16135)

When composing URI, use layer names if layer names are unique. This will
be more stable in case the datasource get layers added/removed (for example for
GeoPackage)
---
 src/app/qgisapp.cpp                   | 20 ++++++++++++++++++--
 src/providers/ogr/qgsogrdataitems.cpp | 26 +++++++++++++++++++-------
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
index 4b59b115963..83de2c942d0 100644
--- a/src/app/qgisapp.cpp
+++ b/src/app/qgisapp.cpp
@@ -4367,6 +4367,8 @@ void QgisApp::askUserForOGRSublayers( QgsVectorLayer *layer )
   QStringList sublayers = layer->dataProvider()->subLayers();
 
   QgsSublayersDialog::LayerDefinitionList list;
+  QMap< QString, int > mapLayerNameToCount;
+  bool uniqueNames = true;
   Q_FOREACH ( const QString &sublayer, sublayers )
   {
     // OGR provider returns items in this format:
@@ -4387,6 +4389,9 @@ void QgisApp::askUserForOGRSublayers( QgsVectorLayer *layer )
       def.layerName = elements[1];
       def.count = elements[2].toInt();
       def.type = elements[3];
+      int count = ++mapLayerNameToCount[def.layerName];
+      if ( count > 1 || def.layerName.isEmpty() )
+        uniqueNames = false;
       list << def;
     }
     else
@@ -4425,7 +4430,16 @@ void QgisApp::askUserForOGRSublayers( QgsVectorLayer *layer )
   Q_FOREACH ( const QgsSublayersDialog::LayerDefinition &def, chooseSublayersDialog.selection() )
   {
     QString layerGeometryType = def.type;
-    QString composedURI = uri + "|layerid=" + QString::number( def.layerId );
+    QString composedURI = uri;
+    if ( uniqueNames )
+    {
+      composedURI += "|layername=" + def.layerName;
+    }
+    else
+    {
+      // Only use layerId if there are ambiguities with names
+      composedURI += "|layerid=" + QString::number( def.layerId );
+    }
 
     if ( !layerGeometryType.isEmpty() )
     {
@@ -9772,7 +9786,9 @@ QgsVectorLayer *QgisApp::addVectorLayer( const QString &vectorLayerPath, const Q
 
     // If the newly created layer has more than 1 layer of data available, we show the
     // sublayers selection dialog so the user can select the sublayers to actually load.
-    if ( sublayers.count() > 1 && ! vectorLayerPath.contains( QStringLiteral( "layerid=" ) ) )
+    if ( sublayers.count() > 1 &&
+         ! vectorLayerPath.contains( QStringLiteral( "layerid=" ) ) &&
+         ! vectorLayerPath.contains( QStringLiteral( "layername=" ) ) )
     {
       askUserForOGRSublayers( layer );
 
diff --git a/src/providers/ogr/qgsogrdataitems.cpp b/src/providers/ogr/qgsogrdataitems.cpp
index 9b5560c42d8..0bf9d370ed8 100644
--- a/src/providers/ogr/qgsogrdataitems.cpp
+++ b/src/providers/ogr/qgsogrdataitems.cpp
@@ -157,12 +157,24 @@ QList<QgsOgrDbLayerInfo *> QgsOgrLayerItem::subLayers( const QString &path, cons
     // Collect mixed-geom layers
     QMultiMap<int, QStringList> subLayersMap;
     const QStringList subLayersList( layer.dataProvider()->subLayers( ) );
+    QMap< QString, int > mapLayerNameToCount;
+    bool uniqueNames = true;
+    int prevIdx = -1;
     for ( const QString &descriptor : subLayersList )
     {
       QStringList pieces = descriptor.split( ':' );
-      subLayersMap.insert( pieces[0].toInt(), pieces );
+      int idx = pieces[0].toInt();
+      subLayersMap.insert( idx, pieces );
+      if ( pieces.count() >= 4 && idx != prevIdx )
+      {
+        QString layerName = pieces[1];
+        int count = ++mapLayerNameToCount[layerName];
+        if ( count > 1 || layerName.isEmpty() )
+          uniqueNames = false;
+      }
+      prevIdx = idx;
     }
-    int prevIdx = -1;
+    prevIdx = -1;
     const auto subLayerKeys = subLayersMap.keys( );
     for ( const int &idx : subLayerKeys )
     {
@@ -194,13 +206,13 @@ QList<QgsOgrDbLayerInfo *> QgsOgrLayerItem::subLayers( const QString &path, cons
           }
           else
           {
+            if ( uniqueNames )
+              uri = QStringLiteral( "%1|layername=%2" ).arg( path, name );
+            else
+              uri = QStringLiteral( "%1|layerid=%2" ).arg( path, layerId );
             if ( values.size() > 1 )
             {
-              uri = QStringLiteral( "%1|layerid=%2|geometrytype=%3" ).arg( path, layerId, geometryType );
-            }
-            else
-            {
-              uri = QStringLiteral( "%1|layerid=%2" ).arg( path, layerId );
+              uri += QStringLiteral( "|geometrytype=" ) + geometryType;
             }
             QgsDebugMsgLevel( QStringLiteral( "Adding %1 Vector item %2 %3 %4" ).arg( driver, name, uri, geometryType ), 3 );
             children.append( new QgsOgrDbLayerInfo( path, uri, name, geometryColumn, geometryType, layerType ) );