Merge pull request #9279 from nyalldawson/gpkg_one_to_many

[gpkg] Also read relationships defined using foreign key constraints

Author: rouault

autotest/ogr/ogr_gpkg.py

@@ -7093,6 +7093,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
 
 

doc/source/drivers/vector/gpkg.rst

@@ -168,9 +168,8 @@ Relationships
 
 .. versionadded:: 3.6
 
-Relationship retrieval is supported, respecting the OGC GeoPackage Related Tables Extension.
-If the Related Tables Extension is not in use then relationships will be reported for tables
-which utilize FOREIGN KEY constraints.
+Many-to-many relationship retrieval is supported, respecting the OGC GeoPackage Related Tables Extension.
+One-to-many relationships will also be reported for tables which utilize FOREIGN KEY constraints.
 
 Relationship creation, deletion and updating is supported since GDAL 3.7. Relationships can
 only be updated to change their base or related table fields, or the relationship related

ogr/ogrsf_frmts/gpkg/ogr_geopackage.h

@@ -280,7 +280,7 @@ class GDALGeoPackageDataset final : public OGRSQLiteBaseDataSource,
     void FixupWrongRTreeTrigger();
     void FixupWrongMedataReferenceColumnNameUpdate();
     void ClearCachedRelationships();
-    void LoadRelationships() const;
+    void LoadRelationships() const override;
     void LoadRelationshipsUsingRelatedTablesExtension() const;
     static std::string
     GenerateNameForRelationship(const char *pszBaseTableName,
@@ -345,12 +345,6 @@ class GDALGeoPackageDataset final : public OGRSQLiteBaseDataSource,
     bool AddFieldDomain(std::unique_ptr<OGRFieldDomain> &&domain,
                         std::string &failureReason) override;
 
-    std::vector<std::string>
-    GetRelationshipNames(CSLConstList papszOptions = nullptr) const override;
-
-    const GDALRelationship *
-    GetRelationship(const std::string &name) const override;
-
     bool AddRelationship(std::unique_ptr<GDALRelationship> &&relationship,
                          std::string &failureReason) override;
 

ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp

@@ -2038,14 +2038,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;
 }
 
@@ -9813,45 +9823,6 @@ bool GDALGeoPackageDataset::AddFieldDomain(
     return true;
 }
 
-/************************************************************************/
-/*                        GetRelationshipNames()                        */
-/************************************************************************/
-
-std::vector<std::string> GDALGeoPackageDataset::GetRelationshipNames(
-    CPL_UNUSED CSLConstList papszOptions) const
-
-{
-    if (!m_bHasPopulatedRelationships)
-        LoadRelationships();
-
-    std::vector<std::string> oasNames;
-    oasNames.reserve(m_osMapRelationships.size());
-    for (auto it = m_osMapRelationships.begin();
-         it != m_osMapRelationships.end(); ++it)
-    {
-        oasNames.emplace_back(it->first);
-    }
-    return oasNames;
-}
-
-/************************************************************************/
-/*                        GetRelationship()                             */
-/************************************************************************/
-
-const GDALRelationship *
-GDALGeoPackageDataset::GetRelationship(const std::string &name) const
-
-{
-    if (!m_bHasPopulatedRelationships)
-        LoadRelationships();
-
-    auto it = m_osMapRelationships.find(name);
-    if (it == m_osMapRelationships.end())
-        return nullptr;
-
-    return it->second.get();
-}
-
 /************************************************************************/
 /*                          AddRelationship()                           */
 /************************************************************************/

ogr/ogrsf_frmts/sqlite/ogr_sqlite.h

@@ -792,12 +792,6 @@ class OGRSQLiteDataSource final : public OGRSQLiteBaseDataSource
         return m_bHaveGeometryColumns;
     }
 
-    std::vector<std::string>
-    GetRelationshipNames(CSLConstList papszOptions = nullptr) const override;
-
-    const GDALRelationship *
-    GetRelationship(const std::string &name) const override;
-
     bool AddRelationship(std::unique_ptr<GDALRelationship> &&relationship,
                          std::string &failureReason) override;
     bool ValidateRelationship(const GDALRelationship *poRelationship,

ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h

@@ -216,7 +216,13 @@ class OGRSQLiteBaseDataSource CPL_NON_FINAL : public GDALPamDataset
     OGRErr PragmaCheck(const char *pszPragma, const char *pszExpected,
                        int nRowsExpected);
 
-    void LoadRelationshipsFromForeignKeys() const;
+    virtual void LoadRelationships() const;
+    void LoadRelationshipsFromForeignKeys(
+        const std::vector<std::string> &excludedTables) const;
+    std::vector<std::string>
+    GetRelationshipNames(CSLConstList papszOptions = nullptr) const override;
+    const GDALRelationship *
+    GetRelationship(const std::string &name) const override;
 
     bool IsSpatialiteLoaded();
     static int MakeSpatialiteVersionNumber(int x, int y, int z)

ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp

@@ -275,45 +275,6 @@ int OGRSQLiteBaseDataSource::GetSpatialiteVersionNumber()
     return v;
 }
 
