Trouble creating relations in GPKG to multigeometry tables

signedav commented 3 years ago

While in PG three layers with the same tables are created, in GPKG it creates three tables. The Relation to it are brocken then.

See example:

INTERLIS 2.3;

MODEL GeometriesStruct (en) 
AT "https://signedav.github.io/usabilitydave/models"
VERSION "2020-06-22" =

    IMPORTS GeometryCHLV95_V1, Units;

    DOMAIN
        Line = POLYLINE WITH (STRAIGHTS) VERTEX GeometryCHLV95_V1.Coord2;
        Surface = SURFACE WITH (STRAIGHTS) VERTEX GeometryCHLV95_V1.Coord2 WITHOUT OVERLAPS > 0.005;

        STRUCTURE Geometry =
            Points: GeometryCHLV95_V1.Coord2;
            Lines: Line;
            Surfaces: Surface;
            Remark : TEXT*20;
        END Geometry; 

    TOPIC Assets =

        CLASS Asset =
            Name: TEXT;
            Geometries: BAG {0..*} OF Geometry;
        END Asset;

    END Assets;

END GeometriesStruct.

signedav commented 2 years ago

In fact we get a relation to the point layer (containing remarks). And in the point layer a relation to the other geometry layers (as T_Id).

But e.g. in the KbS_LV95_V1_4 we have "Belasteter_Standort" -> proper polygon layer and "Belasteter_Standort (Geo_Lage_Punkt)" - How is this managed on working on those data? Is there a sufficient solution for multi-geom-classes with GPKG at all?

@romefi @m-kuhn any idea?

signedav commented 1 year ago

Maybe find better sollution in ili2db - I'll have another look on it.

m-kuhn commented 1 year ago

There was a discussion (with @claeis among others) to create multi-geometry gpkgs with ili2gpkg and to be able to edit these with qgis/ogr.

This would be clearly an extension to the gpkg standard and other software would not be able to properly interact with gpkgs of this format. Still it might be interesting for workflows which are only withing qgis/ogr/ili2gpkg.

claeis commented 1 year ago

It would require a alternative/extension to the gpkg metatable gpkg_contents (a new column column_name) and a change in the constraints of metatable gpkg_geometry_columns (so that they include column column_name)

m-kuhn commented 1 year ago

It would require a alternative/extension to the gpkg metatable gpkg_contents (a new column column_name) and a change in the constraints of metatable gpkg_geometry_columns (so that they include column column_name)

CC @rouault , does this sound like a feasible approach to you?

claeis commented 1 year ago

Other tools/clients might fail with more than one record in gpkg_geometry_columns per table. A more compatible first step could be: add an alternative geometry_columns table without a reference to gpkg_contents (and include columns such as min_x, min_y, max_x, max_y from gpkg_contents if required at all (would be more like OGC SF SQL))

rouault commented 1 year ago

CC @rouault , does this sound like a feasible approach to you?

A potential solution compliant with the existing spec would be to have a table with multiple geometry columns that is not referenced in gpkg_contents, and as many views as geometry columns in it selecting one geometry column at a time and all other fields + triggers to make them update the base table.

signedav commented 1 year ago

What do you think about Even's idea with the views @claeis ?

claeis commented 1 year ago

The main problem, that a more complex table structure results from an application model, is not solved by VIEWs, but displaced. That is why I would rather not do this or only as an optional addition.

m-kuhn commented 1 year ago

From my understanding, the following are the main risks and advantages of the two approaches.

Advantage views:

Needs to be implemented once where the package is created
Compliant with the standard
Existing software should support it

Risk views:

Complexity in the geopackage added
"Hack" to circumvent shortcomings of the standard
Modifications of the table structure in the geopackage post-creation is difficult

Advantage gpkg extension:

Simple and extensible approach
Might trigger a change of the standard (simliar to browser specific css extensions that eventually can become the standard)
Generic approach also for other use cases than Interlis

Risk gpkg extension:

Existing software might not be able to open such a geopackage at all / will certainly not see additional geom columns
Requires patching applications with gpkg support (which for open source mainly means gdal/ogr, ili2gpkg)

Can someone think of more things that I forgot?

rouault commented 1 year ago

From my understanding, the following are the main risks and advantages of the two approaches.

good summary. I've added some minimal (read) support for multiple geometry columns per table in https://github.com/OSGeo/gdal/pull/7633. As noted in the pull request comment, the behaviour might change in the future, but at least with that, such extented geopackages have a minimum form of support.

claeis commented 1 year ago

implemented a new option --gpkgMultiGeomPerTable in ili2gpkg-4.11.1-SNAPSHOT

signedav commented 7 months ago

I finally made some tests with the implementation in ili2gpkg https://github.com/claeis/ili2db/issues/511 and GDAL https://github.com/OSGeo/gdal/pull/7633 Here are some notes:

Creating the model with ili2gpkg and option `--gpkgMultiGeomPerTable`

INTERLIS 2.3;

MODEL MultiGeom (en) 
AT "http://modelbaker.ch"
VERSION "2020-06-22" =

    IMPORTS GeometryCHLV95_V1;

    DOMAIN
      Line = POLYLINE WITH (STRAIGHTS) VERTEX GeometryCHLV95_V1.Coord2;
      Surface = SURFACE WITH (STRAIGHTS) VERTEX GeometryCHLV95_V1.Coord2 WITHOUT OVERLAPS > 0.005;

      TOPIC Spots =
        CLASS POI =
          Name: TEXT;
          Point: GeometryCHLV95_V1.Coord2;
          Line: Line;
          Surface: Surface;
        END POI;
      END Spots;

END MultiGeom.

Creates the following entries:

Test on QGIS

With the currently used GDAL (version 3.4) in QGIS it just read the first available record (means line).

