Discovery of BAGs OF: take into account BAGs OF when no mapping meta attr is present #157
Conversation
…mapping=ARRAY meta attr
…th a BAG OF with no mapping meta attr
… a mapping=ARRAY meta attr
gacarrillor
force-pushed
the
fix_suppress_bag_of_catalogue
branch
from
8d6988d to
09ed6a4
Compare
…pping=ARRAY meta attr set
signedav
left a comment
signedav
left a comment
There was a problem hiding this comment.
Thanks. I don't really get it yet. Maybe you can help me with some comments. I expected it to be simpler, but I guess I don't see all the impacts...
| ON LOWER(meta_attrs_cardinality_min.ilielement) = LOWER(cname.iliname||'.'||cprop.columnname) AND meta_attrs_cardinality_min.attr_name = 'ili2db.ili.attrCardinalityMin' | ||
| LEFT JOIN T_ILI2DB_META_ATTRS as meta_attrs_cardinality_max | ||
| ON LOWER(meta_attrs_cardinality_max.ilielement) = LOWER(cname.iliname||'.'||cprop.columnname) AND meta_attrs_cardinality_max.attr_name = 'ili2db.ili.attrCardinalityMax' | ||
| WHERE cprop.tag = 'ch.ehi.ili2db.foreignKey' AND meta_attrs_array.attr_value = 'ARRAY' |
There was a problem hiding this comment.
What I don't understand is, why it was not sufficient just to return the meta_attrs_array.attr_value as mapping_type and then check it in the suppress_catalogue_reference_layers? Why is the union select needed?
There was a problem hiding this comment.
Fair question, taking e.g., KbS_LV95_V1_3 model from tests:
-
Just returning
meta_attrs_array.attr_value as mapping_typeand commentingmapping=ARRAYinWHERE:- 19 rows returned. I think the other relations are also included.
- Only the rows with
mapping=ARRAYhave proper cardinality, the rest hasNULLvalues for cardinalities.
-
Using the
UNION ALL:- 4 rows returned. One per BAG OF in the model.
- Each row includes proper cardinality.
Nonetheless, I guess the UNION ALL query looks a bit harder to maintain. I'm reverting to keep it simple.
|
@signedav, marking as draft since I prepare some changes here, thanks for the review! |
gacarrillor
changed the title
gacarrillor
force-pushed
the
fix_suppress_bag_of_catalogue
branch
from
e64a317 to
b61de32
Compare
|
@signedav, I've added a commit on top, reverting to the original simple SQL query (tweaked). I've added two custom pytest marks so that we can filter bag_of, or catalogue related tests easily. This needed a If we agree on merging it, we should squash commits, since the first ones have been reverted. |
signedav
left a comment
signedav
left a comment
There was a problem hiding this comment.
Thanks a lot. Some suggestions. But otherwise good to merge...
|
Forgot to squash like you intended. Sorry. |
From
DbConnector.get_bags_of_info()we now get an additional field that allows us to differentiate the type of mapping being used. It can be an empty string if no mapping is present.Includes tests.
Fix opengisch/QgisModelBaker#1082
Note that this does not include the corresponding fix for ili2db v3.