search

3liz / qgis-pgmetadata-plugin

QGIS Plugin to manage some metadata from PostgreSQL layer
GNU General Public License v2.0
12 stars 10 forks source link

add support for views #66

Closed tschuettenberg closed 3 years ago

tschuettenberg commented 3 years ago
tschuettenberg commented 3 years ago

I'am afraid I'm not smart enough to understand what/where exactly the error is on the migration test.

Gustry commented 3 years ago

The easy error was the difference in the comment. You have updated it in the new install, but not in the migration.

But then, I agree, this test is pretty picky, sorry. The SQL in the database must be exactly the same between running migration and a fresh install to have the test OK.

I have fixed the test on the PR. I agree, it's easier when I run the test in docker on my machine ;-) So don't worry fo that.

I will wait for @mdouchin I can try to add a quick test in Python

Wondering about side effects ? Materialized views ?

Did you try the "recompute values" @tschuettenberg ? CF https://docs.3liz.org/qgis-pgmetadata-plugin/processing/#recompute-values-in-the-dataset-table

Thanks for your contribution !

tschuettenberg commented 3 years ago

Thanks for helping, I might have noticed the difference, but did nor realize the effect. :-)

Did try and "recompute values" is working.

But I just realized, that the comments only work for aktual tables, becuase the sql in the function update_postgresql_table_comment certaily is COMMENT ON TABLE.... No idea yet on how to generalize this, but the needed table_type to concat the sql could eventually be taken from information_schema.tables.

Gustry commented 3 years ago

@tschuettenberg I tried to gave you the right to launch the CI, for you only. Otherwise, I needed to approve everytime until you make your first merged PR into the repository.

tschuettenberg commented 3 years ago

please excuse my amateurish trial-and-error-style, I'm learning.

Gustry commented 3 years ago

No worries, it's alright !

Let us know when you have finished on your side ? What about the comment on views ?

tschuettenberg commented 3 years ago

I think I'm done with adding support for views. The views are now recognized by all pgmetadata views, also they get comments by the trigger function, see here https://github.com/3liz/qgis-pgmetadata-plugin/pull/66/commits/ca0851577dcedae649993c6d9a833e3fd9a10713, https://github.com/3liz/qgis-pgmetadata-plugin/pull/66/commits/dcbc931fe876a37fe714ac3cced2ec7ca22bac50 . In order to achieve this v_table_comment_from_metadata got extended in https://github.com/3liz/qgis-pgmetadata-plugin/pull/66/commits/de1b3cec8be563eb972aff1b47d38e39da8e4151 .

mdouchin commented 3 years ago

Thanks a lot for your work ! It seems indeed ok to me for the SQL part, and I think this will also handle the materialized views. I have also checked the triggers functions used to update the calculated fields when a line is inserted or updated in pgmetadata.dataset. It seems all covered :)

Gustry commented 3 years ago

Thanks, see #72