GH-1688 Fix possible incomplete results with service enhaner caching + bulk#1689
GH-1688 Fix possible incomplete results with service enhaner caching + bulk#1689afs merged 1 commit intoapache:mainfrom
Conversation
| */ | ||
| @Test | ||
| public void testEmptyResultSetContribution() { | ||
| String dataStr = String.join(StandardSystemProperty.LINE_SEPARATOR.value(), |
There was a problem hiding this comment.
Not critical but why not "\n"? SPARQL is a web language.
There was a problem hiding this comment.
Isn't it the case that on Windows or Mac a plain "\n" could lead to incorrect formatting when System.out.println-ing?
If you prefer "\n" then I am also fine with it.
...ncer/src/test/java/org/apache/jena/sparql/service/enhancer/impl/TestServiceEnhancerMisc.java
Outdated
Show resolved
Hide resolved
...ncer/src/test/java/org/apache/jena/sparql/service/enhancer/impl/TestServiceEnhancerMisc.java
Outdated
Show resolved
Hide resolved
...ncer/src/test/java/org/apache/jena/sparql/service/enhancer/impl/TestServiceEnhancerMisc.java
Outdated
Show resolved
Hide resolved
Aklakan
force-pushed
the
se-fixes3
branch
3 times, most recently
from
7202822 to
0723aff
Compare
|
Overall this PR should be complete but I'd like @LorenzBuehmann to give this PR a final test himself tomorrow. |
|
I can confirm that the issue I spotted yesterday has been resolved when using the PR code. Thanks. |
...ncer/src/test/java/org/apache/jena/sparql/service/enhancer/impl/TestServiceEnhancerMisc.java
Outdated
Show resolved
Hide resolved
|
@LorenzBuehmann - thanks for checking - it's "draft" though. If in the next 2 hours that changes, I (as release manager) will consider it for 4.7.0. The release testing has already been done, bots have already run on different OS etc - it must be clean. |
…lved partial result sets of cache hits are empty
| */ | ||
| public class VarScopeUtils { | ||
|
|
||
| public static Map<Var, Var> reverseVarRenameMap(Collection<Var> vars) { |
There was a problem hiding this comment.
Moved these methods to the end of the class - probably they can be eventually removed.
| // Peek the next binding on the active iterator and verify that it maps to the current | ||
| // partition key | ||
| while (true) { | ||
| if (activeIter.hasNext()) { |
There was a problem hiding this comment.
Fix of indentation - changed 2 spaces to 4.
|
|
||
| // Handling of a null supOp - can that happen? | ||
| Set<Var> visibleSubOpVars = OpVars.visibleVars(baseSubOp); | ||
| this.visibleSubOpVarsScopedToNorm = VarScopeUtils.normalizeVarScopesGlobal(visibleSubOpVars); |
There was a problem hiding this comment.
This one-liner fixes a scoping issue @LorenzBuehmann found just after he reported that this PR works while trying variants of the query that led to this PR. Instead of cut-off results it may lead to missing columns.
apacheGH-1688 Fix possible incomplete results with service enhaner caching + bulk (cherry picked from commit 979b972)
3 participants
GitHub issue resolved #1688
Pull request Description: Fixes registration of an empty iterator in case a cache hit reveals an empty result set.
[ ] Documentation change and updates are provided for the Apache Jena websiteBy submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.
See the Apache Jena "Contributing" guide.