You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "mkhludnev (via GitHub)" <gi...@apache.org> on 2023/02/23 22:01:37 UTC
[GitHub] [solr] mkhludnev opened a new pull request, #1378: SOLR-16585: examples test for match all paging
mkhludnev opened a new pull request, #1378:
URL: https://github.com/apache/solr/pull/1378
https://issues.apache.org/jira/browse/SOLR-16585
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] mkhludnev commented on a diff in pull request #1378: SOLR-16585: examples test for match all paging
Posted by "mkhludnev (via GitHub)" <gi...@apache.org>.
mkhludnev commented on code in PR #1378:
URL: https://github.com/apache/solr/pull/1378#discussion_r1118603679
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -571,6 +572,67 @@ public void testGetEmptyResults() throws Exception {
assertEquals(0, out.get(1).size());
}
+ @Test
+ public void testMatchAllPaging() throws Exception {
+ SolrClient client = getSolrClient();
+
+ // Empty the database...
+ client.deleteByQuery("*:*"); // delete everything!
+ if (random().nextBoolean()) {
+ client.commit();
+ }
+ // Add eleven docs
+ List<SolrInputDocument> docs = new ArrayList<>();
+ final int docsTotal = CommonParams.ROWS_DEFAULT + 1;
+ for (int i = 0; i < docsTotal; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", "id" + i);
+ doc.addField("name", "doc" + i);
+ doc.addField("price", "" + i);
+ docs.add(doc);
+ if (rarely()) {
+ client.add(docs);
+ client.commit();
+ docs.clear();
+ }
+ }
+ client.add(docs);
+ if (random().nextBoolean()) {
+ client.commit();
+ } else {
+ client.optimize();
+ }
+ final List<String> sorts = Arrays.asList("_docid_", "id", "name", "price", null);
+ Collections.shuffle(sorts, random());
+ final List<Integer> starts =
+ Arrays.asList(0, 1, 2, CommonParams.ROWS_DEFAULT, docsTotal, CommonParams.ROWS_DEFAULT + 2);
+ Collections.shuffle(starts, random());
+ for (String sort : sorts.subList(0, 1 + random().nextInt(sorts.size() - 1))) {
+ for (int start : starts.subList(0, 1 + random().nextInt(starts.size() - 1))) {
+ final SolrQuery query = new SolrQuery("*:*");
+ if (sort != null) {
+ query.setSort(sort, random().nextBoolean() ? SolrQuery.ORDER.asc : SolrQuery.ORDER.desc);
+ }
+ if (start > 0 || random().nextBoolean()) {
+ query.setStart(start);
+ }
+ if (usually()) {
+ query.setRows(CommonParams.ROWS_DEFAULT);
+ }
+ SolrDocumentList results = client.query(query).getResults();
+ assertEquals(docsTotal, results.getNumFound());
+ assertEquals(
+ "page from " + start,
+ Math.max(Math.min(CommonParams.ROWS_DEFAULT, docsTotal - start), 0),
+ results.size());
+ for (SolrDocument doc : results) {
+ assertTrue(doc.containsKey("id") && doc.containsKey("name") && doc.containsKey("price"));
+ break;
Review Comment:
Pardon. With `for(..) {.. break;}` I tried to say if not empty checking the first one, do nothing otherwise. I saves a few micros (I guess) in comparison to looping all five docs. Let it be.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] magibney commented on a diff in pull request #1378: SOLR-16585: examples test for match all paging
Posted by "magibney (via GitHub)" <gi...@apache.org>.
magibney commented on code in PR #1378:
URL: https://github.com/apache/solr/pull/1378#discussion_r1119310253
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -571,6 +572,68 @@ public void testGetEmptyResults() throws Exception {
assertEquals(0, out.get(1).size());
}
+ @Test
+ public void testMatchAllPaging() throws Exception {
+ SolrClient client = getSolrClient();
+
+ // Empty the database...
+ client.deleteByQuery("*:*"); // delete everything!
+ if (random().nextBoolean()) {
+ client.commit();
+ }
+ // Add eleven docs
+ List<SolrInputDocument> docs = new ArrayList<>();
+ final int docsTotal = CommonParams.ROWS_DEFAULT + 1;
+ for (int i = 0; i < docsTotal; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", "id" + i);
+ doc.addField("name", "doc" + i);
+ doc.addField("price", "" + i);
+ docs.add(doc);
+ if (rarely()) {
+ client.add(docs);
+ client.commit();
+ docs.clear();
+ }
+ }
+ client.add(docs);
+ if (random().nextBoolean()) {
+ client.commit();
+ } else {
+ client.optimize();
+ }
+ final List<String> sorts = Arrays.asList("_docid_", "id", "name", "price", null);
+ Collections.shuffle(sorts, random());
+ final List<Integer> starts =
+ Arrays.asList(0, 1, 2, CommonParams.ROWS_DEFAULT, docsTotal, CommonParams.ROWS_DEFAULT + 2);
+ Collections.shuffle(starts, random());
+ for (String sort : sorts.subList(0, 1 + random().nextInt(sorts.size() - 1))) {
+ for (int start : starts.subList(0, 1 + random().nextInt(starts.size() - 1))) {
+ final SolrQuery query = new SolrQuery("*:*");
+ if (sort != null) {
+ query.setSort(sort, random().nextBoolean() ? SolrQuery.ORDER.asc : SolrQuery.ORDER.desc);
+ }
+ if (start > 0 || random().nextBoolean()) {
+ query.setStart(start);
+ }
+ if (usually()) {
+ query.setRows(CommonParams.ROWS_DEFAULT);
+ }
Review Comment:
I see. I get that this block doesn't change the behavior -- so perhaps misleading that I commented on it. My point was that unless `rows` param is set artificially small (and it's always effectively `10` here), the results for all offsets should be within the queryResultCache window, so the first request for a given `sort` will construct a docList, and all subsequent requests for that `sort` will I think be entirely served from the queryResultCache.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] mkhludnev commented on a diff in pull request #1378: SOLR-16585: examples test for match all paging
Posted by "mkhludnev (via GitHub)" <gi...@apache.org>.
mkhludnev commented on code in PR #1378:
URL: https://github.com/apache/solr/pull/1378#discussion_r1116735330
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -571,6 +572,57 @@ public void testGetEmptyResults() throws Exception {
assertEquals(0, out.get(1).size());
}
+ @Test
+ public void testMatchAllPaging() throws Exception {
+ SolrClient client = getSolrClient();
+
+ // Empty the database...
+ client.deleteByQuery("*:*"); // delete everything!
+ if (random().nextBoolean()) {
+ client.commit();
+ }
+ // Add eleven docs
+ List<SolrInputDocument> docs = new ArrayList<>();
+ for (int i = 0; i < 11; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", "id" + i);
+ doc.addField("name", "doc" + i);
+ doc.addField("price", "" + i);
+ docs.add(doc);
+ if (rarely()) {
+ client.commit();
Review Comment:
Oh..gosh. Thanks for spotting it @cpoerschke .
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #1378: SOLR-16585: examples test for match all paging
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #1378:
URL: https://github.com/apache/solr/pull/1378#discussion_r1116715277
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -571,6 +572,57 @@ public void testGetEmptyResults() throws Exception {
assertEquals(0, out.get(1).size());
}
+ @Test
+ public void testMatchAllPaging() throws Exception {
+ SolrClient client = getSolrClient();
+
+ // Empty the database...
+ client.deleteByQuery("*:*"); // delete everything!
+ if (random().nextBoolean()) {
+ client.commit();
+ }
+ // Add eleven docs
+ List<SolrInputDocument> docs = new ArrayList<>();
+ for (int i = 0; i < 11; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", "id" + i);
+ doc.addField("name", "doc" + i);
+ doc.addField("price", "" + i);
+ docs.add(doc);
+ if (rarely()) {
+ client.commit();
Review Comment:
Is `client.add(docs);` need here and optionally `docs.clear()` after?
```suggestion
client.add(docs);
client.commit();
// docs.clear();
```
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -571,6 +572,57 @@ public void testGetEmptyResults() throws Exception {
assertEquals(0, out.get(1).size());
}
+ @Test
+ public void testMatchAllPaging() throws Exception {
+ SolrClient client = getSolrClient();
+
+ // Empty the database...
+ client.deleteByQuery("*:*"); // delete everything!
+ if (random().nextBoolean()) {
+ client.commit();
+ }
+ // Add eleven docs
+ List<SolrInputDocument> docs = new ArrayList<>();
+ for (int i = 0; i < 11; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", "id" + i);
+ doc.addField("name", "doc" + i);
+ doc.addField("price", "" + i);
+ docs.add(doc);
+ if (rarely()) {
+ client.commit();
+ }
+ }
+ client.add(docs);
+ if (random().nextBoolean()) {
+ client.commit();
+ } else {
+ client.optimize();
+ }
+ final List<String> sorts = Arrays.asList("_docid_", "id", "name", "price", null);
+ Collections.shuffle(sorts, random());
+ final List<Integer> starts = Arrays.asList(0, 1, 2, 10, 11, 12);
+ Collections.shuffle(starts, random());
+ for (String sort : sorts.subList(0, 1 + random().nextInt(sorts.size() - 1))) {
+ for (int start : starts.subList(0, 1 + random().nextInt(starts.size() - 1))) {
+ final SolrQuery query = new SolrQuery("*:*");
+ if (sort != null) {
+ query.setSort(sort, random().nextBoolean() ? SolrQuery.ORDER.asc : SolrQuery.ORDER.desc);
+ }
+ if (start > 0 || random().nextBoolean()) {
+ query.setStart(start);
+ }
+ SolrDocumentList results = client.query(query).getResults();
+ assertEquals(11, results.getNumFound());
+ assertEquals("page from " + start, Math.max(Math.min(10, 11 - start), 0), results.size());
Review Comment:
Hmm, not sure about the `Math.max(Math.min(10, 11 - start), 0)` expression e.g. for `start=0` it would evaluate to `10` but paging from `0` would return `11` results, no?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #1378: SOLR-16585: examples test for match all paging
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #1378:
URL: https://github.com/apache/solr/pull/1378#discussion_r1118500640
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -571,6 +572,67 @@ public void testGetEmptyResults() throws Exception {
assertEquals(0, out.get(1).size());
}
+ @Test
+ public void testMatchAllPaging() throws Exception {
+ SolrClient client = getSolrClient();
+
+ // Empty the database...
+ client.deleteByQuery("*:*"); // delete everything!
+ if (random().nextBoolean()) {
+ client.commit();
+ }
+ // Add eleven docs
+ List<SolrInputDocument> docs = new ArrayList<>();
+ final int docsTotal = CommonParams.ROWS_DEFAULT + 1;
+ for (int i = 0; i < docsTotal; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", "id" + i);
+ doc.addField("name", "doc" + i);
+ doc.addField("price", "" + i);
+ docs.add(doc);
+ if (rarely()) {
+ client.add(docs);
+ client.commit();
+ docs.clear();
+ }
+ }
+ client.add(docs);
+ if (random().nextBoolean()) {
+ client.commit();
+ } else {
+ client.optimize();
+ }
+ final List<String> sorts = Arrays.asList("_docid_", "id", "name", "price", null);
+ Collections.shuffle(sorts, random());
+ final List<Integer> starts =
+ Arrays.asList(0, 1, 2, CommonParams.ROWS_DEFAULT, docsTotal, CommonParams.ROWS_DEFAULT + 2);
+ Collections.shuffle(starts, random());
+ for (String sort : sorts.subList(0, 1 + random().nextInt(sorts.size() - 1))) {
+ for (int start : starts.subList(0, 1 + random().nextInt(starts.size() - 1))) {
+ final SolrQuery query = new SolrQuery("*:*");
+ if (sort != null) {
+ query.setSort(sort, random().nextBoolean() ? SolrQuery.ORDER.asc : SolrQuery.ORDER.desc);
+ }
+ if (start > 0 || random().nextBoolean()) {
+ query.setStart(start);
+ }
+ if (usually()) {
+ query.setRows(CommonParams.ROWS_DEFAULT);
+ }
+ SolrDocumentList results = client.query(query).getResults();
+ assertEquals(docsTotal, results.getNumFound());
+ assertEquals(
+ "page from " + start,
+ Math.max(Math.min(CommonParams.ROWS_DEFAULT, docsTotal - start), 0),
+ results.size());
+ for (SolrDocument doc : results) {
+ assertTrue(doc.containsKey("id") && doc.containsKey("name") && doc.containsKey("price"));
+ break;
Review Comment:
Curious why the `break;` is here? And to consider splitting the `&&` clauses to make any failures easier to diagnose.
```suggestion
assertTrue(doc.containsKey("id"));
assertTrue(doc.containsKey("name"));
assertTrue(doc.containsKey("price"));
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] magibney commented on a diff in pull request #1378: SOLR-16585: examples test for match all paging
Posted by "magibney (via GitHub)" <gi...@apache.org>.
magibney commented on code in PR #1378:
URL: https://github.com/apache/solr/pull/1378#discussion_r1118854120
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -571,6 +572,68 @@ public void testGetEmptyResults() throws Exception {
assertEquals(0, out.get(1).size());
}
+ @Test
+ public void testMatchAllPaging() throws Exception {
+ SolrClient client = getSolrClient();
+
+ // Empty the database...
+ client.deleteByQuery("*:*"); // delete everything!
+ if (random().nextBoolean()) {
+ client.commit();
+ }
+ // Add eleven docs
+ List<SolrInputDocument> docs = new ArrayList<>();
+ final int docsTotal = CommonParams.ROWS_DEFAULT + 1;
+ for (int i = 0; i < docsTotal; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", "id" + i);
+ doc.addField("name", "doc" + i);
+ doc.addField("price", "" + i);
+ docs.add(doc);
+ if (rarely()) {
+ client.add(docs);
+ client.commit();
+ docs.clear();
+ }
+ }
+ client.add(docs);
+ if (random().nextBoolean()) {
+ client.commit();
+ } else {
+ client.optimize();
+ }
+ final List<String> sorts = Arrays.asList("_docid_", "id", "name", "price", null);
+ Collections.shuffle(sorts, random());
+ final List<Integer> starts =
+ Arrays.asList(0, 1, 2, CommonParams.ROWS_DEFAULT, docsTotal, CommonParams.ROWS_DEFAULT + 2);
+ Collections.shuffle(starts, random());
+ for (String sort : sorts.subList(0, 1 + random().nextInt(sorts.size() - 1))) {
+ for (int start : starts.subList(0, 1 + random().nextInt(starts.size() - 1))) {
Review Comment:
I'm unclear why we're bothering to do subList here? It's not so many combinations that we can't just do all of them (resulting in 30 requests total, right?). Shuffling both is still a good idea (mainly to vary how subsequent requests interact via the queryRestultCache).
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -571,6 +572,68 @@ public void testGetEmptyResults() throws Exception {
assertEquals(0, out.get(1).size());
}
+ @Test
+ public void testMatchAllPaging() throws Exception {
+ SolrClient client = getSolrClient();
+
+ // Empty the database...
+ client.deleteByQuery("*:*"); // delete everything!
+ if (random().nextBoolean()) {
+ client.commit();
+ }
+ // Add eleven docs
+ List<SolrInputDocument> docs = new ArrayList<>();
+ final int docsTotal = CommonParams.ROWS_DEFAULT + 1;
+ for (int i = 0; i < docsTotal; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", "id" + i);
+ doc.addField("name", "doc" + i);
+ doc.addField("price", "" + i);
+ docs.add(doc);
+ if (rarely()) {
+ client.add(docs);
+ client.commit();
+ docs.clear();
+ }
+ }
+ client.add(docs);
+ if (random().nextBoolean()) {
+ client.commit();
+ } else {
+ client.optimize();
+ }
+ final List<String> sorts = Arrays.asList("_docid_", "id", "name", "price", null);
+ Collections.shuffle(sorts, random());
+ final List<Integer> starts =
+ Arrays.asList(0, 1, 2, CommonParams.ROWS_DEFAULT, docsTotal, CommonParams.ROWS_DEFAULT + 2);
+ Collections.shuffle(starts, random());
+ for (String sort : sorts.subList(0, 1 + random().nextInt(sorts.size() - 1))) {
+ for (int start : starts.subList(0, 1 + random().nextInt(starts.size() - 1))) {
+ final SolrQuery query = new SolrQuery("*:*");
+ if (sort != null) {
+ query.setSort(sort, random().nextBoolean() ? SolrQuery.ORDER.asc : SolrQuery.ORDER.desc);
+ }
+ if (start > 0 || random().nextBoolean()) {
+ query.setStart(start);
+ }
+ if (usually()) {
+ query.setRows(CommonParams.ROWS_DEFAULT);
+ }
Review Comment:
This is ok, but just to point out: issuing a request with rows=ROWS_DEFAULT (either explicit or unset/default) will I think usually cause the docList to be cached in its entirety for the associated sort; so this test will I think largely end up testing queryResultCache?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] mkhludnev commented on pull request #1378: SOLR-16585: examples test for match all paging
Posted by "mkhludnev (via GitHub)" <gi...@apache.org>.
mkhludnev commented on PR #1378:
URL: https://github.com/apache/solr/pull/1378#issuecomment-1449424728
Thanks to everyone for reviewing. Here's the last confirmation: Is it really worth to push in?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] mkhludnev merged pull request #1378: SOLR-16585: examples test for match all paging
Posted by "mkhludnev (via GitHub)" <gi...@apache.org>.
mkhludnev merged PR #1378:
URL: https://github.com/apache/solr/pull/1378
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] mkhludnev commented on a diff in pull request #1378: SOLR-16585: examples test for match all paging
Posted by "mkhludnev (via GitHub)" <gi...@apache.org>.
mkhludnev commented on code in PR #1378:
URL: https://github.com/apache/solr/pull/1378#discussion_r1119299406
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -571,6 +572,68 @@ public void testGetEmptyResults() throws Exception {
assertEquals(0, out.get(1).size());
}
+ @Test
+ public void testMatchAllPaging() throws Exception {
+ SolrClient client = getSolrClient();
+
+ // Empty the database...
+ client.deleteByQuery("*:*"); // delete everything!
+ if (random().nextBoolean()) {
+ client.commit();
+ }
+ // Add eleven docs
+ List<SolrInputDocument> docs = new ArrayList<>();
+ final int docsTotal = CommonParams.ROWS_DEFAULT + 1;
+ for (int i = 0; i < docsTotal; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", "id" + i);
+ doc.addField("name", "doc" + i);
+ doc.addField("price", "" + i);
+ docs.add(doc);
+ if (rarely()) {
+ client.add(docs);
+ client.commit();
+ docs.clear();
+ }
+ }
+ client.add(docs);
+ if (random().nextBoolean()) {
+ client.commit();
+ } else {
+ client.optimize();
+ }
+ final List<String> sorts = Arrays.asList("_docid_", "id", "name", "price", null);
+ Collections.shuffle(sorts, random());
+ final List<Integer> starts =
+ Arrays.asList(0, 1, 2, CommonParams.ROWS_DEFAULT, docsTotal, CommonParams.ROWS_DEFAULT + 2);
+ Collections.shuffle(starts, random());
+ for (String sort : sorts.subList(0, 1 + random().nextInt(sorts.size() - 1))) {
+ for (int start : starts.subList(0, 1 + random().nextInt(starts.size() - 1))) {
+ final SolrQuery query = new SolrQuery("*:*");
+ if (sort != null) {
+ query.setSort(sort, random().nextBoolean() ? SolrQuery.ORDER.asc : SolrQuery.ORDER.desc);
+ }
+ if (start > 0 || random().nextBoolean()) {
+ query.setStart(start);
+ }
+ if (usually()) {
+ query.setRows(CommonParams.ROWS_DEFAULT);
+ }
Review Comment:
Pardon, Michael. I can't get this point, ~~beside of thinking twice~~. Here I just emphasize that following asserts relies on default rows. I don't think it somehow impacts queryResultCache usage, really.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #1378: SOLR-16585: examples test for match all paging
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #1378:
URL: https://github.com/apache/solr/pull/1378#discussion_r1116738630
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -571,6 +572,57 @@ public void testGetEmptyResults() throws Exception {
assertEquals(0, out.get(1).size());
}
+ @Test
+ public void testMatchAllPaging() throws Exception {
+ SolrClient client = getSolrClient();
+
+ // Empty the database...
+ client.deleteByQuery("*:*"); // delete everything!
+ if (random().nextBoolean()) {
+ client.commit();
+ }
+ // Add eleven docs
+ List<SolrInputDocument> docs = new ArrayList<>();
+ for (int i = 0; i < 11; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", "id" + i);
+ doc.addField("name", "doc" + i);
+ doc.addField("price", "" + i);
+ docs.add(doc);
+ if (rarely()) {
+ client.commit();
+ }
+ }
+ client.add(docs);
+ if (random().nextBoolean()) {
+ client.commit();
+ } else {
+ client.optimize();
+ }
+ final List<String> sorts = Arrays.asList("_docid_", "id", "name", "price", null);
+ Collections.shuffle(sorts, random());
+ final List<Integer> starts = Arrays.asList(0, 1, 2, 10, 11, 12);
+ Collections.shuffle(starts, random());
+ for (String sort : sorts.subList(0, 1 + random().nextInt(sorts.size() - 1))) {
+ for (int start : starts.subList(0, 1 + random().nextInt(starts.size() - 1))) {
+ final SolrQuery query = new SolrQuery("*:*");
+ if (sort != null) {
+ query.setSort(sort, random().nextBoolean() ? SolrQuery.ORDER.asc : SolrQuery.ORDER.desc);
+ }
+ if (start > 0 || random().nextBoolean()) {
+ query.setStart(start);
+ }
+ SolrDocumentList results = client.query(query).getResults();
+ assertEquals(11, results.getNumFound());
+ assertEquals("page from " + start, Math.max(Math.min(10, 11 - start), 0), results.size());
Review Comment:
ah, hadn't spotted about implicit `rows=` usage there, thanks!
how about explaining the connection like this?
```suggestion
assertEquals("page from " + start, Math.max(Math.min(CommonParams.ROWS_DEFAULT, 11 - start), 0), results.size());
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] mkhludnev commented on a diff in pull request #1378: SOLR-16585: examples test for match all paging
Posted by "mkhludnev (via GitHub)" <gi...@apache.org>.
mkhludnev commented on code in PR #1378:
URL: https://github.com/apache/solr/pull/1378#discussion_r1116734711
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -571,6 +572,57 @@ public void testGetEmptyResults() throws Exception {
assertEquals(0, out.get(1).size());
}
+ @Test
+ public void testMatchAllPaging() throws Exception {
+ SolrClient client = getSolrClient();
+
+ // Empty the database...
+ client.deleteByQuery("*:*"); // delete everything!
+ if (random().nextBoolean()) {
+ client.commit();
+ }
+ // Add eleven docs
+ List<SolrInputDocument> docs = new ArrayList<>();
+ for (int i = 0; i < 11; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", "id" + i);
+ doc.addField("name", "doc" + i);
+ doc.addField("price", "" + i);
+ docs.add(doc);
+ if (rarely()) {
+ client.commit();
+ }
+ }
+ client.add(docs);
+ if (random().nextBoolean()) {
+ client.commit();
+ } else {
+ client.optimize();
+ }
+ final List<String> sorts = Arrays.asList("_docid_", "id", "name", "price", null);
+ Collections.shuffle(sorts, random());
+ final List<Integer> starts = Arrays.asList(0, 1, 2, 10, 11, 12);
+ Collections.shuffle(starts, random());
+ for (String sort : sorts.subList(0, 1 + random().nextInt(sorts.size() - 1))) {
+ for (int start : starts.subList(0, 1 + random().nextInt(starts.size() - 1))) {
+ final SolrQuery query = new SolrQuery("*:*");
+ if (sort != null) {
+ query.setSort(sort, random().nextBoolean() ? SolrQuery.ORDER.asc : SolrQuery.ORDER.desc);
+ }
+ if (start > 0 || random().nextBoolean()) {
+ query.setStart(start);
+ }
+ SolrDocumentList results = client.query(query).getResults();
+ assertEquals(11, results.getNumFound());
+ assertEquals("page from " + start, Math.max(Math.min(10, 11 - start), 0), results.size());
Review Comment:
but, default `rows=0`, it will cut 11 results to 10-items page.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org