Add internal function ql:isGeoPoint
#1565
Conversation
The function returns true if and only if the argument is a WKT point. This can be checked efficiently by checking the datatype bits of the ID. Note that this function is not part of the GeoSPARQL standard. We add it because we need it for https://github.com/ad-freiburg/qlever-petrimaps to fetch alll WKT literals that are not points efficiently.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1565 +/- ##
==========================================
+ Coverage 89.06% 89.07% +0.01%
==========================================
Files 369 369
Lines 34237 34251 +14
Branches 3868 3870 +2
==========================================
+ Hits 30492 30508 +16
Misses 2481 2481
+ Partials 1264 1262 -2 ☔ View full report in Codecov by Sentry. |
Reason: the predicate is called `geo:asWKT`, so the new name looks more consistent with that.
There was a problem hiding this comment.
Thank you. Your code looks good. Two remarks:
- The GeoSPARQL standard does have a similar-purposed function
geof:geometryType(https://docs.ogc.org/is/22-047r1/22-047r1.html#_function_geofgeometrytype). But I agree that for the special use-case, your new function still makes a lot of sense, because an implementation ofgeof:geometryTypecould not skip reading the string literals from the vocabulary in the non-point case. - I feel like we have introduced quite an amount of additional features around the spatial topics. We should add some documentation, such that users potentially looking for such features can actually find them without looking into thousands of lines of code. This was on my todo-list anyway, so I can address it in another PR myself.
1. It was not a good idea to make this a `geof:` function for two reasons: First, it's not a standard `geof:` function. Second, and worse, the function does not actually check whether the argument is a WKT point, it only checks whether the argument is a WKT point that is stored in the `Id`. There are also WKT points that are not stored in the `Id` for various reasons (notably, when the coordinates are out of range). It is therefore now a QLever-internal function with the name `ql:isGeoPoint`, following the name of the `Datatype`. 2. This is our first QLever-internal function, so far we only had QLever-internal predicates (like `ql:has-predicate` or `ql:langtag`) and entities (like `ql:@en`, which technicall is not a valid IRI, but that is not relevant here). This required new constants in `Constants.h`, which prompted me to give better names to the other constants there related to QLever-internal stuff. This, in turn, required renaming in quite a few files. It looks like a lot, but almost all of it is really just straightforward renaming.
|
@ullingerc Thanks for your comment. I thought about the full Regarding the documentation, that is a great idea, and I see that you have have already started with #1581 . I will have a look at that and comment over there. |
hannahbast
changed the title
geof:isWktPointql:isGeoPoint
Conformance check passed ✅No test result changes. |
|
The function returns true if and only if the argument is a
ValueIdwith datatypeDatatype::GeoPoint, that is, it represents a WKT point and the value is encoded directly in theId. This is useful for https://github.com/ad-freiburg/qlever-petrimaps, which does not have to pre-fetch and store the literals for such points.It is important to note that there are WKT points for which this function evaluates to false. For example, points where the coordinates are out of range, are not encoded in the
Idbut have an entry in the vocabulary. Due to this QLever-specific semantics, we make this a QLever-internal function.This is our first QLever-internal function. So far we only had QLever-internal predicates (like
ql:has-predicateorql:langtag) and Qlever-internal entities (likeql:@en). Used that occasion to make the names of several of the QLever-internal constants in the code more meaningful and consistent.