Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Database: add constraint for unicity of CRS and operation names #4071

Merged
merged 3 commits into from
Mar 9, 2024

Conversation

rouault
Copy link
Member

@rouault rouault commented Mar 7, 2024

More exactly on projected_crs, vertical_crs, compound_crs, helmert_transformation, grid_transformation, other_transformation and concatenated_operation tables.

Bump database layout version to 1.4

CC @jjimenezshaw : triggered by our exchange with IOGP

@kbevers I've noticed a few duplicates of transformation with same names in the NKG namespace. Is that expected or copy&paste errors ?

+        AND NEW.name != 'NKG_ETRF00 to [email protected]' -- NKG:P1_2008_EE and NKG:P1_2008_FI have the same name
+        AND NEW.name != 'NKG_ETRF14 to [email protected]' -- NKG:PAR_2020_EE and NKG:PAR_2020_FI have the same name
+        AND NEW.name != 'NKG_ETRF14 to [email protected]' -- NKG:PAR_2020_NO and NKG:NKG_ETRF14_ETRF93_2000 have the same name
+        AND NEW.name != '[email protected] to [email protected]' -- NKG:ETRF96_2000_TO_ETRF96_1997_56 and NKG:EE_2020_INTRAPLATE have the same name
+        AND NEW.name != '[email protected] to [email protected]' -- NKG:ETRF93_2000_TO_ETRF93_1995 and NKG:NO_2020_INTRAPLATE have the same name
+        AND NEW.name != '[email protected] to [email protected]' -- NKG:ETRF92_2000_TO_ETRF92_1994 and NKG:DK_2020_INTRAPLATE have the same name
+        AND NEW.name != '[email protected] to [email protected]' -- NKG:ETRF96_2000_TO_ETRF96_1997 AND NKG:FI_2020_INTRAPLATE have the same name
+        AND NEW.name != '[email protected] to [email protected]' -- NKG:ETRF97_2000_TO_ETRF97_1999 and NKG:SE_2020_INTRAPLATE have the same name

@rouault rouault added this to the 9.5.0 milestone Mar 7, 2024
More exactly on projected_crs, vertical_crs, compound_crs,
helmert_transformation, grid_transformation, other_transformation and
concatenated_operation tables.

Bump database layout version to 1.4
@kbevers
Copy link
Member

kbevers commented Mar 7, 2024

@kbevers I've noticed a few duplicates of transformation with same names in the NKG namespace. Is that expected or copy&paste errors ?

Expected yes, but probably not necessary. Is it problematic that they are the same?

@rouault
Copy link
Member Author

rouault commented Mar 7, 2024

Is it problematic that they are the same?

Mostly a matter of clarity. Unless the transformations do the same thing (and here it is clearly not the case as at least some of them are Helmert vs grid-based, or using different grids), I believe they should have different names. For example "projinfo -s X -t Y" reports a coordinate operation name for each operation it synthetizes and being able to 1-1 match the name to an elementary operation can make understanding easier"

@kbevers
Copy link
Member

kbevers commented Mar 7, 2024

I haven't looked too closely but there's probably quite a bit of doing the same thing even though it looks different. Re-use is a big feature of this set of transformations. But I see your point and will give this a review when I have time for it in a couple of weeks.

@jjimenezshaw
Copy link
Contributor

If I understand correctly, it is also blocking in case an entry (crs, transformation, ...) has the same name between different authorities. For instance, if there is a NAD83 in EPSG and also a NAD83 in a different authority (and then ISO come to my mind. It will repeat many names... I hope). Is that right? Do we want that?

@rouault
Copy link
Member Author

rouault commented Mar 7, 2024

Is that right? Do we want that?

We have a mind connection, because I've just pushed a commit, that restricts the unicity check to only the objects for the authorities we provide (thanks to a new builtin_authorities table). That way it helps us to detect non-uniqueness in what we ship, while avoiding potentially breaking users that would extend the database with their own authority and where they wouldn't want those unicity checks.

@rouault
Copy link
Member Author

rouault commented Mar 7, 2024

That way it helps us to detect non-uniqueness in what we ship

If we add a ISO registry with duplicated entries, we might revisit the current check. We could for example allow duplicate names when one objects belongs to EPSG and the other one to ISO, but not within the same authority

@busstoptaktik
Copy link
Member

If we add a ISO registry with duplicated entries, we might revisit the current check. We could for example allow duplicate names when one objects belongs to EPSG and the other one to ISO, but not within the same authority

If I understand Roger Lott of the IOGP geodetic committee correctly, EPSG, as a matter of policy, mirrors the contents of the ISO register, so adding ISO should not be necessary/will add duplicates

@jjimenezshaw
Copy link
Contributor

If I understand Roger Lott of the IOGP geodetic committee correctly, EPSG, as a matter of policy, mirrors the contents of the ISO register, so adding ISO should not be necessary/will add duplicates

Good to know.
About including it or not, will a user desire to refer to their CRS with the ISO id, regardless if there is an equivalent in EPSG? But that's for another discussion, not this PR.

@rouault rouault merged commit 4d4f646 into OSGeo:master Mar 9, 2024
23 checks passed
kbevers added a commit to kbevers/PROJ that referenced this pull request Mar 21, 2024
A consequence of new database constrains introduced in OSGeo#4071.
kbevers added a commit to kbevers/PROJ that referenced this pull request Mar 24, 2024
A consequence of new database constrains introduced in OSGeo#4071.
kbevers added a commit to kbevers/PROJ that referenced this pull request Apr 2, 2024
A consequence of new database constrains introduced in OSGeo#4071.
kbevers added a commit to kbevers/PROJ that referenced this pull request Apr 4, 2024
A consequence of new database constrains introduced in OSGeo#4071.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants