Enable pre-filtering (using PrefilterExpressionIndex) of Date values over expression YEAR().
#1820
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1820 +/- ##
==========================================
+ Coverage 90.18% 90.20% +0.02%
==========================================
Files 400 400
Lines 38440 38526 +86
Branches 4306 4318 +12
==========================================
+ Hits 34666 34753 +87
- Misses 2479 2480 +1
+ Partials 1295 1293 -2 ☔ View full report in Codecov by Sentry. |
Conformance check passed ✅No test result changes. |
|
| } else if constexpr (std::is_same_v<std::decay_t<T>, | ||
| LocalVocabEntry>) { | ||
| throw std::runtime_error(absl::StrCat( | ||
| "Provided Literal or Iri with value: ", | ||
| value.asLiteralOrIri().toStringRepresentation(), | ||
| ". This is an invalid reference value for filtering date " | ||
| "values over expression YEAR. Please provide an integer " | ||
| "value as reference year.")); | ||
| } | ||
| throw std::runtime_error( |
There was a problem hiding this comment.
Make this else {static_assert} instead of else if ...,
because the final throw is unreachable and because the code becomes more robust that way.
| [](const IdOrLocalVocabEntry& referenceValue) { | ||
| return std::visit( | ||
| []<typename T>(const T& value) -> ValueId { | ||
| if constexpr (std::is_same_v<std::decay_t<T>, ValueId>) { |
There was a problem hiding this comment.
use ad_utility::SimilarTo instead of same<decay<
(shorter and more expressive.
| // Use for testing only. The definition here is necessary to call | ||
| // `makePrefilterYearImpl` with an invalid `CompOp` argument given that it is |
There was a problem hiding this comment.
Do you mean This function is public s.t. we can test the corner case of...?
| // needs to be applied for the creation of `PrefilterExpression`. | ||
| static std::optional<std::pair<Variable, bool>> getOptVariableAndIsYear( | ||
| const SparqlExpression* child) { | ||
| // (1) The direct child already contains the Variable. |
There was a problem hiding this comment.
Simpler workflow:
bool isYear = false;
if (child->isYearExpression) {
isYear = true;
child = child->child; // pseudocode.
}
... // need to check teh isVariable only once.
There was a problem hiding this comment.
(if overwriting the varaibel child doesn't work, introduce some additional variable.)
No description provided.