Interlis update #414

domi4484 · 2024-09-11T14:51:17Z

Fixes #397

This fixes 2 issues:

We are using the SqlAlchemy session as a cache, even if the documentation recommends not doing so because the instances in the session are weak references. To countourn that we use flag_dirty which in turn makes all not new objects look like modified even if they are not.
Objects updated are not written in the DB, because we don't tell SqlAlchemy which columns are changed (we replace internal values directly (private API)). Yes we flag the object as dirty but at the moment of writing SqlAlchemy notice that no column of that object was changed, and it skips it

The proposed solution keeps the session/cache but tries to tell sql alchemy which attributes where really changed.
Maybe long term it would be better to build an own cache.

Warning

This could have some performance implications. Need to be tested.

To help me debugging this one I pimped up a bit the import dialog with better display of the importing changes:

For categories I added a * state in case there is a modified item inside
Put the "Raw infos" into a tree widget
For modified item, show a Old value column with the current DB value
Highlight changed attributes in orange

cymed · 2024-09-12T07:29:14Z

@domi4484 the Error "SIA405_Base_Abwasser_1_LV95: model(s) not found" has been appearing regularly in CI. I don't know why, but re-running the tests usually helps

domi4484 · 2024-09-12T09:46:34Z

@domi4484 the Error "SIA405_Base_Abwasser_1_LV95: model(s) not found" has been appearing regularly in CI. I don't know why, but re-running the tests usually helps

I have also seen it sometimes. It seems that the repository 405.sia.ch is not always available. Maybe we could ask them if they are aware of it?

To avoid the issue in future we could download a copy of the needed models to keep within the source of the plugin.

ili2db error:

Warning: repository <https://405.sia.ch/models/> ignored; java.net.ConnectException: Connection timed out (Connection timed out); Connection timed out (Connection timed out)

ponceta · 2024-09-18T07:52:20Z

Needs testing but looks promising! Thank you @domi4484 !

ponceta · 2024-09-18T14:28:15Z

I imported the VSA test organisations
Then the VSA DSS dataset
Then modified my data
Then reimport the VSA DSS dataset

Got this error message :

