Gokul Kolady has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24446 )

Change subject: IMPALA-15093: Use RE2 For Profile Analyzer Regex
......................................................................


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/24446/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/24446/1//COMMIT_MSG@9
PS1, Line 9: Replace std::regex usage in query profile parsing/redaction with
           : re2::RE2 so the AI analyzer flow uses a single regex engine
           : end-to-end. This updates pattern definitions and
           : matching/replacement/extraction paths (query ID,
           : fragment/operator
> nit. Each line of commit message should be 72 characters or fewer if possib
Done


http://gerrit.cloudera.org:8080/#/c/24446/1//COMMIT_MSG@14
PS1, Line 14: redaction token collection) while preserving existing behavior.
> Could you please also mention the test you ran for this patch here?
Done


http://gerrit.cloudera.org:8080/#/c/24446/1/be/src/service/query-profile-parsing-tools.cc
File be/src/service/query-profile-parsing-tools.cc:

http://gerrit.cloudera.org:8080/#/c/24446/1/be/src/service/query-profile-parsing-tools.cc@775
PS1, Line 775:       re2::StringPiece plan(plan_value.GetString(), 
plan_value.GetStringLength());
> Can this `plan_text` variable be eliminated?
Done


http://gerrit.cloudera.org:8080/#/c/24446/1/be/src/service/query-profile-parsing-tools.cc@790
PS1, Line 790:       string estimates_match;
> Can this `plan_text` variable be eliminated?
Done


http://gerrit.cloudera.org:8080/#/c/24446/1/be/src/service/query-profile-parsing-tools.cc@1125
PS1, Line 1125:   double total_ms = 0.0;
> Suggestion:  This change would be a great time to add ctest tests for this
Done


http://gerrit.cloudera.org:8080/#/c/24446/1/be/src/service/query-profile-parsing-tools.cc@1134
PS1, Line 1134:       total_ms += amount * 3600000.0;
> Can this extra string create be eliminated?
Done


http://gerrit.cloudera.org:8080/#/c/24446/1/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:

http://gerrit.cloudera.org:8080/#/c/24446/1/be/src/service/query-profile-redaction.cc@99
PS1, Line 99: static vector<string> CollectRegexMatches(
> Suggestion:  This change would be a great time to add ctest tests for this
Done



--
To view, visit http://gerrit.cloudera.org:8080/24446
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie1343b2f049758e862d229f2a647273de56855c4
Gerrit-Change-Number: 24446
Gerrit-PatchSet: 2
Gerrit-Owner: Gokul Kolady <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: Gokul Kolady <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Yida Wu <[email protected]>
Gerrit-Comment-Date: Sat, 13 Jun 2026 00:31:53 +0000
Gerrit-HasComments: Yes

Reply via email to