You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by "thomasmueller (via GitHub)" <gi...@apache.org> on 2023/10/12 14:28:45 UTC
[PR] OAK-10490 Suggest queries return duplicate entries if prefetch is enabled [jackrabbit-oak]
thomasmueller opened a new pull request, #1148:
URL: https://github.com/apache/jackrabbit-oak/pull/1148
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10490 Suggest queries return duplicate entries if prefetch is enabled [jackrabbit-oak]
Posted by "thomasmueller (via GitHub)" <gi...@apache.org>.
thomasmueller commented on code in PR #1148:
URL: https://github.com/apache/jackrabbit-oak/pull/1148#discussion_r1357913024
##########
oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndex.java:
##########
@@ -465,6 +465,9 @@ public void remove() {
@Override
public IndexRow next() {
final IndexRow pathRow = pathCursor.next();
+ // we need to copy the reference to the current row here,
+ // otherwise all the returned IndexRows might reference the same row
+ final FulltextResultRow currentRow = currentRowInPathIterator;
return new IndexRow() {
@Override
Review Comment:
Yes, that would probably be cleaner... My plan was to create the most simple fix, but this seems better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10490 Suggest queries return duplicate entries if prefetch is enabled [jackrabbit-oak]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #1148:
URL: https://github.com/apache/jackrabbit-oak/pull/1148#issuecomment-1761474236
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148)
[](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&metric=new_coverage&view=list) [54.5% Coverage](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&metric=new_duplicated_lines_density&view=list)
 The version of Java (11.0.20.1) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/)
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=sonarcloud-welcome)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10490 Suggest queries return duplicate entries if prefetch is enabled [jackrabbit-oak]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #1148:
URL: https://github.com/apache/jackrabbit-oak/pull/1148#issuecomment-1761289708
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148)
[](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&metric=new_coverage&view=list) [52.4% Coverage](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&metric=new_duplicated_lines_density&view=list)
 The version of Java (11.0.20.1) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/)
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=sonarcloud-welcome)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10490 Suggest queries return duplicate entries if prefetch is enabled [jackrabbit-oak]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #1148:
URL: https://github.com/apache/jackrabbit-oak/pull/1148#issuecomment-1759892749
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148)
[](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1148&metric=new_duplicated_lines_density&view=list)
 The version of Java (11.0.20.1) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10490 Suggest queries return duplicate entries if prefetch is enabled [jackrabbit-oak]
Posted by "nfsantos (via GitHub)" <gi...@apache.org>.
nfsantos commented on code in PR #1148:
URL: https://github.com/apache/jackrabbit-oak/pull/1148#discussion_r1357019222
##########
oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndex.java:
##########
@@ -465,6 +465,9 @@ public void remove() {
@Override
public IndexRow next() {
final IndexRow pathRow = pathCursor.next();
+ // we need to copy the reference to the current row here,
+ // otherwise all the returned IndexRows might reference the same row
+ final FulltextResultRow currentRow = currentRowInPathIterator;
return new IndexRow() {
@Override
Review Comment:
To avoid unintentional capture of state from the parent class, you could create a static subclass of IndexRow and declare an explicit constructor. This requires the caller here to explicitly provide all the arguments that are needed and to copy them into the new object of IndexRow, thus avoiding any accidental use of mutable state from the parent class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10490 Suggest queries return duplicate entries if prefetch is enabled [jackrabbit-oak]
Posted by "nfsantos (via GitHub)" <gi...@apache.org>.
nfsantos commented on code in PR #1148:
URL: https://github.com/apache/jackrabbit-oak/pull/1148#discussion_r1358022250
##########
oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndex.java:
##########
@@ -465,67 +470,13 @@ public void remove() {
@Override
public IndexRow next() {
final IndexRow pathRow = pathCursor.next();
- return new IndexRow() {
-
- @Override
- public boolean isVirtualRow() {
- return currentRow.isVirutal;
- }
-
- @Override
- public String getPath() {
- String sub = pathRow.getPath();
- if (isVirtualRow()) {
- return sub;
- } else if (!"".equals(pathPrefix) && PathUtils.denotesRoot(sub)) {
- return pathPrefix;
- } else if (PathUtils.isAbsolute(sub)) {
- return pathPrefix + sub;
- } else {
- return PathUtils.concat(pathPrefix, sub);
- }
- }
- @Override
- public PropertyValue getValue(String columnName) {
- // overlay the score
- if (QueryConstants.JCR_SCORE.equals(columnName)) {
- return PropertyValues.newDouble(currentRow.score);
- }
- if (QueryConstants.REP_SPELLCHECK.equals(columnName) || QueryConstants.REP_SUGGEST.equals(columnName)) {
- return PropertyValues.newString(currentRow.suggestion);
- }
- if (QueryConstants.OAK_SCORE_EXPLANATION.equals(columnName)) {
- return PropertyValues.newString(currentRow.explanation);
- }
- if (columnName.startsWith(QueryConstants.REP_EXCERPT)) {
- String excerpt = currentRow.excerpts.get(columnName);
- if (excerpt != null) {
- return PropertyValues.newString(excerpt);
- }
- }
- if (columnName.startsWith(QueryConstants.REP_FACET)) {
- try {
- List<Facet> facets = currentRow.getFacets(numberOfFacets, columnName);
- if (facets != null) {
- JsopWriter writer = new JsopBuilder();
- writer.object();
- for (Facet f : facets) {
- writer.key(f.getLabel()).value(f.getCount());
- }
- writer.endObject();
- return PropertyValues.newString(writer.toString());
- }
- } catch (IOException | RuntimeException e) {
- LOG.warn(e.getMessage());
- LOG.debug(e.getMessage(), e);
- throw new RuntimeException(e);
- }
- }
- return pathRow.getValue(columnName);
- }
+ // we need to copy the reference to the current row here,
+ // otherwise all the returned IndexRows might reference the same row
+ // (and then we use _this_ row in the IndexRow class below)
Review Comment:
This comment may not be needed anymore. Or else rephrase it because the copy of the reference is now implicit, it's just the standard call-by-value Java behaviour so maybe there is no need for comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10490 Suggest queries return duplicate entries if prefetch is enabled [jackrabbit-oak]
Posted by "thomasmueller (via GitHub)" <gi...@apache.org>.
thomasmueller merged PR #1148:
URL: https://github.com/apache/jackrabbit-oak/pull/1148
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org