Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/20524 )
Change subject: IMPALA-12426: Adds the backend InternalServer class. ...................................................................... Patch Set 21: (6 comments) Saving my questions at end of day, I'm not finished reviewing yet http://gerrit.cloudera.org:8080/#/c/20524/21/be/src/service/internal-server-test.cc File be/src/service/internal-server-test.cc: http://gerrit.cloudera.org:8080/#/c/20524/21/be/src/service/internal-server-test.cc@101 PS21, Line 101: // DO NOT provide a value for max_records that is less than 5. Delete if 'max_records' is unused (see below) http://gerrit.cloudera.org:8080/#/c/20524/21/be/src/service/internal-server-test.cc@105 PS21, Line 105: max_records Is 'max_records' used? Could it be deleted? http://gerrit.cloudera.org:8080/#/c/20524/21/be/src/service/internal-server-test.cc@121 PS21, Line 121: // Insert some products that have a last_sold time. It seems a bit untidy that all this work is done in a constructor rather than in a separate function. http://gerrit.cloudera.org:8080/#/c/20524/21/be/src/service/internal-server-test.cc@277 PS21, Line 277: query_results results = make_shared<vector<string>>(); Unused I think http://gerrit.cloudera.org:8080/#/c/20524/21/be/src/service/internal-server-test.cc@410 PS21, Line 410: DatabaseTest db_test = DatabaseTest(impala_server_, "invalid1"); Unused http://gerrit.cloudera.org:8080/#/c/20524/21/be/src/service/internal-server-test.cc@440 PS21, Line 440: thread_count Is 'thread_count' used? -- To view, visit http://gerrit.cloudera.org:8080/20524 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I27686aa563fac87429657e4980b29b0da91eb9e1 Gerrit-Change-Number: 20524 Gerrit-PatchSet: 21 Gerrit-Owner: Jason Fehr <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Wed, 06 Dec 2023 01:19:39 +0000 Gerrit-HasComments: Yes
