Skip to content

Commit

Permalink
[fix](nereids) Fix not check column name when create or alter view (a…
Browse files Browse the repository at this point in the history
…pache#42206)

This is brought by apache#32743

set enable_unicode_name_support = true;
If run create view sql should fail beausel_shipdate column name contains
invalid char '(' and ')', but now success
this pr fix this and throw exception
`ERROR 1105 (HY000): errCode = 2, detailMessage = Incorrect column name
'(日期)'. Column regex is
'^[_a-zA-Z@0-9\s/][.a-zA-Z0-9_+-/?@#$%^&*"\s,:]{0,255}$'`


CREATE VIEW view1
AS
SELECT "零售公司", l_shipdate as '(日期)', l_receiptdate as k2
FROM lineitem;

and if run create view sql as following, should success:

CREATE VIEW view2
AS
SELECT "零售公司", l_shipdate as '日期', l_receiptdate as k2
FROM lineitem;

and the schema of view2 should be

mysql> desc view2;
+-------------+-------------+------+-------+---------+-------+
| Field       | Type        | Null | Key   | Default | Extra |
+-------------+-------------+------+-------+---------+-------+
| __literal_0 | varchar(16) | No   | false | NULL    |       |
| 日期        | date        | No   | false | NULL    |       |
| k2          | date        | No   | false | NULL    |       |
+-------------+-------------+------+-------+---------+-------+
3 rows in set (0.01 sec)
  • Loading branch information
seawinde committed Oct 23, 2024
1 parent f2bc4a3 commit 53813f3
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.apache.doris.analysis.AlterViewStmt;
import org.apache.doris.analysis.ColWithComment;
import org.apache.doris.analysis.TableName;
import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.DatabaseIf;
import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.TableIf;
Expand All @@ -31,19 +30,13 @@
import org.apache.doris.common.UserException;
import org.apache.doris.common.util.Util;
import org.apache.doris.mysql.privilege.PrivPredicate;
import org.apache.doris.nereids.NereidsPlanner;
import org.apache.doris.nereids.analyzer.UnboundResultSink;
import org.apache.doris.nereids.properties.PhysicalProperties;
import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.plans.commands.ExplainCommand.ExplainLevel;
import org.apache.doris.nereids.util.PlanUtils;
import org.apache.doris.qe.ConnectContext;

import com.google.common.collect.Lists;
import com.google.common.collect.Sets;

import java.util.List;
import java.util.Set;

/** AlterViewInfo */
public class AlterViewInfo extends BaseViewInfo {
Expand Down Expand Up @@ -83,18 +76,6 @@ public void init(ConnectContext ctx) throws UserException {
createFinalCols(outputs);
}

/**validate*/
public void validate(ConnectContext ctx) throws UserException {
NereidsPlanner planner = new NereidsPlanner(ctx.getStatementContext());
planner.planWithLock(new UnboundResultSink<>(logicalQuery), PhysicalProperties.ANY, ExplainLevel.NONE);
Set<String> colSets = Sets.newTreeSet(String.CASE_INSENSITIVE_ORDER);
for (Column col : finalCols) {
if (!colSets.add(col.getName())) {
ErrorReport.reportAnalysisException(ErrorCode.ERR_DUP_FIELDNAME, col.getName());
}
}
}

/**translateToLegacyStmt*/
public AlterViewStmt translateToLegacyStmt(ConnectContext ctx) {
List<ColWithComment> cols = Lists.newArrayList();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,15 @@
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.ErrorCode;
import org.apache.doris.common.ErrorReport;
import org.apache.doris.common.FeNameFormat;
import org.apache.doris.common.Pair;
import org.apache.doris.common.UserException;
import org.apache.doris.nereids.CascadesContext;
import org.apache.doris.nereids.DorisParser;
import org.apache.doris.nereids.DorisParser.NamedExpressionContext;
import org.apache.doris.nereids.DorisParser.NamedExpressionSeqContext;
import org.apache.doris.nereids.DorisParserBaseVisitor;
import org.apache.doris.nereids.NereidsPlanner;
import org.apache.doris.nereids.StatementContext;
import org.apache.doris.nereids.analyzer.UnboundResultSink;
import org.apache.doris.nereids.jobs.executor.AbstractBatchJobExecutor;
Expand All @@ -44,6 +47,7 @@
import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.expressions.visitor.DefaultExpressionVisitor;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.commands.ExplainCommand.ExplainLevel;
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
import org.apache.doris.nereids.trees.plans.logical.LogicalFileSink;
import org.apache.doris.nereids.trees.plans.logical.LogicalFilter;
Expand All @@ -62,13 +66,15 @@
import org.apache.doris.qe.ConnectContext;

import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import org.antlr.v4.runtime.ParserRuleContext;
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;
import java.util.Set;
import java.util.TreeMap;

/** BaseViewInfo */
Expand All @@ -94,9 +100,6 @@ protected void analyzeAndFillRewriteSqlMap(String sql, ConnectContext ctx) throw
if (logicalQuery instanceof LogicalFileSink) {
throw new AnalysisException("Not support OUTFILE clause in CREATE VIEW statement");
}
if (parsedViewPlan instanceof UnboundResultSink) {
parsedViewPlan = (LogicalPlan) ((UnboundResultSink<?>) parsedViewPlan).child();
}
CascadesContext viewContextForStar = CascadesContext.initContext(
stmtCtx, parsedViewPlan, PhysicalProperties.ANY);
AnalyzerForCreateView analyzerForStar = new AnalyzerForCreateView(viewContextForStar);
Expand Down Expand Up @@ -175,6 +178,23 @@ protected void createFinalCols(List<Slot> outputs) throws org.apache.doris.commo
}
}

/**validate*/
public void validate(ConnectContext ctx) throws UserException {
NereidsPlanner planner = new NereidsPlanner(ctx.getStatementContext());
planner.planWithLock(new UnboundResultSink<>(logicalQuery), PhysicalProperties.ANY, ExplainLevel.NONE);
Set<String> colSets = Sets.newTreeSet(String.CASE_INSENSITIVE_ORDER);
for (Column col : finalCols) {
if (!colSets.add(col.getName())) {
ErrorReport.reportAnalysisException(ErrorCode.ERR_DUP_FIELDNAME, col.getName());
}
try {
FeNameFormat.checkColumnName(col.getName());
} catch (org.apache.doris.common.AnalysisException e) {
throw new org.apache.doris.nereids.exceptions.AnalysisException(e.getMessage(), e);
}
}
}

/** traverse ast to find the outermost project list location information in sql*/
protected static class IndexFinder extends DorisParserBaseVisitor<Void> {
private boolean found = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,27 +20,20 @@
import org.apache.doris.analysis.ColWithComment;
import org.apache.doris.analysis.CreateViewStmt;
import org.apache.doris.analysis.TableName;
import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.Env;
import org.apache.doris.common.ErrorCode;
import org.apache.doris.common.ErrorReport;
import org.apache.doris.common.FeNameFormat;
import org.apache.doris.common.UserException;
import org.apache.doris.common.util.Util;
import org.apache.doris.mysql.privilege.PrivPredicate;
import org.apache.doris.nereids.NereidsPlanner;
import org.apache.doris.nereids.analyzer.UnboundResultSink;
import org.apache.doris.nereids.properties.PhysicalProperties;
import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.plans.commands.ExplainCommand.ExplainLevel;
import org.apache.doris.nereids.util.PlanUtils;
import org.apache.doris.qe.ConnectContext;

import com.google.common.collect.Lists;
import com.google.common.collect.Sets;

import java.util.List;
import java.util.Set;

/**
* CreateViewInfo
Expand Down Expand Up @@ -78,18 +71,6 @@ public void init(ConnectContext ctx) throws UserException {
createFinalCols(outputs);
}

/**validate*/
public void validate(ConnectContext ctx) throws UserException {
NereidsPlanner planner = new NereidsPlanner(ctx.getStatementContext());
planner.planWithLock(new UnboundResultSink<>(logicalQuery), PhysicalProperties.ANY, ExplainLevel.NONE);
Set<String> colSets = Sets.newTreeSet(String.CASE_INSENSITIVE_ORDER);
for (Column col : finalCols) {
if (!colSets.add(col.getName())) {
ErrorReport.reportAnalysisException(ErrorCode.ERR_DUP_FIELDNAME, col.getName());
}
}
}

/**translateToLegacyStmt*/
public CreateViewStmt translateToLegacyStmt(ConnectContext ctx) {
List<ColWithComment> cols = Lists.newArrayList();
Expand Down
24 changes: 23 additions & 1 deletion regression-test/data/ddl_p0/test_create_view_nereids.out
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ test_view_from_view CREATE VIEW `test_view_from_view` AS select `internal`.`regr
7 1

-- !test_backquote_in_view_define_sql --
test_backquote_in_view_define CREATE VIEW `test_backquote_in_view_define` AS select `internal`.`regression_test_ddl_p0`.`mal_test_view`.`a` AS `ab``c`, `internal`.`regression_test_ddl_p0`.`mal_test_view`.`b` AS `c2` from `internal`.`regression_test_ddl_p0`.`mal_test_view`; utf8mb4 utf8mb4_0900_bin
test_backquote_in_view_define CREATE VIEW `test_backquote_in_view_define` AS select `internal`.`regression_test_ddl_p0`.`mal_test_view`.`a` AS `abc`, `internal`.`regression_test_ddl_p0`.`mal_test_view`.`b` AS `c2` from `internal`.`regression_test_ddl_p0`.`mal_test_view`; utf8mb4 utf8mb4_0900_bin

-- !test_backquote_in_table_alias --
\N 6
Expand All @@ -226,6 +226,28 @@ test_backquote_in_view_define CREATE VIEW `test_backquote_in_view_define` AS sel
-- !test_backquote_in_table_alias_sql --
test_backquote_in_table_alias CREATE VIEW `test_backquote_in_table_alias` AS select `internal`.`regression_test_ddl_p0`.`ab``c`.`a` AS `c1`, `internal`.`regression_test_ddl_p0`.`ab``c`.`b` AS `c2` from (select `internal`.`regression_test_ddl_p0`.`mal_test_view`.`a`,`internal`.`regression_test_ddl_p0`.`mal_test_view`.`b` from `internal`.`regression_test_ddl_p0`.`mal_test_view`) `ab``c`; utf8mb4 utf8mb4_0900_bin

-- !test_invalid_column_name_in_table --
\N 6
1 2
1 3
1 4
1 4
1 7
2 8
3 5
3 6
3 9
4 2
5 \N
5 6
5 6
5 6
5 8
7 1

-- !test_invalid_column_name_in_table_define_sql --
test_invalid_column_name_in_table CREATE VIEW `test_invalid_column_name_in_table` AS select `internal`.`regression_test_ddl_p0`.`mal_test_view`.`a` ,`internal`.`regression_test_ddl_p0`.`mal_test_view`.`b` from `internal`.`regression_test_ddl_p0`.`mal_test_view`; utf8mb4 utf8mb4_0900_bin

-- !test_generate --
1 10 A 30
1 10 A 60
Expand Down
31 changes: 29 additions & 2 deletions regression-test/suites/ddl_p0/test_create_view_nereids.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -267,17 +267,44 @@ suite("test_create_view_nereids") {
qt_test_create_view_from_view_sql "show create view test_view_from_view"

// test backquote in name
try {
sql "create view test_backquote_in_view_define(`ab``c`, c2) as select a,b from mal_test_view;"
} catch (Exception e) {
assertTrue(e.getMessage().contains("Incorrect column name 'ab`c'"))
}

sql "drop view if exists test_backquote_in_view_define;"
sql "create view test_backquote_in_view_define(`ab``c`, c2) as select a,b from mal_test_view;"
qt_test_backquote_in_view_define "select * from test_backquote_in_view_define order by `ab``c`, c2;"
sql "create view test_backquote_in_view_define(`abc`, c2) as select a,b from mal_test_view;"
qt_test_backquote_in_view_define "select * from test_backquote_in_view_define order by abc, c2;"
qt_test_backquote_in_view_define_sql "show create view test_backquote_in_view_define;"

sql "drop view if exists test_backquote_in_table_alias;"
sql "create view test_backquote_in_table_alias(c1, c2) as select * from (select a,b from mal_test_view) `ab``c`;"
qt_test_backquote_in_table_alias "select * from test_backquote_in_table_alias order by c1, c2;"
qt_test_backquote_in_table_alias_sql "show create view test_backquote_in_table_alias;"

// test invalid column name
sql """set enable_unicode_name_support = true;"""
sql "drop view if exists test_invalid_column_name_in_table;"
try {
// create view should fail if contains invalid column name
sql "create view test_invalid_column_name_in_table as select a as '(第一列)',b from mal_test_view;"
} catch (Exception e) {
assertTrue(e.getMessage().contains("Incorrect column name '(第一列)'"))
}

sql "create view test_invalid_column_name_in_table as select a ,b from mal_test_view;"
order_qt_test_invalid_column_name_in_table "select * from test_invalid_column_name_in_table"
order_qt_test_invalid_column_name_in_table_define_sql "show create view test_invalid_column_name_in_table;"

try {
// alter view should fail if contains invalid column name
sql "alter view test_invalid_column_name_in_table as select a as '(第一列)',b from mal_test_view;"
} catch (Exception e) {
assertTrue(e.getMessage().contains("Incorrect column name '(第一列)'"))
}
sql """set enable_unicode_name_support = false;"""

sql "drop table if exists create_view_table1"
sql """CREATE TABLE create_view_table1 (
id INT,
Expand Down

0 comments on commit 53813f3

Please sign in to comment.