From 9f6f906f6483d2e828525da1ce6c41b751269238 Mon Sep 17 00:00:00 2001 From: morningman Date: Sat, 7 Nov 2026 16:31:53 +0800 Subject: [PATCH] [fix](nereids) Fix SQL masking and view rewrites after supplementary characters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### 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: #47595 (introduced the bug in audit masking), #32743 (introduced the bug in view rewriting), #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 #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: #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 #32743 (views), #47595 (masking), #33699 (sync MV) and #33262 (EXPLAIN PLAN); hints since at least #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 #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 #70338 with the link. Co-Authored-By: Claude Opus 5.5 --- .../parser/LogicalPlanBuilderForSyncMv.java | 17 +--- .../doris/nereids/parser/NereidsParser.java | 19 ++-- .../plans/commands/info/BaseViewInfo.java | 30 +++--- .../apache/doris/catalog/CreateViewTest.java | 20 ++++ .../doris/nereids/parser/EncryptSQLTest.java | 92 +++++++++++++++++++ .../SupplementaryCharacterOffsetTest.java | 64 +++++++++++++ .../trees/plans/CreateMTMVCommandTest.java | 19 ++++ .../test_create_view_supplementary_chars.out | 16 ++++ ...est_create_view_supplementary_chars.groovy | 45 +++++++++ 9 files changed, 289 insertions(+), 33 deletions(-) create mode 100644 fe/fe-core/src/test/java/org/apache/doris/nereids/parser/SupplementaryCharacterOffsetTest.java create mode 100644 regression-test/data/ddl_p0/create_view_nereids/test_create_view_supplementary_chars.out create mode 100644 regression-test/suites/ddl_p0/create_view_nereids/test_create_view_supplementary_chars.groovy diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilderForSyncMv.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilderForSyncMv.java index 1a3e19c8e0d0c6..c53f1392d45ef5 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilderForSyncMv.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilderForSyncMv.java @@ -33,6 +33,7 @@ import org.apache.doris.nereids.trees.expressions.visitor.DefaultExpressionRewriter; import org.apache.doris.nereids.trees.plans.Plan; import org.apache.doris.nereids.trees.plans.commands.CreateMTMVCommand; +import org.apache.doris.nereids.trees.plans.commands.info.BaseViewInfo; import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate; import org.apache.doris.nereids.trees.plans.logical.LogicalPlan; import org.apache.doris.nereids.util.PlanUtils; @@ -109,7 +110,7 @@ public LogicalPlan visitQuery(DorisParser.QueryContext ctx) { for (Expression expr : outputs) { aggStateFunctionFinder.find(expr, indexInSqlToString); } - querySql = Optional.of(rewriteSql(originSql, indexInSqlToString)); + querySql = Optional.of(BaseViewInfo.rewriteSql(indexInSqlToString, originSql)); } else { querySql = Optional.of(originSql); } @@ -193,18 +194,4 @@ public Expression visitUnboundFunction(UnboundFunction unboundFunction, return unboundFunction; } } - - private static String rewriteSql(String querySql, - Map, String> indexStringSqlMap) { - StringBuilder builder = new StringBuilder(); - int beg = 0; - for (Map.Entry, String> entry : indexStringSqlMap.entrySet()) { - Pair index = entry.getKey(); - builder.append(querySql, beg, index.first); - builder.append(entry.getValue()); - beg = index.second + 1; - } - builder.append(querySql, beg, querySql.length()); - return builder.toString(); - } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/NereidsParser.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/NereidsParser.java index 7a30b276da158f..0484c0145dc8d4 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/NereidsParser.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/NereidsParser.java @@ -200,7 +200,10 @@ public static Optional> tryParseExplainPlan(String return Optional.empty(); } ExplainOptions explainOptions = new ExplainOptions(explainLevel, showPlanProcess); - return Optional.of(Pair.of(explainOptions, sql.substring(explainPlanBody.getStartIndex()))); + // The parser counts a supplementary character such as an emoji as one character, while sql + // counts it as two. + return Optional.of(Pair.of(explainOptions, + sql.substring(sql.offsetByCodePoints(0, explainPlanBody.getStartIndex())))); } catch (Throwable t) { return Optional.empty(); } @@ -349,7 +352,7 @@ private T parse(String sql, @Nullable LogicalPlanBuilder logicalPlanBuilder, CommonTokenStream tokenStream = parseLeanTokens(sql); ParserRuleContext tree = toAst(tokenStream, parseFunction); LogicalPlanBuilder realLogicalPlanBuilder = logicalPlanBuilder == null - ? new LogicalPlanBuilder(getHintMap(sql, tokenStream, DorisParser::selectHint)) + ? new LogicalPlanBuilder(getHintMap(tokenStream, DorisParser::selectHint)) : logicalPlanBuilder; return (T) realLogicalPlanBuilder.visit(tree); } @@ -358,7 +361,7 @@ public LogicalPlan parseForCreateView(String sql) { CommonTokenStream tokenStream = parseLeanTokens(sql); ParserRuleContext tree = toAst(tokenStream, DorisParser::singleStatement); LogicalPlanBuilder realLogicalPlanBuilder = new LogicalPlanBuilderForCreateView( - getHintMap(sql, tokenStream, DorisParser::selectHint)); + getHintMap(tokenStream, DorisParser::selectHint)); return (LogicalPlan) realLogicalPlanBuilder.visit(tree); } @@ -378,13 +381,13 @@ public Optional parseForSyncMv(String sql) { CommonTokenStream tokenStream = parseLeanTokens(sql); ParserRuleContext tree = toAst(tokenStream, DorisParser::singleStatement); LogicalPlanBuilderForSyncMv logicalPlanBuilderForSyncMv = new LogicalPlanBuilderForSyncMv( - getHintMap(sql, tokenStream, DorisParser::selectHint)); + getHintMap(tokenStream, DorisParser::selectHint)); logicalPlanBuilderForSyncMv.visit(tree); return logicalPlanBuilderForSyncMv.getQuerySql(); } /** get hint map */ - public static Map getHintMap(String sql, CommonTokenStream hintTokenStream, + public static Map getHintMap(CommonTokenStream hintTokenStream, Function parseFunction) { // parse hint first round Map selectHintMap = Maps.newHashMap(); @@ -392,8 +395,10 @@ public static Map getHintMap(String sql, CommonToken Iterator tokenIterator = hintTokenStream.getTokens().iterator(); Token hintToken = tokenIterator.hasNext() ? tokenIterator.next() : null; while (hintToken != null && hintToken.getType() != DorisLexer.EOF) { - if (hintToken.getChannel() == 2 && sql.charAt(hintToken.getStartIndex() + 2) == '+') { - String hintSql = sql.substring(hintToken.getStartIndex() + 3, hintToken.getStopIndex() + 1); + // Token positions count a supplementary character such as an emoji as one character, so read + // the hint from the token instead of cutting the SQL string. + if (hintToken.getChannel() == 2 && hintToken.getText().startsWith("/*+")) { + String hintSql = hintToken.getText().substring(3); DorisLexer newHintLexer = new DorisLexer(CaseInsensitiveStream.fromString(hintSql)); CommonTokenStream newHintTokenStream = new CommonTokenStream(newHintLexer); DorisParser hintParser = new DorisParser(newHintTokenStream); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/BaseViewInfo.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/BaseViewInfo.java index 54b1969db56c5b..892c67a4e4866c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/BaseViewInfo.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/BaseViewInfo.java @@ -81,9 +81,9 @@ import com.google.common.collect.Lists; import com.google.common.collect.Sets; import org.antlr.v4.runtime.ParserRuleContext; +import org.antlr.v4.runtime.misc.Interval; import org.antlr.v4.runtime.tree.ParseTree; import org.antlr.v4.runtime.tree.RuleNode; -import org.apache.commons.lang3.StringUtils; import java.util.List; import java.util.Map; @@ -128,18 +128,25 @@ protected void analyzeAndFillRewriteSqlMap(String sql, ConnectContext ctx) throw * Add the full path to the field * * @param indexStringSqlMap key is the start and end position of the sql substring that needs to be replaced, - * and value is the new string used for replacement. + * and value is the new string used for replacement. The positions are the token positions the + * parser reports for querySql. * @param querySql origin query sql * @return sql rewritten sql */ public static String rewriteSql(TreeMap, String> indexStringSqlMap, String querySql) { StringBuilder builder = new StringBuilder(); + // The parser counts a supplementary character such as an emoji as one character, while querySql + // counts it as two. beg and begTokenPosition are the same place on both scales; the keys ascend, + // so each position is converted by scanning forward from the previous one. int beg = 0; + int begTokenPosition = 0; for (Map.Entry, String> entry : indexStringSqlMap.entrySet()) { Pair index = entry.getKey(); - builder.append(querySql, beg, index.first); + int start = querySql.offsetByCodePoints(beg, index.first - begTokenPosition); + builder.append(querySql, beg, start); builder.append(entry.getValue()); - beg = index.second + 1; + beg = querySql.offsetByCodePoints(start, index.second + 1 - index.first); + begTokenPosition = index.second + 1; } builder.append(querySql, beg, querySql.length()); return builder.toString(); @@ -167,10 +174,11 @@ public static String rewriteProjectsToUserDefineAlias(String resSql, List namedExpressionContexts = finder.getNamedExpressionContexts(); StringBuilder replaceWithColsBuilder = new StringBuilder(); for (int i = 0; i < namedExpressionContexts.size(); ++i) { - NamedExpressionContext namedExpressionContext = namedExpressionContexts.get(i); - int start = namedExpressionContext.expression().start.getStartIndex(); - int stop = namedExpressionContext.expression().stop.getStopIndex(); - replaceWithColsBuilder.append(resSql, start, stop + 1); + ParserRuleContext expression = namedExpressionContexts.get(i).expression(); + // Token positions count a supplementary character such as an emoji as one character, so read + // the expression from the parser's input instead of cutting resSql. + replaceWithColsBuilder.append(expression.start.getInputStream().getText( + Interval.of(expression.start.getStartIndex(), expression.stop.getStopIndex()))); replaceWithColsBuilder.append(" AS `"); String escapeBacktick = finalColNames.get(i).replace("`", "``"); replaceWithColsBuilder.append(escapeBacktick); @@ -179,9 +187,9 @@ public static String rewriteProjectsToUserDefineAlias(String resSql, List, String> selectColumns = new TreeMap<>(new Pair.PairComparator<>()); + selectColumns.put(finder.getIndex(), replaceWithColsBuilder.toString()); + return rewriteSql(selectColumns, resSql); } protected void createFinalCols(List outputs) throws org.apache.doris.common.AnalysisException { diff --git a/fe/fe-core/src/test/java/org/apache/doris/catalog/CreateViewTest.java b/fe/fe-core/src/test/java/org/apache/doris/catalog/CreateViewTest.java index bec3e35a6266cb..b3ea280fb178fe 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/catalog/CreateViewTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/CreateViewTest.java @@ -268,6 +268,26 @@ public void testCreateViewWithDefinedColumnsRewritesAliases() throws Exception { view.getInlineViewDef()); } + @Test + public void testCreateViewRewritesAfterSupplementaryCharacters() throws Exception { + // The parser counts a supplementary character such as an emoji as one character, while + // the Java string of the view definition counts it as two. + ExceptionChecker.expectThrowsNoException(() -> createView( + "create view test.supplementary_char_view as select 'πŸ˜€πŸ˜€' as e, k1 from test.tbl1;")); + ExceptionChecker.expectThrowsNoException(() -> createView( + "create view test.supplementary_char_alias_view(c1, c2) as select 'πŸ˜€πŸ˜€', k1 from test.tbl1;")); + + Database db = Env.getCurrentInternalCatalog().getDbOrDdlException("test"); + View view = (View) db.getTableOrDdlException("supplementary_char_view"); + Assertions.assertEquals( + "select 'πŸ˜€πŸ˜€' as `e`, `internal`.`test`.`tbl1`.`k1` from `internal`.`test`.`tbl1`", + view.getInlineViewDef()); + View aliasView = (View) db.getTableOrDdlException("supplementary_char_alias_view"); + Assertions.assertEquals( + "select 'πŸ˜€πŸ˜€' AS `c1`, `internal`.`test`.`tbl1`.`k1` AS `c2` from `internal`.`test`.`tbl1`", + aliasView.getInlineViewDef()); + } + @Test public void testViewPreservesVarbinary() throws Exception { createView("create view test.vb_view as select X'AB' as vb;"); diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/EncryptSQLTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/EncryptSQLTest.java index 5ccb3ca9400524..dab80292149f26 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/EncryptSQLTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/EncryptSQLTest.java @@ -505,6 +505,98 @@ public void testAlterUserPasswordMasking() throws Exception { } } + @Test + public void testEncryptionAfterSupplementaryCharacters() throws Exception { + ctx.setDatabase("test"); + try (MockedConstruction ignored = Mockito.mockConstruction(StmtExecutor.class, + Mockito.withSettings().defaultAnswer(Mockito.CALLS_REAL_METHODS), + (mock, context) -> { + Mockito.doReturn(false).when(mock).isForwardToMaster(); + Profile profile = new Profile(false, 0, 0); + Deencapsulation.setField(mock, "profile", profile); + Mockito.doReturn(profile).when(mock).getProfile(); + Mockito.doReturn(ctx).when(mock).getContext(); + Mockito.doNothing().when(mock).execute(); + Deencapsulation.setField(mock, "context", ctx); + if (context.arguments().size() >= 2 + && context.arguments().get(1) instanceof StatementBase) { + Deencapsulation.setField(mock, "parsedStmt", + context.arguments().get(1)); + } + if (ctx.getStatementContext() == null) { + ctx.setStatementContext(new StatementContext()); + } + })) { + ctx.setEnv(env); + ctx.setCurrentUserIdentity(UserIdentity.ROOT); + // The parser counts a supplementary character such as an emoji as one character, while + // the Java string of the SQL counts it as two. + String sql = "CREATE CATALOG ctl " + + "PROPERTIES(" + + " \"type\" = \"iceberg\"," + + " \"iceberg.catalog.type\" = \"hadoop\"," + + " \"warehouse\" = \"s3://bucket/dir/key\"," + + " \"s3.access_key\" = \"abc\"," + + " \"s3.secret_key\" = \"πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€secret\"" + + ")"; + String res = "CREATE CATALOG ctl " + + "PROPERTIES(" + + " \"type\" = \"iceberg\"," + + " \"iceberg.catalog.type\" = \"hadoop\"," + + " \"warehouse\" = \"s3://bucket/dir/key\"," + + " \"s3.access_key\" = \"*XXX\"," + + " \"s3.secret_key\" = \"*XXX\"" + + ")"; + parseAndCheck(sql, res); + + sql = "CREATE CATALOG ctl COMMENT \"πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€\" " + + "PROPERTIES(" + + " \"type\" = \"iceberg\"," + + " \"iceberg.catalog.type\" = \"hadoop\"," + + " \"warehouse\" = \"s3://bucket/πŸ˜€/key\"," + + " \"s3.access_key\" = \"abc\"," + + " \"s3.secret_key\" = \"abc\"" + + ")"; + res = "CREATE CATALOG ctl COMMENT \"πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€\" " + + "PROPERTIES(" + + " \"type\" = \"iceberg\"," + + " \"iceberg.catalog.type\" = \"hadoop\"," + + " \"warehouse\" = \"s3://bucket/πŸ˜€/key\"," + + " \"s3.access_key\" = \"*XXX\"," + + " \"s3.secret_key\" = \"*XXX\"" + + ")"; + parseAndCheck(sql, res); + + sql = "CREATE EXTERNAL TABLE broker_tbl(" + + " k1 tinyint) " + + "ENGINE=broker " + + "PROPERTIES(" + + " \"broker_name\" = \"hdfs\"," + + " \"path\" = \"hdfs://abc/πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€/a.txt\"" + + ") " + + "BROKER PROPERTIES(" + + " \"username\" = \"root\"," + + " \"password\" = \"123\"" + + ")"; + res = "CREATE EXTERNAL TABLE broker_tbl(" + + " k1 tinyint) " + + "ENGINE=broker " + + "PROPERTIES(" + + " \"broker_name\" = \"hdfs\"," + + " \"path\" = \"hdfs://abc/πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€/a.txt\"" + + ") " + + "BROKER PROPERTIES(" + + " \"username\" = \"root\"," + + " \"password\" = \"*XXX\"" + + ")"; + parseAndCheck(sql, res); + + sql = "SET PASSWORD FOR 'πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€' = PASSWORD('123456')"; + res = "SET PASSWORD FOR 'πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€πŸ˜€' = PASSWORD('*XXX')"; + parseAndCheck(sql, res); + } + } + private void parseAndCheck(String sql, String expected) throws Exception { processor.executeQuery(sql); AuditEvent event = auditEvents.get(auditEvents.size() - 1); diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/SupplementaryCharacterOffsetTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/SupplementaryCharacterOffsetTest.java new file mode 100644 index 00000000000000..b55a7e8a8cf119 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/SupplementaryCharacterOffsetTest.java @@ -0,0 +1,64 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.nereids.parser; + +import org.apache.doris.nereids.properties.SelectHintOrdered; +import org.apache.doris.nereids.trees.plans.Plan; +import org.apache.doris.nereids.trees.plans.logical.LogicalPlan; +import org.apache.doris.nereids.trees.plans.logical.LogicalSelectHint; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.List; + +/** + * The parser counts a supplementary character such as an emoji as one character, while the Java + * string of the SQL counts it as two. These cases cut the SQL text after such characters. + */ +class SupplementaryCharacterOffsetTest extends ParserTestBase { + private final NereidsParser parser = new NereidsParser(); + + @Test + void recognizesHintAfterSupplementaryCharacter() { + LogicalPlan plan = parser.parseSingle("/* πŸ˜€ */ SELECT /*+ ORDERED */ a FROM t"); + List> hintPlans = plan.collectToList(LogicalSelectHint.class::isInstance); + + Assertions.assertEquals(1, hintPlans.size()); + Assertions.assertTrue(hintPlans.get(0).getHints().stream() + .anyMatch(SelectHintOrdered.class::isInstance)); + } + + @Test + void extractsExplainPlanBodyAfterSupplementaryCharacters() { + Assertions.assertEquals("SELECT 100", + NereidsParser.tryParseExplainPlan("EXPLAIN /* abc */ PLAN SELECT 100").orElseThrow().second); + Assertions.assertEquals("SELECT 100", + NereidsParser.tryParseExplainPlan("EXPLAIN /* πŸ˜€πŸ˜€πŸ˜€ */ PLAN SELECT 100").orElseThrow().second); + } + + @Test + void rewritesSyncMvAggregateAfterSupplementaryCharacter() { + Assertions.assertEquals("SELECT /* a */ k1, avg_union(avg_state(k2)) FROM t GROUP BY k1", + parser.parseForSyncMv("CREATE MATERIALIZED VIEW mv AS " + + "SELECT /* a */ k1, avg(k2) FROM t GROUP BY k1").orElseThrow()); + Assertions.assertEquals("SELECT /* πŸ˜€ */ k1, avg_union(avg_state(k2)) FROM t GROUP BY k1", + parser.parseForSyncMv("CREATE MATERIALIZED VIEW mv AS " + + "SELECT /* πŸ˜€ */ k1, avg(k2) FROM t GROUP BY k1").orElseThrow()); + } +} diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/CreateMTMVCommandTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/CreateMTMVCommandTest.java index f0fe9fc3da2e82..df810ff920c135 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/CreateMTMVCommandTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/CreateMTMVCommandTest.java @@ -626,6 +626,25 @@ public void testCreateMTMVRewriteQuerySqlWithDefinedColumnsForScanPlan() throws Assertions.assertTrue(createMTMVInfo.getQuerySql().contains("AS `mv_score`")); } + @Test + public void testCreateMTMVRewritesQuerySqlAfterSupplementaryCharacters() throws Exception { + createTable("create table test.mtmv_supplementary_char_base (id int, score int)\n" + + "duplicate key(id)\n" + + "distributed by hash(id) buckets 1\n" + + "properties('replication_num' = '1');"); + + // The parser counts a supplementary character such as an emoji as one character, while + // the Java string of the query counts it as two. + CreateMTMVInfo createMTMVInfo = getPartitionTableInfo("CREATE MATERIALIZED VIEW mtmv_supplementary_char\n" + + " BUILD DEFERRED REFRESH COMPLETE ON MANUAL\n" + + " PROPERTIES ('replication_num' = '1')\n" + + " AS\n" + + " SELECT 'πŸ˜€πŸ˜€' AS e, id FROM mtmv_supplementary_char_base;"); + + Assertions.assertEquals("SELECT 'πŸ˜€πŸ˜€' AS `e`, `internal`.`test`.`mtmv_supplementary_char_base`.`id` " + + "FROM `internal`.`test`.`mtmv_supplementary_char_base`", createMTMVInfo.getQuerySql()); + } + @Test public void testCreateMTMVRewriteQuerySqlWithDefinedColumnsForProjectScanPlan() throws Exception { createTable("create table test.mtmv_project_scan_base (id int, score int)\n" diff --git a/regression-test/data/ddl_p0/create_view_nereids/test_create_view_supplementary_chars.out b/regression-test/data/ddl_p0/create_view_nereids/test_create_view_supplementary_chars.out new file mode 100644 index 00000000000000..05a50df32b21c3 --- /dev/null +++ b/regression-test/data/ddl_p0/create_view_nereids/test_create_view_supplementary_chars.out @@ -0,0 +1,16 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !show_view -- +test_create_view_supplementary_chars_view CREATE VIEW `test_create_view_supplementary_chars_view`\n(`e`,`k1`,`v1`)\n AS select 'πŸ˜€πŸ˜€' as `e`, `internal`.`regression_test_ddl_p0_create_view_nereids`.`test_create_view_supplementary_chars_base`.`k1`, `internal`.`regression_test_ddl_p0_create_view_nereids`.`test_create_view_supplementary_chars_base`.`v1` from `internal`.`regression_test_ddl_p0_create_view_nereids`.`test_create_view_supplementary_chars_base` where `internal`.`regression_test_ddl_p0_create_view_nereids`.`test_create_view_supplementary_chars_base`.`v1` != 'πŸ˜€πŸ˜€πŸ˜€'; utf8mb4 utf8mb4_0900_bin + +-- !select_view -- +πŸ˜€πŸ˜€ 1 aπŸ˜€ +πŸ˜€πŸ˜€ 2 b + +-- !show_alias -- +test_create_view_supplementary_chars_alias CREATE VIEW `test_create_view_supplementary_chars_alias`\n(`c1`,`c2`)\n AS select 'πŸ˜€πŸ˜€' AS `c1`, `internal`.`regression_test_ddl_p0_create_view_nereids`.`test_create_view_supplementary_chars_base`.`k1` AS `c2` from `internal`.`regression_test_ddl_p0_create_view_nereids`.`test_create_view_supplementary_chars_base`; utf8mb4 utf8mb4_0900_bin + +-- !select_alias -- +πŸ˜€πŸ˜€ 1 +πŸ˜€πŸ˜€ 2 +πŸ˜€πŸ˜€ 3 + diff --git a/regression-test/suites/ddl_p0/create_view_nereids/test_create_view_supplementary_chars.groovy b/regression-test/suites/ddl_p0/create_view_nereids/test_create_view_supplementary_chars.groovy new file mode 100644 index 00000000000000..ea0e7636baa034 --- /dev/null +++ b/regression-test/suites/ddl_p0/create_view_nereids/test_create_view_supplementary_chars.groovy @@ -0,0 +1,45 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// The parser counts a supplementary character such as an emoji as one character, while the Java string +// of the view query counts it as two; the view definition is rewritten on that string. +suite("test_create_view_supplementary_chars") { + sql "drop table if exists test_create_view_supplementary_chars_base" + sql """ + create table test_create_view_supplementary_chars_base (k1 int, v1 varchar(32)) + duplicate key(k1) + distributed by hash(k1) buckets 1 + properties('replication_num' = '1') + """ + sql "insert into test_create_view_supplementary_chars_base values (1, 'aπŸ˜€'), (2, 'b'), (3, 'πŸ˜€πŸ˜€πŸ˜€')" + + sql "drop view if exists test_create_view_supplementary_chars_view" + sql """ + create view test_create_view_supplementary_chars_view as + select 'πŸ˜€πŸ˜€' as e, k1, v1 from test_create_view_supplementary_chars_base where v1 != 'πŸ˜€πŸ˜€πŸ˜€' + """ + qt_show_view "show create view test_create_view_supplementary_chars_view" + order_qt_select_view "select * from test_create_view_supplementary_chars_view" + + sql "drop view if exists test_create_view_supplementary_chars_alias" + sql """ + create view test_create_view_supplementary_chars_alias (c1, c2) as + select 'πŸ˜€πŸ˜€', k1 from test_create_view_supplementary_chars_base + """ + qt_show_alias "show create view test_create_view_supplementary_chars_alias" + order_qt_select_alias "select * from test_create_view_supplementary_chars_alias" +}