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

Refactor queries for fetching&counting the geometries #14

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

hannahbast
Copy link
Member

@hannahbast hannahbast commented Aug 7, 2024

There are now three predefined queries for fetching the geometries from a SPARQL endpoint: one using geo:asWKT (the default), one using wdt:P525 (used for backends where the name starts with wikidata), and one using a SERVICE request to Wikidata (used for backends where the name starts with dblp). Apart from these differences, the queries have the exact same structure. The query for counting the number of geometries is now automatically derived from the fetching query, in a way that is as efficient as possible (in particular, more efficient than before).

@hannahbast hannahbast requested a review from patrickbr August 7, 2024 15:20
@hannahbast hannahbast changed the title Simpler and more efficient fetch-geometry and count queries Generic and more efficient COUNT query Aug 7, 2024
Copy link
Member

@patrickbr patrickbr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @hannahbast, this looks good!

However, I don't think it is a good idea to set QUERY_ASGEOMETRY as the default return value in getQuery() (also, I think it should be named QUERY_HASGEOMETRY). AFAIK, this query is only relevant for historic osm2rdf outputs in which objects are linked directly to their WKT string via geo:hasGeometry. This is not following the GeoSPARQL standard. IMHO the default should be QUERY_ASWKT, as this follows the standard.

This default fallback is already present in the current master and (as I just realized) is the reason I couldn't use master against OHM this afternoon during our meeting: in the old master, OHM wasn't handled explicitly, and the fallback query (equivalent to the query now called QUERY_ASGEOMETRY) then returned the intermediate geometry objects, not the WKT strings. Essentially, RDF data that follows the GeoSPARQL standard but is not explicitly handled in getQuery() is currently unusable with master, and also still with this PR.

We should drop QUERY_ASGEOMETRY completely, make QUERY_ASWKT the default in getQuery() and also remove the explicit handling of OSM and OHM in getQuery().

@hannahbast hannahbast changed the title Generic and more efficient COUNT query Refactor queries for fetching the geometries Sep 24, 2024
@hannahbast hannahbast changed the title Refactor queries for fetching the geometries Refactor queries for fetching&counting the geometries Sep 24, 2024
@hannahbast hannahbast force-pushed the improve-get-geometries-queries branch from a071670 to e48e53b Compare September 24, 2024 18:30
@patrickbr patrickbr merged commit 9713f2b into master Sep 25, 2024
6 checks passed
@patrickbr patrickbr deleted the improve-get-geometries-queries branch September 25, 2024 09:27
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.

2 participants