Repository navigation
[feature](nereids)use mtmv to match legacy mv - #33699
Merged
Merged
Conversation
|
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
starocean999
force-pushed
the
nereids_0714
branch
from
June 4, 2024 11:16
7d8db1f to
9a5e5ef
Compare
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 41237 ms |
TPC-DS: Total hot run time: 172130 ms |
ClickBench: Total hot run time: 30.91 s |
starocean999
force-pushed
the
nereids_0714
branch
from
June 5, 2024 13:32
9a5e5ef to
c145f6b
Compare
Contributor
Author
|
run buildall |
starocean999
force-pushed
the
nereids_0714
branch
from
June 6, 2024 01:43
c145f6b to
a1cc70e
Compare
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 40948 ms |
TPC-DS: Total hot run time: 172845 ms |
ClickBench: Total hot run time: 30.47 s |
starocean999
force-pushed
the
nereids_0714
branch
from
June 6, 2024 13:26
a1cc70e to
e3027a2
Compare
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 40058 ms |
TPC-DS: Total hot run time: 169660 ms |
ClickBench: Total hot run time: 30.54 s |
starocean999
force-pushed
the
nereids_0714
branch
from
June 7, 2024 10:20
e3027a2 to
73d9d07
Compare
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 39664 ms |
TPC-DS: Total hot run time: 173003 ms |
ClickBench: Total hot run time: 31.4 s |
starocean999
force-pushed
the
nereids_0714
branch
from
June 11, 2024 12:52
73d9d07 to
f7aa3e1
Compare
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 40218 ms |
TPC-DS: Total hot run time: 174348 ms |
ClickBench: Total hot run time: 30.23 s |
starocean999
force-pushed
the
nereids_0714
branch
from
June 12, 2024 04:33
f7aa3e1 to
10d2b3f
Compare
starocean999
force-pushed
the
nereids_0714
branch
from
July 1, 2024 04:02
846c958 to
0d8c3d8
Compare
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 39684 ms |
TPC-DS: Total hot run time: 172168 ms |
ClickBench: Total hot run time: 30.68 s |
starocean999
marked this pull request as ready for review
July 1, 2024 09:35
morrySnow
approved these changes
Jul 1, 2024
englefly
approved these changes
Jul 1, 2024
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions
Bot
added
the
approved
Contributor
|
PR approved by anyone and no changes requested. |
dataroaring
pushed a commit
that referenced
this pull request
Jul 2, 2024
2 of 3 tasks
starocean999
pushed a commit
that referenced
this pull request
Jul 8, 2024
… name is same (#37311) ## Proposed changes This is brought by #33699 `AbstractMaterializedViewRule#isMaterializationValid` check would fail when sync and async materialized view name is same, because the Materialization valid check result is cached by name。this will cause the async materialized view check fail wrongly. This fix this by make sync materialized view name unique in every place.
dataroaring
pushed a commit
that referenced
this pull request
Jul 17, 2024
… name is same (#37311) ## Proposed changes This is brought by #33699 `AbstractMaterializedViewRule#isMaterializationValid` check would fail when sync and async materialized view name is same, because the Materialization valid check result is cached by name。this will cause the async materialized view check fail wrongly. This fix this by make sync materialized view name unique in every place.
16 tasks
HappenLee
pushed a commit
to HappenLee/incubator-doris
that referenced
this pull request
Apr 24, 2026
HappenLee
pushed a commit
to HappenLee/incubator-doris
that referenced
this pull request
Apr 24, 2026
… name is same (apache#37311) ## Proposed changes This is brought by apache#33699 `AbstractMaterializedViewRule#isMaterializationValid` check would fail when sync and async materialized view name is same, because the Materialization valid check result is cached by name。this will cause the async materialized view check fail wrongly. This fix this by make sync materialized view name unique in every place.
morningman
added a commit
to morningman/doris
that referenced
this pull request
Nov 13, 2026
…characters ### Background Several FE features rewrite the text of a statement by cutting the original SQL string at token positions from the parser. Audit masking replaces credential properties and passwords with `*XXX` before the statement reaches `fe.audit.log`, `__internal_schema.audit_log` and the FE log; CREATE / ALTER VIEW and CREATE MATERIALIZED VIEW expand `*` and qualify names in the definition they store; the parser reads `/*+ ... */` hints from comment tokens. The lexer counts its input by Unicode code point, while a Java `String` holds a supplementary character, such as an emoji, as two `char`s. ### What problem does this PR solve? Issue Number: None Related PR: apache#47595 (introduced the bug in audit masking), apache#32743 (introduced the bug in view rewriting), apache#70338 (its review found the masking leak) PR Type: Bugfix Problem Summary: After each supplementary character in a statement, these token positions point one `char` too early, so the text is cut at the wrong place. A credential after or inside such characters stays partly or fully in clear text in the audit log, CREATE VIEW fails with a syntax error, CREATE MATERIALIZED VIEW stores a broken query, and a hint after such a character is silently ignored. This PR converts the positions to `String` offsets, or reads the text through the token, at the five places that cut the original SQL. Tests are 89% of the diff. ### Details - **Change map:** production code +33 -33 in 3 files; tests +240 in 5 files (FE unit tests and one regression suite); generated `.out` +16. (1) `BaseViewInfo.rewriteSql`, shared by audit masking, views and MTMV, converts its positions; the sync-MV builder drops its private copy of it, and `rewriteProjectsToUserDefineAlias` replaces the select list through it. (2) `NereidsParser.getHintMap` reads hints from the token text. (3) `NereidsParser.tryParseExplainPlan` converts the start of the EXPLAIN PLAN body. - **Frequency:** Always, when a character above U+FFFF (an emoji, CJK Extension B) comes before or inside the rewritten part, for example a catalog COMMENT of emoji in front of `'s3.secret_key' = 'secret'`. - **Root cause:** `CaseInsensitiveStream.fromString` keeps ANTLR's code-point stream for any input with a surrogate (before apache#67452 every input used `CharStreams.fromString`), so `Token.getStartIndex()` and `getStopIndex()` count code points. The five places in the Change map used them as `String` indexes. - **Why existing tests missed it:** Unit tests: the masking (`EncryptSQLTest`, `RepositoryAuditEncryptionTest`), view and MTMV (`CreateViewTest`, `CreateMTMVCommandTest`) and hint tests (`LeanTokenModeTest`) use only BMP text, where both kinds of position coincide; `CaseInsensitiveStreamTest` uses an emoji but checks only the stream's own positions. Regression: `audit/test_external_catalog_credential_masking` and the `ddl_p0/create_view_nereids` suites have no supplementary characters, and the masking suite only checks that the whole credential is absent, which a leaked tail passes. - **Fix:** Convert where the text is cut. `rewriteSql` converts each position with `String.offsetByCodePoints`, walking the sorted positions once so the cost stays linear; `rewriteProjectsToUserDefineAlias` reads expressions through the token's input stream; `getHintMap` reads the hint from the token text, as `removeCommentAndTrimBlank` does. Changing the stream to report `String` positions was rejected: apache#67452 made code-point positions part of `CaseInsensitiveStream`'s contract, and `CaseInsensitiveStreamTest` pins them. - **Other paths and branches:** Fixed here: the 19 masking visitors of `LogicalPlanBuilderForEncryption` with every caller of `geneEncryptionSQL` (audit log, FE statement log, `_query_plan`) and the streaming insert job; the view, MTMV and sync-MV rewrites. Not affected: the other uses of token positions read text through the token's own stream (such as `getOriginSql`) or only compare positions with each other. branch-3.1, branch-4.0, branch-4.1 and branch-4.2 have the same code. Introduced by apache#32743 (views), apache#47595 (masking), apache#33699 (sync MV) and apache#33262 (EXPLAIN PLAN); hints since at least apache#39590. ### Testing - New unit tests fail on master `f1bc8b3690c` and pass with the fix, run locally: `EncryptSQLTest#testEncryptionAfterSupplementaryCharacters`, through `MysqlConnectProcessor` to the audit event (emoji inside a credential, in a catalog COMMENT, in the first of two property clauses, in a user name before `PASSWORD(...)`; master left `secret`, `abc`, `123` and `123456` in clear text); `CreateViewTest#testCreateViewRewritesAfterSupplementaryCharacters` (master: `mismatched input '`e`'`); `CreateMTMVCommandTest#testCreateMTMVRewritesQuerySqlAfterSupplementaryCharacters` (master kept ``SELECT '😀😀' A`e` e`internal`...``); `SupplementaryCharacterOffsetTest` (master dropped the hint, cut the EXPLAIN PLAN body to `AN SELECT 100`, and rewrote the sync MV to `avg_stateg(k)))`). - New regression suite `ddl_p0/create_view_nereids/test_create_view_supplementary_chars`: fails without the fix with the same syntax error and passes with it; the `.out` is generated by the script. Run locally on one FE and one BE, an older master FE build with the three changed classes swapped in. - 27 existing FE UT classes around the change (467 tests) pass. Reverting each part of the fix alone, moving the end of a replaced span by one, or not advancing the position cursor each fails a new test; the off-by-one also fails existing ASCII cases. - By hand on that cluster, without the fix: the secret stayed in `audit_log`, `/* 😀 */ select /*+ SET_VAR(query_timeout=1) */ sleep(3)` ran for 3 s, and an MTMV over `select '😀😀' as e, ...` was created but failed to refresh (`Nereids parse failed`). With the fix: `"s3.secret_key" = "*XXX")`, and the query times out after 1 s. ### Notes for Reviewers - `getHintMap` loses its `sql` parameter, which nothing reads any more; its three callers are in `NereidsParser`. - `rewriteProjectsToUserDefineAlias` replaces the select list through `rewriteSql` instead of `StringUtils.overlay`, so the conversion lives in one method. - Stored definitions are not repaired: an MTMV created by an affected version from such a query keeps its broken definition and has to be recreated. ### Release note Fix passwords and credentials left partly or fully unmasked in the audit log and the FE log when a supplementary character, such as an emoji, comes before or inside them. Also fix CREATE VIEW failing, CREATE MATERIALIZED VIEW storing a broken query, and hints such as SET_VAR being ignored after such a character; materialized views already created with a broken query must be recreated. ### Check List (For Author) - Behavior changed: No - Does this need documentation: No - Pre-submission status: Ready ### AI Disclosure - Tool: Claude Code (Claude Opus 5.5) - Generated: the fix, the tests, and this description - Human review: The author reviewed part of the changes. - Prompts: After the review bot on apache#70338 reported the masking leak, the author confirmed it is an existing engine defect shared by all masking paths and asked for a separate engine PR that fixes it, followed by a reply on apache#70338 with the link. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Issue Number: close #xxx
Further comments
If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...