Joe McDonnell 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: (2 comments) http://gerrit.cloudera.org:8080/#/c/23900/5/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/23900/5/be/src/service/impala-server.cc@2030 PS5, Line 2030: if (FLAGS_use_calcite_planner) { > Maybe use_calcite_as_default_planner? I was thinking more like having the startup param specify a string value equivalent to the query option rather than a boolean. (It would need to be subject to the same validation.) 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); > It doesn't, but this might be tricky? If "runtime_planners" is empty when you are about to set it on query_options, return a not-OK status. I'm not sure if that is actually possible, but I think we have a precondition in the frontend requiring the list to be non-empty. There may be a distinction between empty string (which has length 0) and empty quotes (length 2 with two quote characters). We can do a std::find(runtime_planners.begin(), runtime_planners.end(), planner_type) != runtime_planners.end() for checking that we are not adding a duplicate. -- 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: Wed, 28 Jan 2026 01:09:45 +0000 Gerrit-HasComments: Yes