sqlalchemy.exc.ArgumentError: Executable SQL or text() construct expected, got . 
Traceback (most recent call last):
  File "C:\Users/pulpoar/AppData/Roaming/QGIS/QGIS3\profiles\pulpoar/python/plugins\teksi_wastewater\teksi_wastewater_plugin.py", line 612, in actionImportClicked
    self.interlisImporterExporter.action_import()
  File "C:\Users/pulpoar/AppData/Roaming/QGIS/QGIS3\profiles\pulpoar/python/plugins\teksi_wastewater\interlis\gui\interlis_importer_exporter_gui.py", line 79, in action_import
    raise exception
  File "C:\Users/pulpoar/AppData/Roaming/QGIS/QGIS3\profiles\pulpoar/python/plugins\teksi_wastewater\interlis\gui\interlis_importer_exporter_gui.py", line 60, in action_import
    self.interlis_importer_exporter.interlis_import(
  File "C:\Users/pulpoar/AppData/Roaming/QGIS/QGIS3\profiles\pulpoar/python/plugins\teksi_wastewater\interlis\interlis_importer_exporter.py", line 148, in interlis_import
    raise exception
  File "C:\Users/pulpoar/AppData/Roaming/QGIS/QGIS3\profiles\pulpoar/python/plugins\teksi_wastewater\interlis\interlis_importer_exporter.py", line 106, in interlis_import
    tww_session = self._import_from_intermediate_schema(import_model)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users/pulpoar/AppData/Roaming/QGIS/QGIS3\profiles\pulpoar/python/plugins\teksi_wastewater\interlis\interlis_importer_exporter.py", line 261, in _import_from_intermediate_schema
    interlisImporterToIntermediateSchema.tww_import(skip_closing_tww_session=True)
  File "C:\Users/pulpoar/AppData/Roaming/QGIS/QGIS3\profiles\pulpoar/python/plugins\teksi_wastewater\interlis\interlis_model_mapping\interlis_importer_to_intermediate_schema.py", line 41, in tww_import
    raise exception
  File "C:\Users/pulpoar/AppData/Roaming/QGIS/QGIS3\profiles\pulpoar/python/plugins\teksi_wastewater\interlis\interlis_model_mapping\interlis_importer_to_intermediate_schema.py", line 33, in tww_import
    self._tww_import(skip_closing_tww_session)
  File "C:\Users/pulpoar/AppData/Roaming/QGIS/QGIS3\profiles\pulpoar/python/plugins\teksi_wastewater\interlis\interlis_model_mapping\interlis_importer_to_intermediate_schema.py", line 66, in _tww_import
    self._import_sia405_abwasser()
  File "C:\Users/pulpoar/AppData/Roaming/QGIS/QGIS3\profiles\pulpoar/python/plugins\teksi_wastewater\interlis\interlis_model_mapping\interlis_importer_to_intermediate_schema.py", line 116, in _import_sia405_abwasser
    self._import_haltung_alternativverlauf()
  File "C:\Users/pulpoar/AppData/Roaming/QGIS/QGIS3\profiles\pulpoar/python/plugins\teksi_wastewater\interlis\interlis_model_mapping\interlis_importer_to_intermediate_schema.py", line 1929, in _import_haltung_alternativverlauf
    reach_progression_alternative = self.create_or_update(
                                    ^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users/pulpoar/AppData/Roaming/QGIS/QGIS3\profiles\pulpoar/python/plugins\teksi_wastewater\interlis\interlis_model_mapping\interlis_importer_to_intermediate_schema.py", line 450, in create_or_update
    value = self.session_tww.scalar(value)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\OSGeo4W\apps\Python312\Lib\site-packages\sqlalchemy\orm\session.py", line 2354, in scalar
    return self._execute_internal(
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\OSGeo4W\apps\Python312\Lib\site-packages\sqlalchemy\orm\session.py", line 2089, in _execute_internal
    statement = coercions.expect(roles.StatementRole, statement)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\OSGeo4W\apps\Python312\Lib\site-packages\sqlalchemy\sql\coercions.py", line 396, in expect
    resolved = impl._literal_coercion(
               ^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\OSGeo4W\apps\Python312\Lib\site-packages\sqlalchemy\sql\coercions.py", line 634, in _literal_coercion
    self._raise_for_expected(element, argname)
  File "C:\OSGeo4W\apps\Python312\Lib\site-packages\sqlalchemy\sql\coercions.py", line 518, in _raise_for_expected
    raise exc.ArgumentError(msg, code=code) from err
sqlalchemy.exc.ArgumentError: Executable SQL or text() construct expected, got .

Restart QGIS

Same error.

ponceta · 2024-09-18T14:41:29Z

It looks a bit slower than before but it is quite better in terms of visibility (what we import, what we will update,...)

domi4484 · 2024-09-20T08:04:07Z

@ponceta thanks for testing! The error should be fixed with the last commit.

But I have seen another glitch, new to be imported objects are displayed as modified instead of new.

It looks a bit slower than before but it is quite better in terms of visibility (what we import, what we will update,...)

In fact on my machine the DSS import take pretty much the same time (around 40 seconds) with both versions

ponceta · 2024-09-20T08:16:58Z

2024-09-20T10:16:12 WARNING Subclass Count error : Too many superclass entries for tww_od.wastewater_structure

Importing again using new commit.

    Workaround :
    Disabling 
    - re_maintenance_event_wastewater_structure
    - re_building_group_disposal
    Import works.

ponceta · 2024-09-20T08:30:59Z

It works well for the organisations :

Import Organisations
Modify an organisation
Re-import organisations

ponceta · 2024-09-20T09:01:28Z

Bugs on update :

re_maintenance_event_wastewater_structure
re_building_group_disposal

wastewater_node (situation3d_geometry)

reach (length_effective)

cover (situation3d_geometry)

bio_ecol_assessment (time_point / date_last_examen)

building_group (renovation_date)

Measure (date_entry)

measurement_result (time, value)

mutation (date_mutation)

profile_geometry (x,y)

maintenance (time_point)

Remaining questions :

if modification date is older than actual data, do we warn the user?
if there are no modifications (attribute or geometry) but the last_modification attribute, do we modify the last_modification with the import data last_modification?

Check for already existing re_... instances

domi4484 · 2024-09-26T20:36:04Z

@ponceta most points should be solved with last commit, but there is one open issue with geometries that I am not sure how to solve.

It seems to me that in the model we have sometimes redundant data: for example for wastewater_node ther is bottom_level and the z coordinate of situation3d_geometry and following happens:

On first import (example wastewater_node 1.001):

	Bottom level	Situation geom Z
Database before	None	None
Importing value	1913.5	0.0
Database after	1913.5	1913.5

On second import of the same data we lose Z information!

	Bottom level	Situation geom Z
Database before	1913.5	1913.5
Importing value	1913.5	0.0
Database after	0.0	0.0

On third import of same data Z is back

	Bottom level	Situation geom Z
Database before	0.0	0.0
Importing value	1913.5	0.0
Database after	1913.5	1913.5

On forth import we lose Z again
and so on...

First import:

Second import

Third import

cymed · 2024-09-27T06:48:43Z

The problem lies here:

wastewater/datamodel/app/functions/14_geometry_functions.sql

Lines 10 to 28 in c7c7e36

    
           CREATE OR REPLACE FUNCTION tww_app.synchronize_level_with_altitude_on_wastewater_node() 
        
             RETURNS trigger AS 
        
           $BODY$ 
        
           BEGIN 
        
             CASE 
        
               WHEN TG_OP = 'INSERT' THEN 
        
                 NEW.situation3d_geometry = ST_SetSRID( ST_MakePoint( ST_X(NEW.situation3d_geometry), ST_Y(NEW.situation3d_geometry), COALESCE(NEW.bottom_level,'NaN') ), %1$s); 
        
               WHEN TG_OP = 'UPDATE' THEN 
        
                 IF NEW.bottom_level <> OLD.bottom_level OR (NEW.bottom_level IS NULL AND OLD.bottom_level IS NOT NULL) OR (NEW.bottom_level IS NOT NULL AND OLD.bottom_level IS NULL) THEN 
        
                   NEW.situation3d_geometry = ST_SetSRID( ST_MakePoint( ST_X(NEW.situation3d_geometry), ST_Y(NEW.situation3d_geometry), COALESCE(NEW.bottom_level,'NaN') ), %1$s); 
        
                 ELSE 
        
                   IF ST_Z(NEW.situation3d_geometry) <> ST_Z(OLD.situation3d_geometry) THEN 
        
                     NEW.bottom_level = NULLIF(ST_Z(NEW.situation3d_geometry),'NaN'); 
        
                   END IF; 
        
                 END IF; 
        
             END CASE; 
        
             RETURN NEW; 
        
           END; $BODY$

ponceta · 2024-09-27T06:53:20Z

@domi4484 we constructed these functions in the past with @3nids.

Could you exchange with him and find the better of the two worlds? Is this due to QGIS or SQLAlchemy cache?

cymed · 2024-09-27T07:03:30Z

@ponceta For now, we only Force3D the input situation geometry in

wastewater/plugin/teksi_wastewater/interlis/interlis_model_mapping/interlis_importer_to_intermediate_schema.py

Line 1932 in c7c7e36

situation3d_geometry=ST_Force3D(row.lage),

The smart thing to do imho is to force the sohlenkote as z value on the importer side. The input geometry should then be equal to the existing one, thus not fire the trigger. Same goes for cover

domi4484 · 2024-09-27T07:35:46Z

The smart thing to do imho is to force the sohlenkote as z value on the importer side. The input geometry should then be equal to the existing one, thus not fire the trigger. Same goes for cover

This sounds the more correct way to me. Ill implement it like that.
But still I don't completely understand why we keep both Z and bottom_level in TWW. Are there situation where the values should not be equal?

cymed · 2024-09-27T10:58:18Z

Are there situation where the values should not be equal?

There is no such case. The only reason we have both values is that most users edit in 2.5d while the geometry is 3d and the attribute setup is mirroring the 2.5d INTERLIS structure

ponceta · 2024-09-27T12:20:20Z

Altitude can come from geometry (3D point import) or attribute (bottom level). That's the best solution we found yet to have both approach working.

I'm open to any suggestions on how to do it better. ;-)

domi4484 · 2024-09-29T19:24:29Z

I'm open to any suggestions on how to do it better. ;-)

It's just that it can be confusing to have redundant data in the DB. Another possibility would be to remove the quote and bottom_level columns and have a view to expose the Z coordinate as quote/bottom_level. But I am not an expert at all at postgres, maybe is not worth adding additional complexity to the model.

Now the suggestion from @cymed are implemented. @ponceta it would be nice if you can test again.

reach (length_effective)

This still looks as modified, can it be that a trigger recompute this to a different value? In that case I don't now what we could do to handle that on import

sjib · 2024-09-30T06:33:58Z

Altitude can come from geometry (3D point import) or attribute (bottom level). That's the best solution we found yet to have both approach working.

I'm open to any suggestions on how to do it better. ;-)

Some thoughts from my side:

I would like to suggest the following principals:
a) An INTERLIS import should never change any dataset without the user knowing that anything is beeing changed and have the possibility to deny it.
b) INTERLIS transfer is still based on 2.5 D - so I suggest that the 2.5 D attributes (levels) are treated as the superior information and z-information in 3D geometries is updated in the DB based on that information if it is conflicting. Else we risk to loose information on the import process.

c) As we are coming from the 2.5 D world and are still on the way towards the 3D modelling we should (re-)define some hierarchy on how information is changed:
We have the following 3D attributes with overlapping information:

cover.geometry3D_geometry and cover.level
wastewater_node.geometry3D_geometry and wastewater_node.bottom_level
reach.progression3D and reach_point.level (from and to)

May be we have to distinguish between

editing and adding new information in TEKSI and (where we have the triggers that make sure we are in sync always)
the INTERLIS import process? (where we have follow rule b)

And may be we have to have a tool that updates data to a stage so that we do not have conflicting information in the DB (like level exists, but respective z-value is missing or set to 0.00).
That would be good to have that also in qgep, so that we first solve those conflicts before going for an INTERLIS export.

For the migration process qgep to tww we have the intermediary points in progression3D that we are not able to transfer with INTERLIS - so we should treat them in an extra issue.

Hope this helps

cymed · 2024-09-30T07:25:44Z

I would like to suggest the following principals:
a) An INTERLIS import should never change any dataset without the user knowing that anything is being changed and have the possibility to deny it.