-/************************************************************************/
-/*                        GetRelationshipNames()                        */
-/************************************************************************/
-
-std::vector<std::string> OGRSQLiteDataSource::GetRelationshipNames(
-    CPL_UNUSED CSLConstList papszOptions) const
-
-{
-    if (!m_bHasPopulatedRelationships)
-        LoadRelationshipsFromForeignKeys();
-
-    std::vector<std::string> oasNames;
-    oasNames.reserve(m_osMapRelationships.size());
-    for (auto it = m_osMapRelationships.begin();
-         it != m_osMapRelationships.end(); ++it)
-    {
-        oasNames.emplace_back(it->first);
-    }
-    return oasNames;
-}
-
-/************************************************************************/
-/*                        GetRelationship()                             */
-/************************************************************************/
-
-const GDALRelationship *
-OGRSQLiteDataSource::GetRelationship(const std::string &name) const
-
-{
-    if (!m_bHasPopulatedRelationships)
-        LoadRelationshipsFromForeignKeys();
-
-    auto it = m_osMapRelationships.find(name);
-    if (it == m_osMapRelationships.end())
-        return nullptr;
-
-    return it->second.get();
-}
-
 /************************************************************************/
 /*                          AddRelationship()                           */
 /************************************************************************/
@@ -704,18 +665,28 @@ OGRErr OGRSQLiteBaseDataSource::PragmaCheck(const char *pszPragma,
 }
 
 /************************************************************************/
-/*                LoadRelationshipsFromForeignKeys()                    */
+/*                     LoadRelationships()                              */
 /************************************************************************/
 
-void OGRSQLiteBaseDataSource::LoadRelationshipsFromForeignKeys() const
+void OGRSQLiteBaseDataSource::LoadRelationships() const
 
 {
     m_osMapRelationships.clear();
+    LoadRelationshipsFromForeignKeys({});
+    m_bHasPopulatedRelationships = true;
+}
 
+/************************************************************************/
+/*                LoadRelationshipsFromForeignKeys()                    */
+/************************************************************************/
+
+void OGRSQLiteBaseDataSource::LoadRelationshipsFromForeignKeys(
+    const std::vector<std::string> &excludedTables) const
+
+{
     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 +699,27 @@ 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() ? "," : "";
+                char *pszEscapedName =
+                    sqlite3_mprintf("'%q'", osExcludedTable.c_str());
+                oExcludedTablesList += pszEscapedName;
+                sqlite3_free(pszEscapedName);
+            }
+
+            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)
         {
@@ -806,9 +796,50 @@ void OGRSQLiteBaseDataSource::LoadRelationshipsFromForeignKeys() const
                     std::move(poRelationship);
             }
         }
+    }
+}
+
+/************************************************************************/
+/*                        GetRelationshipNames()                        */
+/************************************************************************/
+
+std::vector<std::string> OGRSQLiteBaseDataSource::GetRelationshipNames(
+    CPL_UNUSED CSLConstList papszOptions) const
+
+{
+    if (!m_bHasPopulatedRelationships)
+    {
+        LoadRelationships();
+    }
+
+    std::vector<std::string> oasNames;
+    oasNames.reserve(m_osMapRelationships.size());
+    for (auto it = m_osMapRelationships.begin();
+         it != m_osMapRelationships.end(); ++it)
+    {
+        oasNames.emplace_back(it->first);
+    }
+    return oasNames;
+}
+
+/************************************************************************/
+/*                        GetRelationship()                             */
+/************************************************************************/
 
-        m_bHasPopulatedRelationships = true;
+const GDALRelationship *
+OGRSQLiteBaseDataSource::GetRelationship(const std::string &name) const
+
+{
+    if (!m_bHasPopulatedRelationships)
+    {
+        LoadRelationships();
     }
+
+    auto it = m_osMapRelationships.find(name);
+    if (it == m_osMapRelationships.end())
+        return nullptr;
+
+    return it->second.get();
 }
 
 /***********************************************************************/