You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Gokul Kolady (Code Review)" <ge...@cloudera.org> on 2026/05/11 22:10:13 UTC
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Gokul Kolady has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24282
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targetted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,142 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/2
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 2
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24282/8/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/8/be/src/service/query-profile-redaction.cc@215
PS8, Line 215: boost::algorithm::replace_all(text, entry.first, entry.second);
replace_all is O(N^2) and we could have some really large profiles. We should optimize this.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 8
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 May 2026 16:01:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 16:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/22467/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 16
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 19:46:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 10:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/24282/10/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/10/be/src/service/query-profile-redaction.cc@98
PS10, Line 98: // Merges original-to-alias mappings into an alias-to-original lookup table.
: static unordered_map<string_view, string_view> BuildAliasReverseMap(
> A bit concern of returning the string_view, maybe we can add a comment like
Done
http://gerrit.cloudera.org:8080/#/c/24282/10/be/src/service/query-profile-redaction.cc@286
PS10, Line 286: auto
> Can we use explicit type unordered_map<string_view, string_view>, so that v
Done
http://gerrit.cloudera.org:8080/#/c/24282/10/be/src/service/query-profile-redaction.cc@527
PS10, Line 527:
> Can we also add a logging here, something like below for debug purpose?
Done
http://gerrit.cloudera.org:8080/#/c/24282/10/be/src/service/query-profile-redaction.cc@550
PS10, Line 550: return Status("Query profile input must be a valid JSON object");
> Can we add a logging here to print the error info?
Done
http://gerrit.cloudera.org:8080/#/c/24282/10/be/src/service/query-profile-redaction.cc@550
PS10, Line 550: return Status("Query profile input must be a valid JSON object");
> On second thought, maybe LOG(WARNING) is good enough for the profile redact
Done
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 10
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 May 2026 22:25:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 16:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/24282/15/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/15/be/src/service/query-profile-redaction.cc@51
PS15, Line 51:
: static constexpr const char* REDACTED_SQL_STATEMENT = "[REDACTED_SQL_STA
> These should probably be startup options so that these could be tuned if ne
Done
http://gerrit.cloudera.org:8080/#/c/24282/15/be/src/service/query-profile-redaction.cc@90
PS15, Line 90: regex::optimize | regex::nosubs);
> We should get process mem_limit from:
Done
http://gerrit.cloudera.org:8080/#/c/24282/15/be/src/service/query-profile-redaction.cc@388
PS15, Line 388: const unordered_set<string_view>& target_keys, vector<string>* values) {
> seen_host_tokens could be removed as profile should only have unique hosts
Done
http://gerrit.cloudera.org:8080/#/c/24282/15/be/src/service/query-profile-redaction.cc@473
PS15, Line 473: }
> We should avoid this copy as profiles could be quite huge. We should update
Done
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 16
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 19:24:36 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#17).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,460 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/17
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 17
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 10:
Fixed code and unit tests to ensure tests pass and redaction is complete in coverage
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 10
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 May 2026 20:18:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 11: Code-Review+1
(6 comments)
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@289
PS11, Line 289: const unordered_map<string_view, string_view> reverse_aliases =
BuildAliasReverseMap seems unnecessary can can possibly be removed? We could simply iterate over aliases and store the key:value as value:key in alias_to_original?
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@319
PS11, Line 319: static vector<string> CollectIdentifierContextsFromJsonProfile(const Value& node) {
we're returning a vector with every recursion and doing too many allocations/deallocations. We could instead pass the vector by reference to the recursion tree.
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@358
PS11, Line 358: static vector<string> CollectInfoStringValuesByKeys(
We could pass the vector by reference down the recursion tree.
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@395
PS11, Line 395: set<string> seen_host_tokens;
We need ordering? If not we could just use a vector<string> here. We could assume that the host names are not duplicates in the profile.
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@516
PS11, Line 516: status = BuildApplyAndTrackAliases(all_ip_tokens, "ip", redacted, alias_to_original);
We're calling BuildApplyAndTrackAliases multiple times and essentially scanning the profile and copying it multiple times. We could instead build a dictionary using all the tokens and apply all the aliases in one pass doing a single copy of profile.
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@573
PS11, Line 573: unordered_map<string, string> alias_to_original;
local map seems unnecessary? You could directly pass `alias_to_original` and remove the move operation later.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 11
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 01:57:16 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#14).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,392 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/14
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 14
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#8).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,279 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/8
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 8
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 12:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/22460/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 12
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 02:55:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 4:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@138
PS3, Line 138: }
Pass entries by reference.
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@148
PS3, Line 148: // Builds aliases from tokens, applies them to text, and tracks reverse mappings.
Pass parameters by reference.
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@95
PS4, Line 95: "Per Host Min Memory Reservation", "Per Host Number of Fragment Instances"};
We could use one of these sections, maybe "Per Host Min Memory Reservation" ?
Otherwise we're getting host names twice.
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@99
PS4, Line 99: set<string> host_tokens;
We probably don't need an ordered set here so maybe use unordered_set.
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@115
PS4, Line 115: set<string> sorted_tokens(tokens.cbegin(), tokens.cend());
We don't need a temporary set here. We can simply iterate through the input tokens?
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@143
PS4, Line 143: vector<std::pair<string, string>>(alias_map.cbegin(), alias_map.cend()),
Not sure why we need a temporary vector here ? We can pass the map by reference and iterate through it? Not sure why we need any ordering in this case?
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@154
PS4, Line 154: const auto reverse_aliases = BuildAliasReverseMap(aliases);
Pass alias_to_original by reference to BuildAliasReverseMap to avoid temp copies.
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@164
PS4, Line 164: const string plan_text_copy(plan_text);
avoid local copy here, either pass string by reference to this function or use regex_search constructor which accepts a char* parameter instead of a string.
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@328
PS4, Line 328: string redacted = profile_text;
We're making 2 copies here and in the following line. We need one copy for redacted profile but for parsing you can directly pass char* to source_json.Parse(..)
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@334
PS4, Line 334: vector<string> sql_statements =
Either pass sql_statements by reference to CollectInfoStringValuesByKeys or use move semantics to avoid copy.
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@336
PS4, Line 336: for (const string& sql_statement : sql_statements) {
Do we ever have multiple "Sql Statement" in the profile?
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@339
PS4, Line 339: boost::algorithm::replace_all(redacted, sql_statement, REDACTED_SQL_STATEMENT);
This seems unnecessary since we've already redacted above ?
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@377
PS4, Line 377: const string_view& text, const map<string, string>& alias_to_original) {
We can use an unordered_map unless we really need ordered replacements in L379.
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@387
PS4, Line 387: const string profile_text_copy(profile_text);
This seems redundant? We can directly pass profile_text.data() and profile_text.size() to source_json.Parse() ?
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 4
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 15:27:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 11:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@115
PS11, Line 115: unordered_set<string> seen_matches;
use unordered_set<string_view> instead since you're just using it for comparison in local scope.
```
...
string_view token(match[group_index].first, match[group_index].second);
if (!seen_matches.insert(token).second) continue;
results.emplace_back(token.begin(), token.end());
...
```
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@137
PS11, Line 137: unordered_set<string> seen_matches;
Use unordered_set<string_view> to avoid unnecessary copies.
```
...
string_view candidate(text.data() + start, pos - start);
...
results.emplace_back(candidate);
...
```
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 11
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 00:11:41 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 17:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/13505/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 17
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sun, 17 May 2026 02:45:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 4:
(20 comments)
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.h
File be/src/service/query-profile-redaction.h:
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.h@30
PS3, Line 30: const std::string_view& profi
> Nit: declare this as const std::string_view&
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.h@31
PS3, Line 31: const std::string_vie
> Nit: declare this as const std::string_view&
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.h@36
PS3, Line 36: std::map<std::string, std::string> alias_to_original_;
> This map is sorted in a couple different places. Declare it as a std::map
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@50
PS3, Line 50:
> This is called in a single place so you could also embed `boost::algorithm:
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@49
PS3, Line 49: static constexpr const char* REDACTED_SQL_STATEMENT = "[REDACTED_SQL_STATEMENT]";
:
: // Escapes a string so it can be safely matched within JSON-serialized text.
: s
> Eliminate this function entirely and call boost::trim_copy directly. As wr
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@55
PS3, Line 55: writer.String(input.data(), static_cast<rapidjson::SizeType>(input.size()));
> I think instead of regex search we could rely on extracting hostnames based
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@55
PS3, Line 55: ic_cast<rapidjson
> Use const string_view&. Also on lines 66, 116, 130, 165, 175, 343, 433, 44
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@70
PS3, Line 70: }
: return alias_to_original;
: }
:
: sta
> We're doing 3 copies here L70, L72 and L73. One copy is unavoidable so we s
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@71
PS3, Line 71: return alias_to_original;
: }
:
: sta
> This code will not handle the situation where either the first or last (but
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@99
PS3, Line 99: ;
> Use .cbegin() and .cend() here.
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@100
PS3, Line 100: vector<string> host_sections =
> Since ordering is needed, use a set instead of unordered_set so that sortin
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@117
PS3, Line 117: size_t idx = 0;
: for (const string& token : sorted_tokens) {
> Instead of sorting a vector, can tokens be a std::multiset or std::set?
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@128
PS3, Line 128: vector<std::pair<string, string>> entries, string text) {
: std::sort(entries.begin(), entries.end(),
: [](const std::pair<string, string>& a, const std::pair<string, string>& b) {
: return a.first.size() > b.first.size();
: });
: for (const auto& entry : entries) {
:
> No need for this function, call boost::algorithm::replace_all or boost::alg
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@136
PS3, Line 136: }
: return text;
: }
:
: // Applies a full alias map to a text blob.
: static string ApplyAliasMap(const unordered_map<string, string>& alias_map, string text) {
: return ApplySortedReplacements(
: vector<std::pair<string, string>>(alias_map.cbegin(), alias_map.cend()),
:
> This function is only called from one place, move the code to ApplySortedRe
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@183
PS3, Line 183: if (node.IsArray()) {
> return boost::algorithm::trim_copy which will enable return value optimizat
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@335
PS3, Line 335: CollectInfoStringValuesByKeys(source_json, {"Sql Statement"});
: for (const string& sql_statement : sql_statements) {
: boost::algorithm::replace_all(
: redacted, JsonEscapeString(sql_statement), REDAC
> Declare table_set and column_set as type std::set<string> and the sorting w
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@344
PS3, Line 344: nst std::regex user_kv_re(
> A shared_ptr doesn't provide value here. Pass in an unordered_map<string,
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@376
PS3, Line 376: static string UnredactTextWithAliases(
: const string_view& text, const map<string, string>& alias_to_original) {
: if (alias_to_original.empty()) return text;
: return ApplySortedReplacements(vector<std::pai
> It does not seem possible for this else case to happen. Same on lines 388,
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@441
PS3, Line 441:
> Ensure this function does not get called twice by adding a DCHECK:
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@455
PS3, Line 455:
> Ensure the Redact function has already been called by adding a DCHECK:
Done
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 4
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 13 May 2026 21:53:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 4:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/22417/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 4
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 13 May 2026 22:01:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 5:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/22433/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 5
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 20:21:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 11: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 11
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 May 2026 22:46:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 17:
Removed startup flags here for profile size limit, default limit can now only be overwritten by caller.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 17
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sun, 17 May 2026 01:36:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 18: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 18
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sun, 17 May 2026 07:38:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#3).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,130 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/3
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 3
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 6:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@74
PS6, Line 74: static const std::regex IP_RE(
We should probably redact IPV6 addresses also.
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@50
PS6, Line 50: // Matches hostnames followed by a port (e.g. coordinator.example.com:22000).
: static const std::regex HOST_WITH_PORT_RE(
: R"(\b([A-Za-z][A-Za-z0-9-]*(?:\.[A-Za-z0-9-]+)*)\:(\d{2,5})\b)", regex::optimize);
: // Matches the analyzed query subsection embedded in the textual plan.
: static const std::regex ANALYZED_RE(
: R"(Analyzed query:\s*([\s\S]*?)\n\nF\d+:PLAN FRAGMENT)", regex::optimize);
: // Matches fully-qualified identifiers with at least one dot (e.g. db.tbl.col).
: static const std::regex QUALIFIED_ID_RE(
: R"(\b([A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*)+)\b)", regex::optimize);
: // Matches table tokens following FROM/JOIN clauses.
: static const std::regex FROM_JOIN_TABLE_RE(
: R"(\b(?:from|join)\s+([A-Za-z_][A-Za-z0-9_\.]*)\b)",
: regex::optimize | regex::icase);
: // Matches snake_case identifiers that are candidates for column tokens.
: static const std::regex SNAKE_CASE_ID_RE(
: R"(\b([A-Za-z_][A-Za-z0-9_]*_[A-Za-z0-9_]*)\b)", regex::optimize);
: // Matches e-mail addresses.
: static const std::regex EMAIL_RE(
: R"([A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,})",
: regex::optimize | regex::nosubs);
: // Matches user/userid key-value pairs like user=alice.
: static const std::regex USER_KV_RE(
: R"(\b(?:user|uid)=([A-Za-z0-9._@-]+)\b)", regex::optimize | regex::icase);
: // Matches IPv4 addresses.
: static const std::regex IP_RE(
: R"(\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}"
: R"((?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\b)",
: regex::optimize | regex::nosubs);
unit tests for theses regexes would be good.
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@98
PS6, Line 98: alias_to_original[entry.second] = entry.first;
we guarantee aliases are unique?
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@118
PS6, Line 118: std::sort(results.begin(), results.end());
I think I missed why we need to sort the matched results?
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@126
PS6, Line 126: unordered_set<string> seen_tokens;
seen_tokens is unecessary as you can look up in the alias_map.
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@132
PS6, Line 132: std::stringstream alias;
: alias << prefix << "_" << std::setfill('0') << std::setw(3) << (++idx);
We could avoid stringstream altogether and do string manipulation using append() ?
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@134
PS6, Line 134: alias_map[token] = alias.str();
when possible use emplace.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 6
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 22:25:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 7:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@405
PS6, Line 405:
: // Redacts sensitive profile values and optionally records alias-to-original mappings.
: static Status RedactQueryProfileWithAliases(const string_view& profile_text,
: const Value& source_json,
:
> We used to iterate over all matches but based on earlier review comments de
Ack.
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@302
PS7, Line 302: if (node.HasMember("info_strings") && node["info_strings"].IsArray()) {
: for (const auto& entry : node["info_strings"].GetArray()) {
: const auto [key, value] = ParseInfoStringEntry(entry);
: if (key == nullptr || value == nullptr) continue;
: if (target_keys.find(key) == target_keys.end()) continue;
: if (*value != '\0') values.push_back(value);
: }
: }
:
: for (auto it = node.MemberBegin(); it != node.MemberEnd(); ++it) {
: auto child_values = CollectInfoStringValuesByKeys(it->value, target_keys);
: values.insert(
: values.end(), make_move_iterator(child_values.begin()),
: make_move_iterator(child_values.end()));
: }
Here is similar to CollectIdentifierContextsFromJsonProfile().
We could put the info_strings logic into the node for loop to reduce search cost and the second prcess on info_strings node.
"
for (auto it = node.MemberBegin(); it != node.MemberEnd(); ++it) {
if (strcmp(it->name.GetString(), "info_strings") == 0) {
if (!it->value.IsArray()) continue;
for (const auto& entry : it->value.GetArray()) {
...
}
continue;
}
auto child_values = CollectInfoStringValuesByKeys(it->value, target_keys);
values.insert(
values.end(), make_move_iterator(child_values.begin()),
make_move_iterator(child_values.end()));
}
"
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@418
PS7, Line 418: JsonEscapeString(sql_statement)
Trying to understand what the sql_statement input would be like, is it "SELECT * FROM users WHERE email = \"bob@test.com\"" or "SELECT * FROM users WHERE email = "bob@test.com""
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 7
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 May 2026 04:22:00 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/22443/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 8
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 May 2026 05:05:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#12).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,357 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/12
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 12
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 13:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24282/12/be/src/service/query-profile-redaction.h
File be/src/service/query-profile-redaction.h:
http://gerrit.cloudera.org:8080/#/c/24282/12/be/src/service/query-profile-redaction.h@28
PS12, Line 28: // Redacts sensitive query-p
> I think we should add a detailed comment to the QueryProfileRedactor class
Done
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 13
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 03:33:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 15:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/24282/15/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/15/be/src/service/query-profile-redaction.cc@51
PS15, Line 51: static constexpr int64_t MAX_REDACTION_PROFILE_SIZE_LIMIT_BYTES = 256L * 1024L * 1024L;
: static constexpr int64_t PROFILE_SIZE_LIMIT_MEM_FRACTION_DIVISOR = 100L;
These should probably be startup options so that these could be tuned if needed.
For reference: https://github.com/apache/impala/blob/master/be/src/service/data-stream-service.cc#L50
http://gerrit.cloudera.org:8080/#/c/24282/15/be/src/service/query-profile-redaction.cc@90
PS15, Line 90: const int64_t host_mem_limit = MemInfo::physical_mem();
We should get process mem_limit from:
```
ExecEnv::GetInstance()->process_mem_tracker()->limit()
```
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 15
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 05:37:02 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 10:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/24282/10/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/10/be/src/service/query-profile-redaction.cc@98
PS10, Line 98: // Merges original-to-alias mappings into an alias-to-original lookup table.
: static unordered_map<string_view, string_view> BuildAliasReverseMap(
A bit concern of returning the string_view, maybe we can add a comment like this to warn:
"// Returns string_views that may tie to the memory of strings in 'original_to_alias'.
// To prevent dangling pointers, callers must use these views only when 'original_to_alias' exists."
http://gerrit.cloudera.org:8080/#/c/24282/10/be/src/service/query-profile-redaction.cc@286
PS10, Line 286: auto
Can we use explicit type unordered_map<string_view, string_view>, so that visually obvious to anyone who might use this later
http://gerrit.cloudera.org:8080/#/c/24282/10/be/src/service/query-profile-redaction.cc@527
PS10, Line 527:
Can we also add a logging here, something like below for debug purpose?
"
VLOG(1) << "Query profile redaction complete. Extracted items : "
<< "SQL statements: " << sql_statements.size() << ", "
<< "Users: " << username_tokens.size() << ", "
<< "IPs: " << all_ip_tokens.size() << ", "
<< "Tables: " << table_tokens.size() << ", "
<< "Columns: " << column_tokens.size() << ", "
<< "Hosts: " << host_tokens.size() << ". "
<< "Total aliases generated: " << alias_to_original.size();
"
http://gerrit.cloudera.org:8080/#/c/24282/10/be/src/service/query-profile-redaction.cc@550
PS10, Line 550: return Status("Query profile input must be a valid JSON object");
Can we add a logging here to print the error info?
"LOG(ERROR) << "Profile redaction failed with the RapidJSON error code: "
<< source_json.GetParseError() << " at offset: "
<< source_json.GetErrorOffset();"
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 10
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 May 2026 21:58:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#16).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,490 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/16
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 16
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 18:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/13506/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 18
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sun, 17 May 2026 07:38:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 18: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 18
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sun, 17 May 2026 12:04:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 17:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/22471/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 17
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sun, 17 May 2026 01:56:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#4).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,078 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/4
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 4
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 3:
(2 comments)
Addressed code review from Jason Fehr
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@79
PS2, Line 79: static unordered_map<string, string> BuildAliasReverseMap(
> Will double quotes within the string also mess up JSON serialization?
No, rapidjson::Writer::String(...) escapes embedded quotes inside the payload as \". erase_first/erase_last here only strip the outer JSON quotes, not the escaped internal ones.
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@104
PS2, Line 104: // Adds hostname-like regex matches to the provided deduplicated token set.
> Why return a vector instead of just returning the unique set?
We use a vector here in order to maintain consistent ordering for matches found, so that when aliases are applied they can be assigned deterministically (helps especially with avoiding flaky golden file comparison for unit tests).
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 3
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 13 May 2026 01:50:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#9).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,288 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/9
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 9
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@166
PS6, Line 166: for (size_t nonce = 0;; ++nonce) {
We should probably add a boundary limit here (maybe 10,000) to avoid a potential infinite loop. If it exceeds the boundary, it would be good to return an error Status and fail the operation. Making the limit configurable could be nicer
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@244
PS6, Line 244: if (node.HasMember("info_strings") && node["info_strings"].IsArray()) {
: for (const auto& entry : node["info_strings"].GetArray()) {
: const auto [key, value] = ParseInfoStringEntry(entry);
: if (key == nullptr || value == nullptr) continue;
: if (strcmp(key, "Select Columns") == 0 || strcmp(key, "Where Columns") == 0
: || strcmp(key, "Join Columns") == 0) {
: if (*value != '\0') contexts.push_back(value);
: } else if (strcmp(key, "Analyzed query") == 0) {
: if (*value != '\0') contexts.push_back(value);
: } else if (strcmp(key, "Plan") == 0) {
: string analyzed_query = ExtractAnalyzedQueryFromPlanText(value);
: if (!analyzed_query.empty()) contexts.push_back(analyzed_query);
: }
: }
: }
:
: for (auto it = node.MemberBegin(); it != node.MemberEnd(); ++it) {
: auto child_contexts = CollectIdentifierContextsFromJsonProfile(it->value);
: contexts.insert(
: contexts.end(), std::make_move_iterator(child_contexts.begin()),
: std::make_move_iterator(child_contexts.end()));
: }
It seems we process the info_strings node twice here. Also, calling HasMember and then node["info_strings"] before the for loop results in redundant searches. We can combine this into a single loop to save some searches and prevent the double recursion if the order doesn't matter:
"
for (auto it = node.MemberBegin(); it != node.MemberEnd(); ++it) {
if (strcmp(it->name.GetString(), "info_strings") == 0) {
if (it->value.IsArray()) {
for (const auto& entry : it->value.GetArray()) {
...
}
}
continue;
}
auto child_contexts = CollectIdentifierContextsFromJsonProfile(it->value);
contexts.insert(
contexts.end(), std::make_move_iterator(child_contexts.begin()),
std::make_move_iterator(child_contexts.end()));
}
"
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@405
PS6, Line 405: if (!sql_statements.empty()) {
: const string& sql_statement = sql_statements.front();
: boost::algorithm::replace_all(
: redacted, JsonEscapeString(sql_statement), REDACTED_SQL_STATEMENT);
: }
Do we expect all the 'Sql Statement' values to be exactly the same? I think we should use a for loop to replace all of them just to be safe. Also, do you think we should deduplicate the results inside CollectInfoStringValuesByKeys?
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 6
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 23:16:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction-test.cc
File be/src/service/query-profile-redaction-test.cc:
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction-test.cc@79
PS4, Line 79: TEST(QueryProfileRedactionTest, RedactedProfileMatchesGoldenForTpcds72) {
> Added a unit test for this edge case
Thanks. I think we can also add some negative test cases to verify it is not crashing:
1. Empty string input and some random malformed JSON as the input profile_text.
2. Normal profile_text input, but unredact with some unrelated random text.
Besides, could we also have a test to cover all the regex patterns (EMAIL_RE, IP_RE, SNAKE_CASE_ID_RE, HOST_WITH_PORT_RE, etc.)? It would be great to have a simple, isolated test case with a dummy JSON string to ensure the regexes catch them all correctly and don't regress in the future
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@197
PS6, Line 197: // Builds aliases from tokens, applies them to text, and tracks reverse mappings.
nit. The comment is better to sit above the code, we can move it above the template
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 6
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 21:32:45 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#10).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,366 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/10
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 10
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#13).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,388 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/13
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 13
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 13:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/22461/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 13
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 03:43:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 3:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/24282/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/24282/2//COMMIT_MSG@12
PS2, Line 12: targeted
> Nit: targeted
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction-test.cc
File be/src/service/query-profile-redaction-test.cc:
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction-test.cc@34
PS2, Line 34: static constexpr const char* PROFILE_FILE = "tpcds_72_local_run_profile.json";
> Instead of using an anonymous namespace, add the "static" keyword to the in
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction-test.cc@140
PS2, Line 140:
:
:
:
:
> Can the unredacted output be compared to the tpcds_72_local_run_profile.jso
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.h
File be/src/service/query-profile-redaction.h:
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.h@36
PS3, Line 36: std::unordered_map<std::string, std::string> alias_to_original_;
This map is sorted in a couple different places. Declare it as a std::map instead so the sorting happens automatically.
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@55
PS3, Line 55: string_view token
Use const string_view&. Also on lines 66, 116, 130, 165, 175, 343, 433, 441, 454.
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@136
PS3, Line 136: // Sorts replacement entries by descending token length to avoid partial overlap issues.
: static vector<std::pair<string, string>> SortByTokenLengthDesc(
: vector<std::pair<string, string>> entries) {
: std::sort(entries.begin(), entries.end(),
: [](const std::pair<string, string>& a, const std::pair<string, string>& b) {
: return a.first.size() > b.first.size();
: });
: return entries;
: }
This function is only called from one place, move the code to ApplySortedReplacements and delete this function. Another option is to use a std::set with a custom comparison function which would be much cleaner overall.
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@344
PS3, Line 344: shared_ptr<unordered_map<string, string>>
A shared_ptr doesn't provide value here. Pass in an unordered_map<string, string>&.
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@376
PS3, Line 376: } else {
: unordered_map<string, string> aliases = BuildAliasMap(
: vector<string>(username_tokens.cbegin(), username_tokens.cend()), "user");
: redacted = ApplyAliasMap(aliases, redacted);
It does not seem possible for this else case to happen. Same on lines 388, 400, and 422.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 3
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 13 May 2026 19:35:36 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 4:
(18 comments)
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.h
File be/src/service/query-profile-redaction.h:
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.h@30
PS3, Line 30: const std::string_view& profi
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.h@31
PS3, Line 31: const std::string_vie
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.h@36
PS3, Line 36: std::map<std::string, std::string> alias_to_original_;
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@49
PS3, Line 49: static constexpr const char* REDACTED_SQL_STATEMENT = "[REDACTED_SQL_STATEMENT]";
:
: // Escapes a string so it can be safely matched within JSON-serialized text.
: s
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@55
PS3, Line 55: ic_cast<rapidjson
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@71
PS3, Line 71: return alias_to_original;
: }
:
: sta
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@99
PS3, Line 99: ;
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@100
PS3, Line 100: vector<string> host_sections =
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@117
PS3, Line 117: size_t idx = 0;
: for (const string& token : sorted_tokens) {
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@136
PS3, Line 136: }
: return text;
: }
:
: // Applies a full alias map to a text blob.
: static string ApplyAliasMap(const unordered_map<string, string>& alias_map, string text) {
: return ApplySortedReplacements(
: vector<std::pair<string, string>>(alias_map.cbegin(), alias_map.cend()),
:
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@183
PS3, Line 183: if (node.IsArray()) {
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@335
PS3, Line 335: CollectInfoStringValuesByKeys(source_json, {"Sql Statement"});
: for (const string& sql_statement : sql_statements) {
: boost::algorithm::replace_all(
: redacted, JsonEscapeString(sql_statement), REDAC
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@344
PS3, Line 344: nst std::regex user_kv_re(
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@376
PS3, Line 376: static string UnredactTextWithAliases(
: const string_view& text, const map<string, string>& alias_to_original) {
: if (alias_to_original.empty()) return text;
: return ApplySortedReplacements(vector<std::pai
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@441
PS3, Line 441:
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@455
PS3, Line 455:
> Done
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@74
PS4, Line 74: static vector<string> CollectInfoStringValuesByKeys(
: const Value& node, const unordered_set<string>& target_keys);
No need to have a separate declaration for static functions.
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@96
PS4, Line 96: static const std::regex HOST_WITH_PORT_RE(
: R"(\b([A-Za-z][A-Za-z0-9-]*(?:\.[A-Za-z0-9-]+)*)\:(\d{2,5})\b)");
Declare all regexs after line 49 like this:
static const std::regex HOST_WITH_PORT_RE(R"(\b([A-Za-z][A-Za-z0-9-]*(?:\.[A-Za-z0-9-]+)*)\:(\d{2,5})\b)", regex::optimize);
If capturing groups are not needed, also include the regex::nosubs flag:
static const std::regex HOST_WITH_PORT_RE(R"(\b([A-Za-z][A-Za-z0-9-]*(?:\.[A-Za-z0-9-]+)*)\:(\d{2,5})\b)", regex::optimize | regex::nosubs);
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 4
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 13 May 2026 22:51:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/22450/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 9
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 May 2026 18:36:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 5:
(8 comments)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction-test.cc
File be/src/service/query-profile-redaction-test.cc:
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction-test.cc@124
PS5, Line 124: "value": "select decoy_table_002.id from decoy_table_002 join original_target on decoy_table_002.id = original_target.id"
line too long (127 > 90)
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@75
PS5, Line 75: R"(\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\b)",
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@303
PS5, Line 303: static const unordered_set<string> HOST_SECTION_KEYS = {"Per Host Min Memory Reservation"};
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@309
PS5, Line 309: for (std::sregex_iterator it(section.cbegin(), section.cend(), HOST_WITH_PORT_RE), end;
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@344
PS5, Line 344: if (last_dot == string_view::npos || last_dot == 0 || last_dot + 1 >= fq.size()) continue;
line too long (96 > 90)
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@414
PS5, Line 414: redacted = BuildApplyAndTrackAliases(username_tokens, "user", redacted, alias_to_original);
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@421
PS5, Line 421: redacted = BuildApplyAndTrackAliases(table_tokens, "table", redacted, alias_to_original);
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@422
PS5, Line 422: redacted = BuildApplyAndTrackAliases(column_tokens, "column", redacted, alias_to_original);
line too long (93 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 5
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 20:11:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#7).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,275 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/7
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 7
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 12:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@115
PS11, Line 115: static bool IsValidIpv6Address(const string& candidate) {
> use unordered_set<string_view> instead since you're just using it for compa
Done
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@137
PS11, Line 137: bool has_colon = false;
> Use unordered_set<string_view> to avoid unnecessary copies.
Done
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@289
PS11, Line 289: }
> BuildAliasReverseMap seems unnecessary can can possibly be removed? We coul
Done
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@319
PS11, Line 319: continue;
> we're returning a vector with every recursion and doing too many allocation
Done
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@358
PS11, Line 358:
> We could pass the vector by reference down the recursion tree.
Done
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@395
PS11, Line 395: unordered_set<string_view> table_set;
> We need ordering? If not we could just use a vector<string> here. We could
Done
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@516
PS11, Line 516: static string UnredactTextWithAliases(
> We're calling BuildApplyAndTrackAliases multiple times and essentially scan
Done
http://gerrit.cloudera.org:8080/#/c/24282/11/be/src/service/query-profile-redaction.cc@573
PS11, Line 573:
> local map seems unnecessary? You could directly pass `alias_to_original` an
Done
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 12
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 02:45:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 2:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/22383/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 2
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Mon, 11 May 2026 22:21:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 6:
(34 comments)
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction-test.cc
File be/src/service/query-profile-redaction-test.cc:
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction-test.cc@79
PS4, Line 79: TEST(QueryProfileRedactionTest, RedactedProfileMatchesGoldenForTpcds72) {
> It seems we should add more tests here. One kind of test case I think we ne
Added a unit test for this edge case
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction-test.cc
File be/src/service/query-profile-redaction-test.cc:
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction-test.cc@124
PS5, Line 124: "value": "select decoy_table_002.id from decoy_table_002 "
> line too long (127 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@138
PS3, Line 138:
> Pass entries by reference.
This method has been removed as a result of CR changes
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@148
PS3, Line 148: if (a.first.size() != b.first.size()) return a.first.size() > b.first.size();
> Pass parameters by reference.
This method has been removed as a result of CR changes
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@67
PS4, Line 67: static const std::regex EMAIL_RE(
> Since alias_to_original basically points to entries in original_to_alias we
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@74
PS4, Line 74: static const std::regex IP_RE(
: R"(\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}"
> No need to have a separate declaration for static functions.
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@80
PS4, Line 80: static string JsonEscapeString(const string_view& input) {
> use unordered_set<string_view> since ordering is not required and is more e
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@95
PS4, Line 95: unordered_map<string_view, string_view> alias_to_original;
> We could use one of these sections, maybe "Per Host Min Memory Reservation"
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@96
PS4, Line 96: alias_to_original.reserve(original_to_alias.size());
: for (const auto& entry : original_to_alias) {
> Declare all regexs after line 49 like this:
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@99
PS4, Line 99: }
> We probably don't need an ordered set here so maybe use unordered_set.
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@115
PS4, Line 115: vector<string> results;
> We don't need a temporary set here. We can simply iterate through the input
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@129
PS4, Line 129: for (const string& token : tokens) {
: if (token.empty() || seen_tokens.find(token) != seen_tokens.end()) continue;
: seen_tokens.insert(token);
: std::stringstream alias;
: alias << prefix << "_" << std::setfill('0') << std::setw(3) << (++idx);
: alias_map[token] = alias.str();
: }
: r
> The logic here seems vulnerable. For example, if the original tables (decoy
Updated this logic to do two passes. The first pass unredacts aliases into temporary unique names that won't get confused for other aliases, then the second pass unredacts these temporary unique names into the original strings.
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@143
PS4, Line 143: for (const auto& entry : replacements) {
> Not sure why we need a temporary vector here ? We can pass the map by refer
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@154
PS4, Line 154: static bool ReplacementTokenCollides(const string& token, const string& text,
> Pass alias_to_original by reference to BuildAliasReverseMap to avoid temp c
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@154
PS4, Line 154: static bool ReplacementTokenCollides(const string& token, const string& text,
> Actually, this looks like copy elision so no change needed.
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@164
PS4, Line 164: static string BuildReplacementToken(size_t idx, const string& text,
> avoid local copy here, either pass string by reference to this function or
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@263
PS4, Line 263: contexts.end(), std::make_move_iterator(child_contexts.begin()),
: std::make_move_it
> use unordered_set.
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@266
PS4, Line 266: return contexts;
> This function is non trivial. Maybe add comments explaining what each of th
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@328
PS4, Line 328: if (source_json.HasParseError() || !source_json.IsObject()) return {};
> We're making 2 copies here and in the following line. We need one copy for
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@334
PS4, Line 334: const vector<string>& contexts) {
> This is also using copy elision so we are good.
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@334
PS4, Line 334: const vector<string>& contexts) {
> Either pass sql_statements by reference to CollectInfoStringValuesByKeys or
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@336
PS4, Line 336: unordered_set<string_view> column_set;
> Do we ever have multiple "Sql Statement" in the profile?
Updated logic to only use the first SQL statement occurrence.
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@339
PS4, Line 339: for (const string& context : contexts) {
> This seems unnecessary since we've already redacted above ?
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@377
PS4, Line 377: if (match.length(1) == 0) continue;
> We can use an unordered_map unless we really need ordered replacements in L
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@387
PS4, Line 387: table_tokens.reserve(table_set.size());
> This seems redundant? We can directly pass profile_text.data() and profile_
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@387
PS4, Line 387: table_tokens.reserve(table_set.size());
: for (const string_view token : table_set) table_tokens.emplace_back(token);
: vector<string> column_tokens;
: column_tokens.reserve(column_set.size());
: for (const string_view token : column_set) column_tokens.emplace_back(token);
: s
> We will call RedactQueryProfileWithAliases() later, but RedactQueryProfileW
Removed redundant redaction
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@393
PS4, Line 393: std::sort(column_tokens.begin(), column_tokens.end());
: return {table_tokens, column_tokens};
: }
:
> Is there a specific reason alias_to_original_ needs to be an ordered std::m
Done
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@75
PS5, Line 75: R"(\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}"
> line too long (93 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@303
PS5, Line 303: static vector<string> ExtractHostTokensFromPerHostSections(const Value& source_json) {
> line too long (93 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@309
PS5, Line 309: CollectInfoStringValuesByKeys(source_json, HOST_SECTION_KEYS);
> line too long (91 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@344
PS5, Line 344: if (match.length(1) == 0) continue;
> line too long (96 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@414
PS5, Line 414: for (const string& value : user_values) username_tokens.insert(value);
> line too long (93 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@421
PS5, Line 421: redacted = BuildApplyAndTrackAliases(
> line too long (91 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@422
PS5, Line 422: username_tokens, "user", redacted, alias_to_original);
> line too long (93 > 90)
Done
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 6
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 20:40:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@75
PS6, Line 75: R"(\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}"
This regex seems broken, it seems to be R"(\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3})", which was missing the last ')', can you confirm it?
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 6
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 21:56:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 7:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@142
PS7, Line 142: static vector<pair<string_view, string_view>> GetSortedReplacementEntries(
Add comment explaining what this function does.
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@149
PS7, Line 149: sort(entries.begin(), entries.end(),
Add a comment explaining why we need to sort the entries.
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@157
PS7, Line 157: static bool ReplacementTokenCollides(const string& token, const string& text,
Add comments explaining what this function does.
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@167
PS7, Line 167: static Status BuildReplacementToken(size_t idx, const string& text,
Add comments explaining what this function does.
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@191
PS7, Line 191: vector<pair<string, string>> pending_restores;
We could use vector<pair<string, string_view>> pending_restores as the 'to' entry points to string in entries.
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@201
PS7, Line 201: boost::algorithm::replace_all(text, string(from), token);
We can pass 'from' as is (string_view)
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@202
PS7, Line 202: pending_restores.emplace_back(move(token), string(to));
We can pass 'to' as is (string_view)
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 7
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 May 2026 04:12:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24282/12/be/src/service/query-profile-redaction.h
File be/src/service/query-profile-redaction.h:
http://gerrit.cloudera.org:8080/#/c/24282/12/be/src/service/query-profile-redaction.h@28
PS12, Line 28: class QueryProfileRedactor {
I think we should add a detailed comment to the QueryProfileRedactor class to explain:
Purpose and usage: What the class does and, at a high level, how it works, the expected lifecycle (e.g., calling Redact exactly once, and how unredact maps aliases back to the original text).
Redacted Types: An explicit list of exactly what data types are currently targeted for redaction (e.g., SQL statements, Usernames/Emails, IPv4/IPv6 addresses, Table/Column names, Hostnames), and an example of the substitution (like original_table_name -> table_001).
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 12
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 03:04:07 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 15:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/22463/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 15
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 05:34:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 8:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@142
PS7, Line 142: // Materializes replacement map entries as string_views and sorts them
> Add comment explaining what this function does.
Done
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@149
PS7, Line 149: entries.emplace_back(entry.first, entry.second);
> Add a comment explaining why we need to sort the entries.
Done
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@157
PS7, Line 157: });
> Add comments explaining what this function does.
Done
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@167
PS7, Line 167: if (entry.first.find(token) != string_view::npos) return true;
> Add comments explaining what this function does.
Done
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@191
PS7, Line 191: static Status ApplyAliasMap(const unordered_map<string, string>& alias_map,
> We could use vector<pair<string, string_view>> pending_restores as the 'to'
Done
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@201
PS7, Line 201:
> We can pass 'from' as is (string_view)
Done
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@202
PS7, Line 202: // Phase 1: replace original tokens with unique sentinels.
> We can pass 'to' as is (string_view)
Done
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@302
PS7, Line 302: values.insert(
: values.end(), make_move_iterator(child_values.begin()),
: make_move_iterator(child_values.end()));
: }
: return values;
: }
: if (!node.IsObject()) return values;
:
: for (auto it = node.MemberBegin(); it != node.MemberEnd(); ++it) {
: if (strcmp(it->name.GetString(), "info_strings") == 0) {
: if (!it->value.IsArray()) continue;
: for (const auto& entry : it->value.GetArray()) {
: const auto [key, value] = ParseInfoStringEntry(entry);
: if (key == nullptr || value == nullptr) continue;
:
> Here is similar to CollectIdentifierContextsFromJsonProfile().
Done
http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@418
PS7, Line 418: tring, string>& alias_to_origin
> Trying to understand what the sql_statement input would be like, is it "SEL
sql_statement comes from entry["value"].GetString() on a parsed rapidjson::Value, which gives the unescaped string content.
*redacted is the original JSON text blob, so the code calls JsonEscapeString(sql_statement) to convert it to the escaped form (\"...\") before replace_all.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 8
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 May 2026 04:44:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 3:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/22399/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 3
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 13 May 2026 02:01:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 15: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/24282/15/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/15/be/src/service/query-profile-redaction.cc@388
PS15, Line 388: unordered_set<string_view> seen_host_tokens;
seen_host_tokens could be removed as profile should only have unique hosts under "Per Host Min Memory Reservation" section.
http://gerrit.cloudera.org:8080/#/c/24282/15/be/src/service/query-profile-redaction.cc@473
PS15, Line 473: const string profile_text_string(profile_text);
We should avoid this copy as profiles could be quite huge. We should update the interfaces (CollectRegexMatches and CollectIpv6Matches) to accept a string_view instead.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 15
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 15:28:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 15: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 15
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 15:53:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Reviewed-on: http://gerrit.cloudera.org:8080/24282
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,460 insertions(+), 0 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 19
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24282/10/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/10/be/src/service/query-profile-redaction.cc@550
PS10, Line 550: return Status("Query profile input must be a valid JSON object");
> Can we add a logging here to print the error info?
On second thought, maybe LOG(WARNING) is good enough for the profile redaction.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 10
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 May 2026 22:07:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 15:
Added query profile size limit for redaction input min(256M, 1% host mem_limit)
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 15
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 05:11:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 14:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/22462/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 14
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 May 2026 05:13:51 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/22451/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 10
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 May 2026 20:40:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 17: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/13505/
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 17
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sun, 17 May 2026 07:07:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction-test.cc
File be/src/service/query-profile-redaction-test.cc:
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction-test.cc@79
PS4, Line 79: TEST(QueryProfileRedactionTest, RedactedProfileMatchesGoldenForTpcds72) {
It seems we should add more tests here. One kind of test case I think we need is to use some mixed edge-case names for original tables or hosts (like test_table_001 or test_host_002) to have unit tests comparing the actual output
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@129
PS4, Line 129: std::sort(entries.begin(), entries.end(),
: [](const std::pair<string, string>& a, const std::pair<string, string>& b) {
: return a.first.size() > b.first.size();
: });
: for (const auto& entry : entries) {
: if (entry.first.empty()) continue;
: boost::algorithm::replace_all(text, entry.first, entry.second);
: }
The logic here seems vulnerable. For example, if the original tables (decoy_table_002, original_target) are aliased to (table_001, table_002), during the unredaction phase, if we replace table_001 with decoy_table_002 first, the text now contains the literal string 'table_002'. In the next pass, when the loop searches for 'table_002' to replace it with 'original_target', it will overwrite the substring inside 'decoy_table_002' by mistake. Can you confirm this?
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@387
PS4, Line 387: const string profile_text_copy(profile_text);
: Document source_json;
: source_json.Parse(profile_text_copy.c_str());
: if (source_json.HasParseError() || !source_json.IsObject()) {
: return Status("Query profile input must be a valid JSON object");
: }
We will call RedactQueryProfileWithAliases() later, but RedactQueryProfileWithAliases() also have
"static string RedactQueryProfileWithAliases(const string_view& profile_text...) {
...
const string profile_text_copy(profile_text);
Document source_json;
source_json.Parse(profile_text_copy.c_str());
if (source_json.HasParseError() || !source_json.IsObject()) return redacted;", it seems that we parse the same 'profile_text' twice. I think maybe we can remove this entirely and return the Status in RedactQueryProfileWithAliases(), or pass the source_json reference to the RedactQueryProfileWithAliases as the input.
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@393
PS4, Line 393: unordered_map<string, string> alias_to_original;
: redacted_profile_text_ = RedactQueryProfileWithAliases(profile_text, alias_to_original);
: alias_to_original_ = map<string, string>(
: alias_to_original.cbegin(), alias_to_original.cend());
Is there a specific reason alias_to_original_ needs to be an ordered std::map? Changing alias_to_original_ to an std::unordered_map seems more clear and allow us to avoid extra copy step
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 4
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 05:30:06 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@154
PS4, Line 154: const auto reverse_aliases = BuildAliasReverseMap(aliases);
> Pass alias_to_original by reference to BuildAliasReverseMap to avoid temp c
Actually, this looks like copy elision so no change needed.
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@334
PS4, Line 334: vector<string> sql_statements =
> Either pass sql_statements by reference to CollectInfoStringValuesByKeys or
This is also using copy elision so we are good.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 4
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 17:32:45 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#6).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,158 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/6
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 6
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 7:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/22440/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 7
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 23:58:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 7:
(18 comments)
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction-test.cc
File be/src/service/query-profile-redaction-test.cc:
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction-test.cc@79
PS4, Line 79: TEST(QueryProfileRedactionTest, RedactedProfileMatchesGoldenForTpcds72) {
> Thanks. I think we can also add some negative test cases to verify it is no
Done
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@51
PS5, Line 51: // Matches the analyzed query subsection embedded in the textual plan.
: static const regex ANALYZED_RE(
: R"(Analyzed query:\s*([\s\S]*?)\n\nF\d+:PLAN FRAGMENT)", regex::optimize);
: // Matches fully-qualified identifiers with at least one dot (e.g. db.tbl.col).
: static const regex QUALIFIED_ID_RE(
: R"(\b([A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*)+)\b)", regex::optimize);
: // Matches table tokens following FROM/JOIN clauses.
: static const regex FROM_JOIN_TABLE_RE(
: R"(\b(?:from|join)\s+([A-Za-z_][A-Za-z0-9_\.]*)\b)",
: regex::optimize | regex::icase);
: // Matches snake_case identifiers that are candidates for column tokens.
: static const regex SNAKE_CASE_ID_RE(
: R"(\b([A-Za-z_][A-Za-z0-9_]*_[A-Za-z0-9_]*)\b)", regex::optimize);
: // Matches e-mail addresses.
: static const regex EMAIL_RE(
: R"([A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,})",
: regex::optimize | regex::nosubs);
: // Matches user/userid key-value pairs like user=alice.
: static const regex USER_KV_RE(
: R"(\b(?:user|uid)=([A-Za-z0-9._@-]+)\b)", regex::optimize | regex::icase);
: // Matches IPv4 addresses.
: static const regex IPV4_RE(
: R"(\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3})"
: R"((?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\b)",
: regex::optimize | regex::nosubs);
> Don't use the std:: prefix unless absolutely necessary to avoid compiler co
Done
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@74
PS6, Line 74: R"((?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\b)",
> We should probably redact IPV6 addresses also.
Done
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@75
PS6, Line 75: regex::optimize | regex::nosubs);
> This regex seems broken, it seems to be R"(\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[
Done
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@50
PS6, Line 50: R"(\b([A-Za-z][A-Za-z0-9-]*(?:\.[A-Za-z0-9-]+)*)\:(\d{2,5})\b)", regex::optimize);
: // Matches the analyzed query subsection embedded in the textual plan.
: static const regex ANALYZED_RE(
: R"(Analyzed query:\s*([\s\S]*?)\n\nF\d+:PLAN FRAGMENT)", regex::optimize);
: // Matches fully-qualified identifiers with at least one dot (e.g. db.tbl.col).
: static const regex QUALIFIED_ID_RE(
: R"(\b([A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*)+)\b)", regex::optimize);
: // Matches table tokens following FROM/JOIN clauses.
: static const regex FROM_JOIN_TABLE_RE(
: R"(\b(?:from|join)\s+([A-Za-z_][A-Za-z0-9_\.]*)\b)",
: regex::optimize | regex::icase);
: // Matches snake_case identifiers that are candidates for column tokens.
: static const regex SNAKE_CASE_ID_RE(
: R"(\b([A-Za-z_][A-Za-z0-9_]*_[A-Za-z0-9_]*)\b)", regex::optimize);
: // Matches e-mail addresses.
: static const regex EMAIL_RE(
: R"([A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,})",
: regex::optimize | regex::nosubs);
: // Matches user/userid key-value pairs like user=alice.
: static const regex USER_KV_RE(
: R"(\b(?:user|uid)=([A-Za-z0-9._@-]+)\b)", regex::optimize | regex::icase);
: // Matches IPv4 addresses.
: static const regex IPV4_RE(
: R"(\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3})"
: R"((?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\b)",
: regex::optimize | regex::nosubs);
: // Matches IPv6 addresses, including compressed forms.
: static const regex IPV6_RE(
> unit tests for theses regexes would be good.
Done
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@98
PS6, Line 98: const unordered_map<string, string>& original_to_alias) {
> we guarantee aliases are unique?
Yes
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@106
PS6, Line 106:
> I'm not sure what the purpose is of this intermediate unordered_set. Why n
Done
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@118
PS6, Line 118: if (!seen_matches.insert(token).second) continue;
> I think I missed why we need to sort the matched results?
Done
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@118
PS6, Line 118: if (!seen_matches.insert(token).second) continue;
> According to the comment at https://gerrit.cloudera.org/c/24282/4..5/be/src
Done
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@126
PS6, Line 126: static unordered_map<string, string> BuildAliasMap(
> seen_tokens is unecessary as you can look up in the alias_map.
Done
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@132
PS6, Line 132: string alias(prefix);
: alias.push_back('_');
> We could avoid stringstream altogether and do string manipulation using app
Done
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@134
PS6, Line 134: string idx_string = to_string(++idx);
> when possible use emplace.
Done
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@166
PS6, Line 166:
> We should probably add a boundary limit here (maybe 10,000) to avoid a pote
Done
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@197
PS6, Line 197: if (from.empty()) continue;
> nit. The comment is better to sit above the code, we can move it above the
Done
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@244
PS6, Line 244: }
: return {entry["key"].GetString(), entry["value"].GetString()};
: }
:
: // Recursively collects profile text contexts that contain query identifiers.
: static vector<string> CollectIdentifierContextsFromJsonProfile(const Value& node) {
: vector<string> contexts;
: if (node.IsArray()) {
: for (const auto& item : node.GetArray()) {
: auto child_contexts = CollectIdentifierContextsFromJsonProfile(item);
: contexts.insert(
: contexts.end(), make_move_iterator(child_contexts.begin()),
: make_move_iterator(child_contexts.end()));
: }
: return contexts;
: }
: if (!node.IsObject()) return contexts;
: for (auto it = node.MemberBegin(); it != node.MemberEnd(); ++it) {
: if (strcmp(it->name.GetString(), "info_strings") == 0) {
: if (!it->value.IsArray()) continue;
: for (const auto& entry : it->value.GetArray()) {
:
> It seems we process the info_strings node twice here. Also, calling HasMemb
Done
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@307
PS6, Line 307: if (*value != '\0') values.push_back(value);
> Now that BuildApplyAndTrackAliases is a templatized function, there is no n
Done
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@392
PS6, Line 392: for (sregex_iterator it(context.cbegin(), context.cend(), SNAKE_CASE_ID_RE), end;
: it != end; ++it) {
> Is sorting required?
Comment no longer applicable
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@405
PS6, Line 405:
: // Redacts sensitive profile values and optionally records alias-to-original mappings.
: static Status RedactQueryProfileWithAliases(const string_view& profile_text,
: const Value& source_json,
:
> Do we expect all the 'Sql Statement' values to be exactly the same? I think
We used to iterate over all matches but based on earlier review comments decided to switch to only the first match because the SQL Statement section should only show up once in the profile (this is intended to find the original raw SQL statement because it may contain non-fully-qualified table/column names). As for other recreations of the SQL statement in the profile such as in the Analyzed Query section, we're okay with skipping those because we will catch any fully qualified table/column names in those sections during table/column redaction.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 7
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 23:50:49 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24282/8/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/8/be/src/service/query-profile-redaction.cc@215
PS8, Line 215: alias_to_original.reserve(alias_to_original.size() + reverse_aliases.size());
> replace_all is O(N^2) and we could have some really large profiles. We shou
Done
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 9
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 May 2026 18:15:19 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#15).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,453 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/15
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 15
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 6:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@74
PS4, Line 74: static const std::regex IP_RE(
: R"(\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}"
> No need to have a separate declaration for static functions.
Done
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@96
PS4, Line 96: alias_to_original.reserve(original_to_alias.size());
: for (const auto& entry : original_to_alias) {
> Declare all regexs after line 49 like this:
Done
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/5/be/src/service/query-profile-redaction.cc@51
PS5, Line 51: static const std::regex HOST_WITH_PORT_RE(
: R"(\b([A-Za-z][A-Za-z0-9-]*(?:\.[A-Za-z0-9-]+)*)\:(\d{2,5})\b)", regex::optimize);
: // Matches the analyzed query subsection embedded in the textual plan.
: static const std::regex ANALYZED_RE(
: R"(Analyzed query:\s*([\s\S]*?)\n\nF\d+:PLAN FRAGMENT)", regex::optimize);
: // Matches fully-qualified identifiers with at least one dot (e.g. db.tbl.col).
: static const std::regex QUALIFIED_ID_RE(
: R"(\b([A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*)+)\b)", regex::optimize);
: // Matches table tokens following FROM/JOIN clauses.
: static const std::regex FROM_JOIN_TABLE_RE(
: R"(\b(?:from|join)\s+([A-Za-z_][A-Za-z0-9_\.]*)\b)",
: regex::optimize | regex::icase);
: // Matches snake_case identifiers that are candidates for column tokens.
: static const std::regex SNAKE_CASE_ID_RE(
: R"(\b([A-Za-z_][A-Za-z0-9_]*_[A-Za-z0-9_]*)\b)", regex::optimize);
: // Matches e-mail addresses.
: static const std::regex EMAIL_RE(
: R"([A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,})",
: regex::optimize | regex::nosubs);
: // Matches user/userid key-value pairs like user=alice.
: static const std::regex USER_KV_RE(
: R"(\b(?:user|uid)=([A-Za-z0-9._@-]+)\b)", regex::optimize | regex::icase);
: // Matches IPv4 addresses.
: static const std::regex IP_RE(
: R"(\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}"
Don't use the std:: prefix unless absolutely necessary to avoid compiler confusion.
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@106
PS6, Line 106: unordered_set<string_view> unique_views;
I'm not sure what the purpose is of this intermediate unordered_set. Why not just directly emplace strings into the results vector?
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@118
PS6, Line 118: std::sort(results.begin(), results.end());
According to the comment at https://gerrit.cloudera.org/c/24282/4..5/be/src/service/query-profile-redaction.cc#b80, the results do not need to be sorted. Is this sort necessary?
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@307
PS6, Line 307: vector<string> host_tokens;
Now that BuildApplyAndTrackAliases is a templatized function, there is no need for this second vector. Return seen_host_tokens instead.
http://gerrit.cloudera.org:8080/#/c/24282/6/be/src/service/query-profile-redaction.cc@392
PS6, Line 392: std::sort(table_tokens.begin(), table_tokens.end());
: std::sort(column_tokens.begin(), column_tokens.end());
Is sorting required?
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 6
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 21:27:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 11:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/22453/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 11
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 May 2026 22:45:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 17: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 17
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sun, 17 May 2026 01:44:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 3:
(30 comments)
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.h
File be/src/service/query-profile-redaction.h:
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.h@30
PS3, Line 30: std::string_view profile_text
Nit: declare this as const std::string_view&
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.h@31
PS3, Line 31: std::string_view text
Nit: declare this as const std::string_view&
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.h
File be/src/service/query-profile-redaction.h:
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.h@25
PS2, Line 25:
> It may be cleaner to define a class that encompasses these functions. Then
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.h@27
PS2, Line 27:
> use std::string, same comment for lines 28 and 30
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.h@28
PS2, Line 28: s QueryProfileRedactor {
> If possible, use std::string_view instead of const string&. Same comment f
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.h@37
PS2, Line 37: };
> What is the need for this function? Can't the original profile be used ins
This function performs partial unredaction so the results from the LLM show the original values from the query profile.
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@35
PS2, Line 35: #include <rapidjson/document.h>
> add using namespace std;
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@46
PS2, Line 46:
> General style feedback:
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@50
PS2, Line 50: static string Trim(string_view s) {
: return boost::algorithm::trim_copy(string(s));
: }
:
: // Heuristically checks whether a token looks like a hostname-like identifier.
: static bool IsLikelyHostnameToken(string_view token) {
: if (token.empty()) return false;
:
> use boost::trim instead of writing a trim function
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@62
PS2, Line 62: return false;
: }
:
> No need for these variables, just return true from the if statements on lin
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@73
PS2, Line 73: escaped = boost::algorithm::erase_last_copy(escaped, "\"");
> Typically escaping strings means to add a backslash before control characte
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@79
PS2, Line 79: static unordered_map<string, string> BuildAliasReverseMap(
> No, rapidjson::Writer::String(...) escapes embedded quotes inside the paylo
Ack
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@87
PS2, Line 87:
> Return a new unordered_map instead of having a pointer passed in.
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@97
PS2, Line 97: if (!token.empty()) unique.insert(token);
> Unless ordering is needed, use unordered_set instead of set.
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@100
PS2, Line 100: in(), results.en
> Shouldn't need this static_cast. May need to define the type of group_inde
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@104
PS2, Line 104: // Adds hostname-like regex matches to the provided deduplicated token set.
> We use a vector here in order to maintain consistent ordering for matches f
Ack
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@109
PS2, Line 109: ameToken(token)) host_to
> Avoid raw pointers as function inputs in favor of returning an unordered_se
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@121
PS2, Line 121: std::stringstream alias;
> Could use for (int idx=0; idx < tokens.size(); idx++) instead of declaring
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@129
PS2, Line 129: static string ReplaceAllOccurrences(
: string_view token, string_view replacement, string text) {
: if (token.empty()) return text;
: boost::algorithm::replace_all(text, string(token), string(replacement));
: return text;
: }
:
: // Sorts replacement entries by descending token length to avoid partial overlap issues.
: s
> Instead of a custom function, use boost functions to replace all occurrence
Done
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@289
PS2, Line 289: tor it(context.c
> Avoid this static cast, define i with a type of size_t
Done
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@49
PS3, Line 49: // Trims leading and trailing whitespace from input text.
: static string Trim(string_view s) {
: return boost::algorithm::trim_copy(string(s));
: }
Eliminate this function entirely and call boost::trim_copy directly. As written now, it makes a copy of the underlying string from the string_view. This duplicate copy is unnecessary.
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@71
PS3, Line 71: if (escaped.size() >= 2 && escaped.front() == '"' && escaped.back() == '"') {
: escaped = boost::algorithm::erase_first_copy(escaped, "\"");
: escaped = boost::algorithm::erase_last_copy(escaped, "\"");
: }
This code will not handle the situation where either the first or last (but not both) double quotes is missing.
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@99
PS3, Line 99: unique.begin(), unique.end()
Use .cbegin() and .cend() here.
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@100
PS3, Line 100: std::sort(results.begin(), results.end());
Since ordering is needed, use a set instead of unordered_set so that sorting happens automatically at insert time.
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@117
PS3, Line 117: vector<string> sorted_tokens = tokens;
: std::sort(sorted_tokens.begin(), sorted_tokens.end());
Instead of sorting a vector, can tokens be a std::multiset or std::set?
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@128
PS3, Line 128: // Replaces every occurrence of a token with a replacement string.
: static string ReplaceAllOccurrences(
: string_view token, string_view replacement, string text) {
: if (token.empty()) return text;
: boost::algorithm::replace_all(text, string(token), string(replacement));
: return text;
: }
No need for this function, call boost::algorithm::replace_all or boost::algorithm::replace_all_copy directly instead.
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@183
PS3, Line 183: return Trim(m[1].str());
return boost::algorithm::trim_copy which will enable return value optimization to avoid an unnecessary string create.
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@335
PS3, Line 335: vector<string> table_tokens(table_set.cbegin(), table_set.cend());
: vector<string> column_tokens(column_set.cbegin(), column_set.cend());
: std::sort(table_tokens.begin(), table_tokens.end());
: std::sort(column_tokens.begin(), column_tokens.end());
Declare table_set and column_set as type std::set<string> and the sorting will happen automatically at insert time.
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@441
PS3, Line 441: Status QueryProfileRedactor::Redact(string_view profile_text) {
Ensure this function does not get called twice by adding a DCHECK:
DCHECK(redacted_profile_text_.empty()) << "Cannot call Redact function more than once";
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@455
PS3, Line 455: return UnredactTextWithAliases(text, alias_to_original_);
Ensure the Redact function has already been called by adding a DCHECK:
DCHECK(!redacted_profile_text_.empty()) << "Redact function has not been called, no profile to unredact";
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 3
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 13 May 2026 19:09:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 4:
(4 comments)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@103
PS4, Line 103: for (std::sregex_iterator it(section.cbegin(), section.cend(), HOST_WITH_PORT_RE), end;
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@356
PS4, Line 356: redacted = BuildApplyAndTrackAliases(username_tokens, "user", redacted, alias_to_original);
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@366
PS4, Line 366: redacted = BuildApplyAndTrackAliases(table_tokens, "table", redacted, alias_to_original);
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@367
PS4, Line 367: redacted = BuildApplyAndTrackAliases(column_tokens, "column", redacted, alias_to_original);
line too long (93 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 4
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 13 May 2026 21:51:48 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#5).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,147 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/5
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 5
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 6:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/22434/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 6
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 20:37:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@50
PS3, Line 50: static string Trim(string_view s) {
This is called in a single place so you could also embed `boost::algorithm::trim_copy(string(s))` in the code instead.
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@55
PS3, Line 55: static bool IsLikelyHostnameToken(string_view token) {
I think instead of regex search we could rely on extracting hostnames based on following sections (lines) which we know will be there in the query profiles?
```
Per Host Min Memory Reservation: d1c0917.pcd.dfw.cloudera.com:27000(0) d1c0919.pcd.dfw.cloudera.com:27000(1.00 MB) d1c0916.pcd.dfw.cloudera.com:27000(140.94 MB)
Per Host Number of Fragment Instances: d1c0917.pcd.dfw.cloudera.com:27000(0) d1c0919.pcd.dfw.cloudera.com:27000(1) d1c0916.pcd.dfw.cloudera.com:27000(4)
```
http://gerrit.cloudera.org:8080/#/c/24282/3/be/src/service/query-profile-redaction.cc@70
PS3, Line 70: string escaped = string(buffer.GetString(), buffer.GetSize());
: if (escaped.size() >= 2 && escaped.front() == '"' && escaped.back() == '"') {
: escaped = boost::algorithm::erase_first_copy(escaped, "\"");
: escaped = boost::algorithm::erase_last_copy(escaped, "\"");
: }
We're doing 3 copies here L70, L72 and L73. One copy is unavoidable so we should aim for that, so maybe something like:
```
const char* p = buffer.GetString();
const size_t n = buffer.GetSize();
if (n >= 2 && p[0] == '"' && p[n - 1] == '"') {
return string(p + 1, n - 2);
}
return string(p, n);
```
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 3
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 13 May 2026 18:31:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@67
PS4, Line 67: unordered_map<string, string> alias_to_original;
Since alias_to_original basically points to entries in original_to_alias we could use string_view to avoid deep copy.
unordered_map<string, string> alias_to_original;
```
std::unordered_map<std::string_view, std::string_view> alias_to_original;
alias_to_original.reserve(original_to_alias.size());
for (const auto& entry : original_to_alias) {
alias_to_original[entry.second] = entry.first;
}
```
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@80
PS4, Line 80: set<string> unique;
use unordered_set<string_view> since ordering is not required and is more efficient for inserts.
Also, using string_view you avoid deep copies.
Using string_view we can also avoid creating a temp string when we insert into 'unique'.
```
std::unordered_set<std::string_view> unique_views;
for (std::sregex_iterator it(text.cbegin(), text.cend(), pattern), end;
it != end; ++it) {
....
unique_views.emplace(text.data() + match.position(group_index),
match.length(group_index));
}
vector<string> results;
results.reserve(unique_views.size());
for (const auto& sv : unique_views) {
results.emplace_back(sv);
}
```
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@263
PS4, Line 263: set<string> table_set;
: set<string> column_set;
use unordered_set.
The regex match loops create many temporary strings we could use string_view instead. Also, stringstream can probably be avoided.
http://gerrit.cloudera.org:8080/#/c/24282/4/be/src/service/query-profile-redaction.cc@266
PS4, Line 266: static const std::regex QUALIFIED_ID_RE(
This function is non trivial. Maybe add comments explaining what each of the regex patterns match?
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 4
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 May 2026 19:17:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24282 )
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
Patch Set 2:
(2 comments)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.h
File be/src/service/query-profile-redaction.h:
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.h@28
PS2, Line 28: const string& profile_text, std::unordered_map<std::string, std::string>* alias_to_original);
line too long (97 > 90)
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:
http://gerrit.cloudera.org:8080/#/c/24282/2/be/src/service/query-profile-redaction.cc@363
PS2, Line 363: R"(\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\b)");
line too long (96 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 2
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Mon, 11 May 2026 22:12:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14961: Query Profile Redaction
Posted by "Gokul Kolady (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Abhishek Rawat, Jason Fehr, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24282
to look at the new patch set (#11).
Change subject: IMPALA-14961: Query Profile Redaction
......................................................................
IMPALA-14961: Query Profile Redaction
The query profile redaction component protects sensitive profile content
by replacing user, host, IP, SQL, table, and column identifiers with
deterministic aliases while preserving enough structure for analysis.
Table and column identifiers are targeted for redaction if they
appear in the select columns, where columns, join columns, and analyzed
query sections of the profile. The component also provides a reverse
unredaction path that restores aliases in a string back to original
identifiers using the captured alias map.
Tests added: query-profile-redaction-test.cc with golden-file coverage
for a TPC-DS profile redaction output, validation of alias-based
unredaction behavior, and negative tests ensuring non-JSON profile input
is rejected for both redaction and unredaction paths.
Code Generated-by: Cursor AI
Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
---
M be/src/service/CMakeLists.txt
A be/src/service/query-profile-redaction-test.cc
A be/src/service/query-profile-redaction.cc
A be/src/service/query-profile-redaction.h
A testdata/impala-profiles/query-profile-redaction-expected/tpcds_72_local_run_profile.redacted.json
A testdata/impala-profiles/tpcds_72_local_run_profile.json
6 files changed, 108,387 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24282/11
--
To view, visit http://gerrit.cloudera.org:8080/24282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 11
Gerrit-Owner: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Gokul Kolady <go...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>