Except Attributes introduced by TWW that are currently set to NULL

b) INTERLIS transfer is still based on 2.5 D - so I suggest that the 2.5 D attributes (levels) are treated as the superior information and z-information in 3D geometries is updated in the DB based on that information if it is conflicting. Else we risk to loose information on the import process.

On Import I agree. On Editing I think we should use the Z of the node, transfer it into the level field, and allow overruling it.

c) As we are coming from the 2.5 D world and are still on the way towards the 3D modelling we should (re-)define some hierarchy on how information is changed: We have the following 3D attributes with overlapping information:
* cover.geometry3D_geometry and cover.level

* wastewater_node.geometry3D_geometry and wastewater_node.bottom_level

* reach.progression3D and reach_point.level (from and to)
May be we have to distinguish between
1. editing and adding new information in TEKSI and (where we have the triggers that make sure we are in sync always)

2. the INTERLIS import process? (where we have follow rule b)

From a DB Admin perspective it is smarter to implicitly store 2.5D-values in the Z coordinate of the point and expose them via a view (Single PointZ of truth). We lose no information, but an attribute. This ensures we have no contradictory information

And may be we have to have a tool that updates data to a stage so that we do not have conflicting information in the DB (like level exists, but respective z-value is missing or set to 0.00). That would be good to have that also in qgep, so that we first solve those conflicts before going for an INTERLIS export.

