hossman commented on PR #2691: URL: https://github.com/apache/solr/pull/2691#issuecomment-2329635680
Hmmmm.... IMO this change does not seem like an overall improvement to the doc First off, the premise of the PR seems to conflate two diff scenarios... > "You can repeat this process until you’ve fetched as many docs as you want, or until the nextCursorMark returned matches the cursorMark you’ve already specified — indicating that there are no more results." > The pseudo code misses the first part. There is a difference between the "until you’ve fetched as many docs **as you _want_**" scenario (ie: "I as a client want to fetch docs from solr until i have enough to satisfy my purposes" - [there is a separate example for that later in the doc](https://solr.apache.org/guide/solr/latest/query-guide/pagination-of-results.html#fetch-first-n-docs-based-on-post-processing)) and "solr ran out of documents to return" scenario being demonstrated here. Adding a "did solr return the full number of rows requested?" check to the code doesn't do anything to demonstrate the first scenario Second: The original psuedo-code here explicitly did *NOT* check for "did solr return the full number of rows requested?" because doing so is only a viable way to optimize away the lsat request in static indexes. In a "Tailing a Cursor" type scenario you absolutely should not check the number of docs returned -- this is a type of use case that is introduced later in the document, and the psuedocode in both sections was designed to be structuraly the same -- except for the addition of loop-forever/sleep wrapped around it If we're going to change _this_ pseudo code block to do an explicit rows check, then the example should probably note it's an optimization, and the _other_ psuedo code example in the later section on "tailing a cursor" should have a note that you _can't_ use that optimization in this situation. Third: This PR only changed the psuedo-code example, even though the very next line after the example says... > Using SolrJ, this pseudocode would be: ...but now the psuedo-code and SolrJ code no longer match. Likewise the sequential "curl" example (following the SolrJ example) no longer mtaches either, because it still does a final request looking or an unchanged `nextCursorMark` (even though it notes the previous request request returned less docs then the `rows` param) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
