Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13883 )
Change subject: IMPALA-8779, IMPALA-8780: RowBatchQueue re-factoring and BufferedPRS impl ...................................................................... Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/13883/11/be/src/exec/buffered-plan-root-sink.cc File be/src/exec/buffered-plan-root-sink.cc: http://gerrit.cloudera.org:8080/#/c/13883/11/be/src/exec/buffered-plan-root-sink.cc@123 PS11, Line 123: // For now, if num_results < batch->num_rows(), we terminate returning results : // early. : if (num_results > 0 && num_results < batch->num_rows()) { > the case in which the total number of result rows is not a multiple of row > batch size. That case is still handled properly. The value of num_results is the same as the fetch size configured by any client. For impala-shell, I believe that is 1024, for other drivers such as Simba the docs state that the "RowsFetchedPerBlock" ("The maximum number of rows that a query returns at a time.") is 10000. This condition is only hit if a user configures a fetch size under 1024. Otherwise, everything works as expected (no partial results) regardless of the number of rows returned by the actual query. The only way partial results could be returned is if a user sets the fetch size to a value above 1024, fetches rows, sets the fetch size to under 1024, and then fetches rows. However, thats non-default behavior, and a bit of an odd pattern, which is why I decided to defer fixing this to a future patch. Do we want to a CHECK for this? That would cause a crash that affects all Impala users, which is presumably worse than the current behavior? -- To view, visit http://gerrit.cloudera.org:8080/13883 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9b1bb4b9c6f6e92c70e8fbee6ccdf48c2f85b7be Gerrit-Change-Number: 13883 Gerrit-PatchSet: 11 Gerrit-Owner: Sahil Takiar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Mon, 29 Jul 2019 16:25:53 +0000 Gerrit-HasComments: Yes
