Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22734 )
Change subject: IMPALA-10268: Validate the debug actions when they are set ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/22734/4/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/22734/4/be/src/service/query-options.cc@235 PS4, Line 235: VerifyDebugAction Rename to VerifyExecNodeDebugAction. http://gerrit.cloudera.org:8080/#/c/22734/4/be/src/service/query-options.cc@319 PS4, Line 319: "invalid command" > Yes all errors should be propagated, except this one special case: https:// Thanks for your explanation. I understand know that, the reason this specific "invalid command" catched here is because there is a possibility that this is a valid ExecNode DebugAction, even if it is an invalid General DebugAction. http://gerrit.cloudera.org:8080/#/c/22734/4/be/src/util/debug-util.cc File be/src/util/debug-util.cc: http://gerrit.cloudera.org:8080/#/c/22734/4/be/src/util/debug-util.cc@459 PS4, Line 459: if (!verify_only) { : DCHECK(false) << "Invalid debug action"; : } : return Status(Substitute(ERROR_MSG, components[0], action_str, "invalid command")); Is it possible to move the ExecNode DebugAction verification here? bool throwStatus = true; if (!verify_only) { DCHECK(false) << "Invalid debug action"; return Status(Substitute(ERROR_MSG, components[0], action_str, "invalid command")); } else { // An invalid General DebugAction might be a valid ExecNode DebugAction. // In verification mode, parse it to TDebugOptions and verify using // VerifyExecNodeDebugAction. DebugOptions debug_options(value); RETURN_IF_ERROR(VerifyExecNodeDebugAction(debug_options.ToThrift())); Status verificationStatus = VerifyExecNodeDebugAction(debug_options.ToThrift()); } Thus, DebugActionVerifyOnly will be consistent. It always validates both possibility of General and ExecNode DebugAction. -- To view, visit http://gerrit.cloudera.org:8080/22734 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I53816aba2c79b556688d3b916883fee7476fdbb5 Gerrit-Change-Number: 22734 Gerrit-PatchSet: 4 Gerrit-Owner: Mihaly Szjatinya <msz...@pm.me> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Mihaly Szjatinya <msz...@pm.me> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Comment-Date: Thu, 17 Apr 2025 14:58:09 +0000 Gerrit-HasComments: Yes
