Skip to content

Commit

Permalink
[gpkg] Also read relationships defined using foreign key constraints
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
nyalldawson committed Feb 22, 2024
1 parent 4926535 commit 288977e
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 14 deletions.
27 changes: 27 additions & 0 deletions autotest/ogr/ogr_gpkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
18 changes: 14 additions & 4 deletions ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
3 changes: 2 additions & 1 deletion ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
39 changes: 30 additions & 9 deletions ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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())
Expand Down Expand Up @@ -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 "
Expand All @@ -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)
{
Expand Down

0 comments on commit 288977e

Please sign in to comment.