Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23900 )
Change subject: IMPALA-14488: Calcite planner: Support fallback to Original planner ...................................................................... Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/23900/5/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/23900/5/be/src/service/query-options.cc@1396 PS5, Line 1396: case TImpalaQueryOptions::RUNTIME_PLANNERS: { : std::vector<TPlannerType::type> runtime_planners; : const string planners = std::regex_replace(value, std::regex("^\"|\"$"), ""); : vector<string> str_types; : split(str_types, planners, is_any_of(","), token_compress_on); : TrimAndRemoveEmptyString(str_types); : for (const auto& t : str_types) { : TPlannerType::type planner_type; : RETURN_IF_ERROR(GetThriftEnum(t, "planner type", : _TPlannerType_VALUES_TO_NAMES, &planner_type)); : runtime_planners.push_back(planner_type); : } : query_options->__set_runtime_planners(runtime_planners); > Here is what I mean: If the local variable "runtime_planners" is empty when Ok, just tested and there is indeed a difference between empty and "" I can do what you are saying and catch "" and reject it. It would be different behavior from empty string though which is caught at line 289 in this method (266 before changes). Wouldn't we want the same behavior? If so, I can change line 289 to do what you are saying, but that's what I meant above about "breaking precedence" because I'd have to put special case functionality for that. This whole thing can also be avoided if I leave use_calcite_planner alone and add a boolean for fallback as well. This logic wouldn't be scalable for a third planner, but do we need to be scalable for this? I'm starting to lean towards doing it that way since it won't have this quote problem. What do you think? -- To view, visit http://gerrit.cloudera.org:8080/23900 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id1fdc5ef92fff84e89af0e19c4246cc15e2ea823 Gerrit-Change-Number: 23900 Gerrit-PatchSet: 5 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Thu, 29 Jan 2026 23:49:30 +0000 Gerrit-HasComments: Yes
