Michael Smith 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) http://gerrit.cloudera.org:8080/#/c/20524/20/be/src/service/internal-server-test.cc File be/src/service/internal-server-test.cc: http://gerrit.cloudera.org:8080/#/c/20524/20/be/src/service/internal-server-test.cc@77 PS20, Line 77: size_t id_loc = contents_str.find("\"query_id\": \"" + PrintId(query_id) + "\""); > Done Ack http://gerrit.cloudera.org:8080/#/c/20524/20/be/src/service/internal-server-test.cc@81 PS20, Line 81: const string seek_str = "\"state\""; > Good catch. I didn't realize size_t was unsigned. Fixed by making the con You could use ssize_t. I'm trying to decide if this will still work. It will nolonger assert false if it reaches an open '{', so maybe you need to ASSERT_FALSE at the end of the function to assert we should never reach it. http://gerrit.cloudera.org:8080/#/c/20524/20/be/src/service/internal-server-test.cc@83 PS20, Line 83: // If an opening curly brace is found, then the state field is missing. Could also be ASSERT_NE. http://gerrit.cloudera.org:8080/#/c/20524/20/be/src/service/internal-server-test.cc@86 PS20, Line 86: if (seek_str == contents_str.substr(pos, seek_str.length())) { > Done Ack http://gerrit.cloudera.org:8080/#/c/20524/20/be/src/service/internal-server-test.cc@114 PS20, Line 114: table_name_ = StrCat(database_name_, ".", "products"); > Fixed here and elsewhere throughout the test. Ack http://gerrit.cloudera.org:8080/#/c/20524/20/be/src/service/internal-server-test.cc@456 PS20, Line 456: > Switch to using StrCat Ack -- 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: Tue, 05 Dec 2023 18:02:11 +0000 Gerrit-HasComments: Yes
