Fucun Chu has posted comments on this change. ( http://gerrit.cloudera.org:8080/16706 )
Change subject: IMPALA-10317: Add query option that limits huge joins at runtime ...................................................................... Patch Set 4: (12 comments) Thanks for the review! Addressed the comments. http://gerrit.cloudera.org:8080/#/c/16706/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16706/3//COMMIT_MSG@11 PS3, Line 11: exceed > nit: 'exceeding' instead of exceed Done http://gerrit.cloudera.org:8080/#/c/16706/3//COMMIT_MSG@18 PS3, Line 18: backend metrics for RowsReturned. Example from " > For this example, can you also add the query option ? I assume it was set t Done http://gerrit.cloudera.org:8080/#/c/16706/3/be/src/runtime/coordinator.h File be/src/runtime/coordinator.h: http://gerrit.cloudera.org:8080/#/c/16706/3/be/src/runtime/coordinator.h@230 PS3, Line 230: /// Total num rows produced by each join node. The key is join node id. > Pls indicate what the key of the map is. I am also wondering whether the st The key has been changed to node id http://gerrit.cloudera.org:8080/#/c/16706/3/be/src/runtime/coordinator.h@231 PS3, Line 231: join_rows_produced; > This could more simply be named 'per_join_rows_produced' (see similar comm Done http://gerrit.cloudera.org:8080/#/c/16706/3/be/src/runtime/fragment-instance-state.cc File be/src/runtime/fragment-instance-state.cc: http://gerrit.cloudera.org:8080/#/c/16706/3/be/src/runtime/fragment-instance-state.cc@331 PS3, Line 331: // row count stats for a join node > nit: this comment is a bit confusing. Suggest rephrasing to 'row count sta Done http://gerrit.cloudera.org:8080/#/c/16706/3/be/src/runtime/fragment-instance-state.cc@335 PS3, Line 335: rfind(nested_loop_type > A bit odd that for hash_type you are passing a second argument of 0 while n To implement the startsWith function, the two variable judgments are the same. Done http://gerrit.cloudera.org:8080/#/c/16706/3/be/src/runtime/fragment-instance-state.cc@336 PS3, Line 336: per_join_rows_produced[node->metadata().plan_node_id] = rows_counter->value(); > Is there not a unique numeric id that can be used here instead of the node Done http://gerrit.cloudera.org:8080/#/c/16706/3/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/16706/3/be/src/service/impala-server.cc@2688 PS3, Line 2688: Status err = Status::Expected(TErrorCode::JOIN_ROWS_PRODUCED_LIMIT_EXCEEDED, > it would be useful to know which join node (with the node id) in a complex Done http://gerrit.cloudera.org:8080/#/c/16706/3/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/16706/3/be/src/service/query-options.cc@991 PS3, Line 991: case TImpalaQueryOptions::USE_DOP_FOR_COSTING: { > I think the prefix 'NUM' can be removed and simplify the naming. JOIN_ROWS Done http://gerrit.cloudera.org:8080/#/c/16706/3/be/src/service/query-options.cc@996 PS3, Line 996: StringParser::ParseResult result; > 'rows returned limit' doesn't match the name of the query option. Should th Done http://gerrit.cloudera.org:8080/#/c/16706/3/testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test File testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test: http://gerrit.cloudera.org:8080/#/c/16706/3/testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test@71 PS3, Line 71: ---- QUERY > Suggest adding a test with a mix of joins ..i.e couple of hash joins and a Done http://gerrit.cloudera.org:8080/#/c/16706/3/testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test@87 PS3, Line 87: in > nit: remove trailing whitespace Done -- To view, visit http://gerrit.cloudera.org:8080/16706 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbca7e053b61b4e31b066edcfb3b0398fa859d02 Gerrit-Change-Number: 16706 Gerrit-PatchSet: 4 Gerrit-Owner: Fucun Chu <chufu...@hotmail.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Fucun Chu <chufu...@hotmail.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Sun, 20 Dec 2020 04:12:02 +0000 Gerrit-HasComments: Yes
