[gpkg] Also read relationships defined using foreign key constraints

When reading relationships, always read relationships defined using
foreign key constraints regardless of whether or not the related
tables extension is in use.

The related table extension only permits definition of many-to-many
relationships, so there's a strong case for supporting one-to-many
relationships defined outside of this extension. In fact it's what's
recommended upstream: opengeospatial/geopackage#678 (comment)

Author: nyalldawson

autotest/ogr/ogr_gpkg.py

@@ -7086,6 +7086,33 @@ def test_ogr_gpkg_relations(tmp_vsimem, tmp_path):
     assert rel.GetRightMappingTableFields() == ["related_id"]
     assert rel.GetRelatedTableType() == "features"
 
+    # a one-to-many relationship defined using foreign key constraints
+    ds = gdal.OpenEx(filename, gdal.OF_VECTOR | gdal.OF_UPDATE)
+    ds.ExecuteSQL(
+        "CREATE TABLE test_relation_a(artistid INTEGER PRIMARY KEY, artistname  TEXT)"
+    )
+    ds.ExecuteSQL(
+        "CREATE TABLE test_relation_b(trackid INTEGER, trackname TEXT, trackartist INTEGER, FOREIGN KEY(trackartist) REFERENCES test_relation_a(artistid))"
+    )
+    ds = None
+
+    ds = gdal.OpenEx(filename, gdal.OF_VECTOR | gdal.OF_UPDATE)
+    assert ds.GetRelationshipNames() == [
+        "custom_type",
+        "test_relation_a_test_relation_b",
+    ]
+    assert ds.GetRelationship("custom_type") is not None
+    rel = ds.GetRelationship("test_relation_a_test_relation_b")
+    assert rel is not None
+    assert rel.GetName() == "test_relation_a_test_relation_b"
+    assert rel.GetLeftTableName() == "test_relation_a"
+    assert rel.GetRightTableName() == "test_relation_b"
+    assert rel.GetCardinality() == gdal.GRC_ONE_TO_MANY
+    assert rel.GetType() == gdal.GRT_ASSOCIATION
+    assert rel.GetLeftTableFields() == ["artistid"]
+    assert rel.GetRightTableFields() == ["trackartist"]
+    assert rel.GetRelatedTableType() == "features"
+
     ds = None
 
 

ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp

@@ -2018,14 +2018,24 @@ void GDALGeoPackageDataset::ClearCachedRelationships()
 
 void GDALGeoPackageDataset::LoadRelationships() const
 {
+    m_osMapRelationships.clear();
+
+    std::vector<std::string> oExcludedTables;
     if (HasGpkgextRelationsTable())
     {
         LoadRelationshipsUsingRelatedTablesExtension();
+
+        for (const auto &oRelationship : m_osMapRelationships)
+        {
+            oExcludedTables.emplace_back(
+                oRelationship.second->GetMappingTableName());
+        }
     }
-    else
-    {
-        LoadRelationshipsFromForeignKeys();
-    }
+
+    // Also load relationships defined using foreign keys (i.e. one-to-many
+    // relationships). Here we must exclude any relationships defined from the
+    // related tables extension, we don't want them included twice.
+    LoadRelationshipsFromForeignKeys(oExcludedTables);
     m_bHasPopulatedRelationships = true;
 }
 

ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h

@@ -216,7 +216,8 @@ class OGRSQLiteBaseDataSource CPL_NON_FINAL : public GDALPamDataset
     OGRErr PragmaCheck(const char *pszPragma, const char *pszExpected,
                        int nRowsExpected);
 
-    void LoadRelationshipsFromForeignKeys() const;
+    void LoadRelationshipsFromForeignKeys(
+        const std::vector<std::string> &excludedTables) const;
 
     bool IsSpatialiteLoaded();
     static int MakeSpatialiteVersionNumber(int x, int y, int z)

ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp

@@ -284,7 +284,10 @@ std::vector<std::string> OGRSQLiteDataSource::GetRelationshipNames(
 
 {
     if (!m_bHasPopulatedRelationships)
-        LoadRelationshipsFromForeignKeys();
+    {
+        m_osMapRelationships.clear();
+        LoadRelationshipsFromForeignKeys({});
+    }
 
     std::vector<std::string> oasNames;
     oasNames.reserve(m_osMapRelationships.size());
@@ -305,7 +308,10 @@ OGRSQLiteDataSource::GetRelationship(const std::string &name) const
 
 {
     if (!m_bHasPopulatedRelationships)
-        LoadRelationshipsFromForeignKeys();
+    {
+        m_osMapRelationships.clear();
+        LoadRelationshipsFromForeignKeys({});
+    }
 
     auto it = m_osMapRelationships.find(name);
     if (it == m_osMapRelationships.end())
@@ -707,15 +713,13 @@ OGRErr OGRSQLiteBaseDataSource::PragmaCheck(const char *pszPragma,
 /*                LoadRelationshipsFromForeignKeys()                    */
 /************************************************************************/
 
-void OGRSQLiteBaseDataSource::LoadRelationshipsFromForeignKeys() const
+void OGRSQLiteBaseDataSource::LoadRelationshipsFromForeignKeys(
+    const std::vector<std::string> &excludedTables) const
 
 {
-    m_osMapRelationships.clear();
-
     if (hDB)
     {
-        auto oResult = SQLQuery(
-            hDB,
+        std::string osSQL =
             "SELECT m.name, p.id, p.seq, p.\"table\" AS base_table_name, "
             "p.\"from\", p.\"to\", "
             "p.on_delete FROM sqlite_master m "
@@ -728,8 +732,25 @@ void OGRSQLiteBaseDataSource::LoadRelationshipsFromForeignKeys() const
             // Same with Spatialite system tables
             "AND base_table_name NOT IN ('geometry_columns', "
             "'spatial_ref_sys', 'views_geometry_columns', "
-            "'virts_geometry_columns') "
-            "ORDER BY m.name");
+            "'virts_geometry_columns') ";
+        if (!excludedTables.empty())
+        {
+            std::string oExcludedTablesList;
+            for (const auto &osExcludedTable : excludedTables)
+            {
+                oExcludedTablesList += !oExcludedTablesList.empty() ? "," : "";
+                oExcludedTablesList +=
+                    sqlite3_mprintf("'%q'", osExcludedTable.c_str());
+            }
+
+            osSQL += "AND base_table_name NOT IN (" + oExcludedTablesList +
+                     ")"
+                     " AND m.name NOT IN (" +
+                     oExcludedTablesList + ") ";
+        }
+        osSQL += "ORDER BY m.name";
+
+        auto oResult = SQLQuery(hDB, osSQL.c_str());
 
         if (!oResult)
         {