This is only necessary if we continue to save duplicate data

For the migration process qgep to tww we have the intermediary points in progression3D that we are not able to transfer with INTERLIS - so we should treat them in an extra issue.

Hope this helps

ponceta · 2024-09-30T08:17:01Z

This is great improvement to the actual plugin and solves many issues. Thank you a lot @domi4484

@cymed @sjib I'll open several issues for remaining tasks and opportunities. I let you do the same if you feel additionnal work on the plugin is required.

cymed · 2024-10-01T08:38:19Z

@domi4484 does this functionality base on primary keys to check for updates? In the AG-xx extensions we work with views, which means we have no primary keys.

domi4484 · 2024-10-02T06:12:34Z

@cymed it tries to get an existing row by obj_id. If we can select one its an update, otherwise a new object

domi4484 added 2 commits September 11, 2024 16:50

Test interlis update

3688d91

Update working but not for geometries

5fe0d28

domi4484 changed the title ~~Test interlis update~~ Interlis update Sep 11, 2024

cymed assigned domi4484 Sep 12, 2024

cymed added the INTERLIS About INTERLIS exchange format (import / export) label Sep 12, 2024

For geometries as welll

9d30c41

domi4484 marked this pull request as ready for review September 17, 2024 18:45

Pimp up import dialog

