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

Implement REPLACE #1085

Merged
merged 4 commits into from
Sep 6, 2023
Merged

Conversation

joka921
Copy link
Member

@joka921 joka921 commented Sep 6, 2023

The REPLACE function is now implemented. Note the following details:

  1. According to the standard, REPLACE takes an optional fourth argument for the flags. This is not yet supported. However, the flags can equivalently be prepended to the regular expression. For example, to get a case-insensitive regular expression, prepend (?i). The new RegexValueGetter is not yet used in FILTER clauses, where prefix regular expressions are treated specially for efficiency reasons.

  2. For constant values that are repeated multiple times (for each result row), the value is now evaluated only once and then repeated. In particular, in a REPLACE with a constant regular expression (the typical case), the regular expression is now compiled only once. This also avoids copying a constant string (for example, in a call to CONCAT) many times.

Comment on lines 173 to 176
struct RegexValueGetter {
std::unique_ptr<re2::RE2> operator()(SingleExpressionResult auto&& input,
const EvaluationContext* context) const
requires requires { StringValueGetter{}(AD_FWD(input), context); } {
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO Comment the regexValueGetter and maybe move it somewhere.
also TODO Can we implement the Ordinary regex expression as an NARY-Operation with specialized functions?

@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Patch coverage: 87.50% and project coverage change: +0.33% 🎉

Comparison is base (bc500ba) 79.89% compared to head (4d21171) 80.22%.
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1085      +/-   ##
==========================================
+ Coverage   79.89%   80.22%   +0.33%     
==========================================
  Files         277      279       +2     
  Lines       26426    26792     +366     
  Branches     3266     3301      +35     
==========================================
+ Hits        21113    21495     +382     
+ Misses       3961     3937      -24     
- Partials     1352     1360       +8     
Files Changed Coverage Δ
src/engine/sparqlExpressions/NaryExpression.h 100.00% <ø> (ø)
src/parser/sparqlParser/SparqlQleverVisitor.h 100.00% <ø> (ø)
...ine/sparqlExpressions/SparqlExpressionGenerators.h 70.37% <75.00%> (-0.22%) ⬇️
...e/sparqlExpressions/SparqlExpressionValueGetters.h 91.35% <85.71%> (-0.54%) ⬇️
src/engine/sparqlExpressions/StringExpressions.cpp 95.98% <100.00%> (+0.30%) ⬆️
src/parser/sparqlParser/SparqlQleverVisitor.cpp 92.01% <100.00%> (+0.28%) ⬆️

... and 18 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@hannahbast hannahbast left a comment

Choose a reason for hiding this comment

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

So great to finally have this, minor changes left

@hannahbast hannahbast marked this pull request as ready for review September 6, 2023 19:26
@hannahbast hannahbast changed the title Implement the REPLACE expression Implement REPLACE Sep 6, 2023
@hannahbast hannahbast merged commit 126a39d into ad-freiburg:master Sep 6, 2023
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@joka921 joka921 deleted the replace-expression branch December 7, 2023 09:18
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