Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/24576 )
Change subject: IMPALA-15174: Add AGGREGATED_PROFILE query option to toggle profile formats ...................................................................... Patch Set 4: (3 comments) Thank you for the quick review! http://gerrit.cloudera.org:8080/#/c/24576/2/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/24576/2/be/src/service/impala-server.cc@2186 PS2, Line 2186: CLEAN_EXIT_WITH_ERROR(Substitute("Invalid default query options. Please > ok, the flag can stay from now, maybe we can deprecate and remove it once a Done http://gerrit.cloudera.org:8080/#/c/24576/2/be/src/util/impala-profile-tool.cc File be/src/util/impala-profile-tool.cc: http://gerrit.cloudera.org:8080/#/c/24576/2/be/src/util/impala-profile-tool.cc@a138 PS2, Line 138: > I think that it is important to have easily accessable information in a pro Done. To make sure the output does not change, I have extracted the profile type and have used it here to decide verbosity now. http://gerrit.cloudera.org:8080/#/c/24576/2/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/24576/2/be/src/util/runtime-profile.cc@a1628 PS2, Line 1628: > I noticed that profile_version didn't seem to be used anywhere meaningfully; > it was essentially just being appended based on the aggregated or > unaggregated profile flag. Given that, it didn't seem reasonable for the same > Impala version to output different profile version numbers based solely on > those flags. Additionally, I’m unsure what the exact criteria are for > defining a "new" profile version, and whether the session-based profile type > attribute is already sufficient for this purpose. > Could you let me know if my assumption here is incorrect? Specifically, is > profile_version being relied upon for inference by external tools like > Impyla? My understanding was that they do not use it. > If we do need to keep and update this versioning, could you suggest the > criteria I should use to determine a version bump, and where in the code it > would be most appropriate to implement that logic? Regarding this comment, I meant it towards the code around here. -- To view, visit http://gerrit.cloudera.org:8080/24576 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I96aceda1b044ae53d09e68cb78678fc2930326dd Gerrit-Change-Number: 24576 Gerrit-PatchSet: 4 Gerrit-Owner: Surya Hebbar <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Surya Hebbar <[email protected]> Gerrit-Comment-Date: Mon, 27 Jul 2026 10:35:51 +0000 Gerrit-HasComments: Yes