541a34b

domi4484 force-pushed the interlisUpdate branch from 2c1a269 to 541a34b Compare September 17, 2024 21:39

domi4484 requested a review from ponceta September 17, 2024 21:48

Compute WKB earlier

67625cb

ponceta added the plugin Concerns the wastewater plugin label Sep 24, 2024

Special handling for more time and float columns

ea279d0

Check for already existing re_... instances

Use 2.5D quote for geometry on Interlis import

d3fda64

Repristinate correct showing of status NEW

6c46808

ponceta approved these changes Sep 30, 2024

View reviewed changes

ponceta merged commit 44f630e into main Sep 30, 2024
4 checks passed

ponceta deleted the interlisUpdate branch September 30, 2024 08:14

domi4484 mentioned this pull request Oct 5, 2024

INTERLIS Import: Data in re_tables is ducplicated when re-importing #365

Closed

2 tasks

cymed mentioned this pull request Jan 31, 2025

fix finding updated/modified entries on import wapaAdmin/tww2ag6496#90

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Interlis update #414

Interlis update #414

domi4484 commented Sep 11, 2024 •

edited

Loading

cymed commented Sep 12, 2024

domi4484 commented Sep 12, 2024

ponceta commented Sep 18, 2024

ponceta commented Sep 18, 2024 •

edited

Loading

ponceta commented Sep 18, 2024

domi4484 commented Sep 20, 2024

ponceta commented Sep 20, 2024 •

edited

Loading

ponceta commented Sep 20, 2024

ponceta commented Sep 20, 2024 •

edited

Loading

domi4484 commented Sep 26, 2024

cymed commented Sep 27, 2024

ponceta commented Sep 27, 2024

cymed commented Sep 27, 2024 •

edited

Loading

domi4484 commented Sep 27, 2024

cymed commented Sep 27, 2024

ponceta commented Sep 27, 2024

domi4484 commented Sep 29, 2024

sjib commented Sep 30, 2024

cymed commented Sep 30, 2024

ponceta commented Sep 30, 2024

cymed commented Oct 1, 2024

domi4484 commented Oct 2, 2024

Interlis update #414

Interlis update #414

Conversation

domi4484 commented Sep 11, 2024 • edited Loading

cymed commented Sep 12, 2024

domi4484 commented Sep 12, 2024

ponceta commented Sep 18, 2024

ponceta commented Sep 18, 2024 • edited Loading

ponceta commented Sep 18, 2024

domi4484 commented Sep 20, 2024

ponceta commented Sep 20, 2024 • edited Loading

ponceta commented Sep 20, 2024

ponceta commented Sep 20, 2024 • edited Loading

domi4484 commented Sep 26, 2024

cymed commented Sep 27, 2024

ponceta commented Sep 27, 2024

cymed commented Sep 27, 2024 • edited Loading

domi4484 commented Sep 27, 2024

cymed commented Sep 27, 2024

ponceta commented Sep 27, 2024

domi4484 commented Sep 29, 2024

sjib commented Sep 30, 2024

cymed commented Sep 30, 2024

ponceta commented Sep 30, 2024

cymed commented Oct 1, 2024

domi4484 commented Oct 2, 2024

domi4484 commented Sep 11, 2024 •

edited

Loading

ponceta commented Sep 18, 2024 •

edited

Loading

ponceta commented Sep 20, 2024 •

edited

Loading

ponceta commented Sep 20, 2024 •

edited

Loading

cymed commented Sep 27, 2024 •

edited

Loading