I build it locally (version 3.9.0dev- - a primer but supereasy thanks to https://gdal.org/development/building_from_source.html)
I changed the GDAL stuff in the QGIS Build Settings
Rebuild QGIS

And I could drag'n'drop the GeoPackage and even add stuff to it. No (at least until now) locks on Transaction Mode.

Test with Model Baker

Model Baker did not add the geometry column to the layer why the sources where broken. So I fixed this here https://github.com/opengisch/QgisModelBakerLibrary/pull/91 and as well I added the ili2db parameter

Looks good:

And then I tested with KbS_V1_5

Having now everything ready I made a test with KbS_V1_5 and I did not manage to create a lock...

See this video...

multiediting.webm

ili2gpkg Error on Export

On Export I receive this error @claeis

Error: failed to query KbS_V1_5.Belastete_Standorte.Belasteter_Standort
Error:   [SQLITE_ERROR] SQL error or missing database (no such table: belasteter_standort_geo_lage_punkt)

or

Error: failed to query MultiGeom.Spots.POI
Error:   [SQLITE_ERROR] SQL error or missing database (no such table: poi_line)

~Should I create an issue on ili2db for it or is this "feature" not official enough yet?~ Update 15.5.24: The problem is Model Baker not passing --gpkgMultiGeomPerTable in export/import/validate. This means it needs to recognize this first and then add it accordingly. But shouldn't ili2db still "know" it? Issue here https://github.com/claeis/ili2db/issues/542

And now?

Before getting to enthusiastic it needs to be mentioned, that I'm not sure how ready this all is for production.

@rouault I've been in contact with you quite some time ago. You mentioned the "clean" support of having it in the OGR GeoPackage driver (to e.g. being able to convert such a GPKG to another database etc, right?) and everything else that is needed (like documentation, tests etc). Does this concern the finalization of the current approach or is this an additional (and optional) thing?

Back then you tested it in QGIS and you mentioned: "by quickly testing the OGR provider in QGIS with a datasource with multiple geometry fields, I'm surprised to see it doesn't handle that properly, and only takes into account the first geometry field." But in my use cases it worked - where there implementations on the OGR provider in QGIS meanwhile?

Besides that mentioned concerns about locks. Since it works fine at the moment, it's hard to tell. But I guess it would make sense to make some deeper research. I actually spend some time on this topic (simulated by adding one table multiple times to QGIS) and noted everything here https://github.com/qgis/QGIS/issues/55530 and I will continue with it. Happy for every kind of inputs...

Thank you all.

rouault commented 7 months ago

Does this concern the finalization of the current approach or is this an additional (and optional) thing?

the clean approach I mentioned would be to expose a single OGR layer with multiple geometry columns, instead of the somewhat hacky approach taken in https://github.com/OSGeo/gdal/pull/7633 where one exposes as many layers as there are geometry columns in the table

But in my use cases it worked - where there implementations on the OGR provider in QGIS meanwhile?

I don't think you've tested what I had in mind. If you've tested https://github.com/OSGeo/gdal/pull/7633 , then there are several OGR layers, so QGIS has no trouble with that. I've just tested a GML file with 2 geometry columns in a layer, which OGR sees as a single layer with 2 geometry columns, and as I mentioned QGIS just sees the first geometry column and completely discard the other one(s). So no progress on that front in the QGIS OGR provider

signedav commented 7 months ago

I don't think you've tested what I had in mind. If you've tested https://github.com/OSGeo/gdal/pull/7633 , then there are several OGR layers, so QGIS has no trouble with that.

Ah yes. But this is basically the behavior I expect in QGIS. On a PostgreSQL table with multiple geometry columns I usually get two layers as well.

I've just tested a GML file with 2 geometry columns in a layer, which OGR sees as a single layer with 2 geometry columns, and as I mentioned QGIS just sees the first geometry column and completely discard the other one(s). So no progress on that front in the QGIS OGR provider.

I see. Is this required for this topic here (maybe I miss something)?

rouault commented 7 months ago

Ah yes. But this is basically the behavior I expect in QGIS

if we would want to expose the GPKG table as a single OGR layer with 2 geometry columns (for example to be able to convert it to a single PostgreSQL table with ogr2ogr), then yes we would need to improve the QGIS OGR provider to expose as many QGIS layers as there are geometry columns in the OGR layer.

signedav commented 6 months ago

Okay, but doesn't PostgreSQL provide two layers as well? The QGIS behavior is that there are loaded two layers for two geometry. Otherwise QGIS is not ready to visualize, as far as I understand it.

So why not finalizing the current implementation as a first (and maybe sufficient in this scope) step and make further thoughts on the options to be able to handle as one single layer? Because I think the current state would make lots of QGIS/INTERLIS/ModelBaker users happy...

signedav commented 6 months ago

What do you think about that @rouault Can we go with this approach as a first solution? And if so, is there an overview what GDAL version is used in what QGIS release? Which version [uses|will use] > 3.7 used?

rouault commented 6 months ago

Can we go with this approach as a first solution?

I assume you're talking about the approach of https://github.com/OSGeo/gdal/pull/7633. Fine to me.

And if so, is there an overview what GDAL version is used in what QGIS release? Which version [uses|will use] > 3.7 used?

There is no strong coupling. Depends on packagers. But most of the time if they package the latest QGIS release they will also likely package the latest GDAL version too.

signedav commented 6 months ago

Info: 3.36 is coupled with 3.8.5

rouault commented 6 months ago

3.36 is coupled with 3.8.5

Where do you see that ? Or are you talking about official Windows binaries ? The minimum GDAL requirement is 3.2 otherwise

signedav commented 6 months ago

Yes. Official Windows binaries.

opengisch / QgisModelBaker