morrySnow commented on code in PR #35284:
URL: https://github.com/apache/doris/pull/35284#discussion_r1628900860
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertUtils.java:
##########
@@ -344,6 +357,31 @@ public static Plan normalizePlan(Plan plan, TableIf table)
{
StatementScopeIdGenerator.newRelationId(),
constantExprs.build()));
}
List<LogicalPlan> oneRowRelations = oneRowRelationBuilder.build();
+
+ // if (plan instanceof UnboundTableSink) {
+ // List<String> newColNames = new ArrayList<>();
+ // if (unboundLogicalSink.getColNames().isEmpty()) {
+ // for (Column column : table.getBaseSchema(false)) {
+ // if (column.getGeneratedColumnInfo() != null) {
+ // continue;
+ // }
+ // newColNames.add(column.getName());
+ // }
+ // } else {
+ // for (String name : unboundLogicalSink.getColNames()) {
+ // Column column = table.getColumn(name);
+ // if (column.getGeneratedColumnInfo() != null) {
+ // continue;
+ // }
+ // newColNames.add(name);
+ // }
+ // }
+ // if (!unboundLogicalSink.getColNames().isEmpty() &&
newColNames.isEmpty()) {
+ //
+ // }
+ // plan = ((UnboundTableSink) plan).withColNames(newColNames);
+ // }
Review Comment:
remove useless code
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertUtils.java:
##########
@@ -383,6 +421,10 @@ public static TableIf getTargetTable(Plan plan,
ConnectContext ctx) {
private static NamedExpression generateDefaultExpression(Column column) {
try {
+ GeneratedColumnInfo generatedColumnInfo =
column.getGeneratedColumnInfo();
+ if (generatedColumnInfo != null) {
+ return new Alias(new
NullLiteral(DataType.fromCatalogType(column.getType())), column.getName());
Review Comment:
add comment to explain why use nullliteral as a placeholder
##########
regression-test/suites/ddl_p0/test_create_table_generated_column/fault_tolerance_nereids.groovy:
##########
@@ -0,0 +1,159 @@
+// 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.
+
+suite("test_generated_column_fault_tolerance_nereids") {
+ sql "SET enable_nereids_planner=true;"
+ sql "SET enable_fallback_to_original_planner=false;"
+ test {
+ sql """
+ create table gencol_type_check(a int,b int, c array<int> generated
always as (abs(a+b,3)) not null)
+ DISTRIBUTED BY HASH(a)
+ PROPERTIES("replication_num" = "1");
+ """
+ exception "Can not found function 'abs' which has 2 arity."
+ }
+
+ // gencol_has_sum
+ test {
+ sql """
+ create table gencol_has_sum(a int,b int, c int generated always as
(sum(a)) not null)
+ DISTRIBUTED BY HASH(a)
+ PROPERTIES("replication_num" = "1");
+ """
+ exception "Expression of generated column 'c' contains a disallowed
function"
+ }
+
+ // gencol_has_column_not_define
+ test {
+ sql """
+ create table gencol_has_sum(a int,b int, c int generated always as
(abs(d)) not null)
+ DISTRIBUTED BY HASH(a)
+ PROPERTIES("replication_num" = "1");
+ """
+ exception "Unknown column 'd' in 'generated column function'"
+ }
+
+ // gencol_refer_gencol_after
+ test {
+ sql """
+ create table gencol_refer_gencol(a int,c double generated always as
(abs(a+d)) not null,b int, d int generated always as(c+1))
+ DISTRIBUTED BY HASH(a)
+ PROPERTIES("replication_num" = "1");
+ """
+ exception "Generated column can refer only to generated columns
defined prior to it."
+ }
+
+ sql "set @myvar=2"
+ // gencol_has_var
+ test {
+ sql """
+ create table test_gen_col_not_null100(a varchar(10),c double generated
always as (abs(a+b+@myvar)) not null,b int)
+ DISTRIBUTED BY HASH(a)
+ PROPERTIES("replication_num" = "1");
+ """
+ exception "Generated column expression cannot contain variable."
+ }
+
+ test {
+ sql """
+ create table test_gen_col_auto_increment(a bigint not null
auto_increment, b int, c int as (a*b))
+ distributed by hash(a) properties("replication_num" = "1");
+ """
+ exception "Generated column 'c' cannot refer to auto-increment column."
+ }
+
+ test{
+ sql """
+ create table test_gen_col_subquery(a int,b int, c int generated always
as (a+(select 1)) not null)
+ DISTRIBUTED BY HASH(a)
+ PROPERTIES("replication_num" = "1");
+ """
+ exception "Generated column does not support subquery."
+ }
+
+ test {
+ sql """
+ create table test_gen_col_array_func_lambda(pk int,a array<int>,b
array<int>, c array<int> generated always as (array_count(x->(x%2=0),b)) not
null)
+ DISTRIBUTED BY HASH(pk)
+ PROPERTIES("replication_num" = "1");
+ """
+ exception "Generated column does not support lambda."
+ }
+
+ test {
+ sql """
+ create table test_gen_col_array_func(pk int,a array<int>,b
array<int>, c double generated always as (a+b) not null)
+ DISTRIBUTED BY HASH(pk)
+ PROPERTIES("replication_num" = "1");
+ """
+ exception "Cannot cast from ARRAY<INT> to numeric type"
Review Comment:
The error message should mention that it is related to a generated column
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java:
##########
@@ -376,6 +378,21 @@ private boolean processDropColumn(DropColumnClause
alterClause, OlapTable olapTa
}
}
+ // generated column check
+ Map<String, Column> nameToColumn = new HashMap<>();
Review Comment:
check alter table column datatype in next pr?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindSink.java:
##########
@@ -381,6 +387,20 @@ private static Map<String, NamedExpression>
getColumnToOutput(
}
}
}
+ for (Column column : generatedColumns) {
Review Comment:
add this explaination to a comment to avoid confusing when reading code
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java:
##########
@@ -931,6 +947,9 @@ public String toSql(boolean isUniqueTable, boolean
isCompatible) {
} else {
sb.append(typeStr);
}
+ if (generatedColumnInfo != null) {
+ return generatedColumnToSql(sb);
Review Comment:
why not just append generated column info here and reuse other code?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/analyzer/UnboundTableSink.java:
##########
@@ -173,6 +173,11 @@ public Plan
withGroupExprLogicalPropChildren(Optional<GroupExpression> groupExpr
isPartialUpdate, dmlCommandType, groupExpression,
logicalProperties, children.get(0));
}
+ public Plan withColNames(List<String> colNames) {
+ return new UnboundTableSink<>(nameParts, colNames, hints,
temporaryPartition, partitions, autoDetectPartition,
+ isPartialUpdate, dmlCommandType, groupExpression,
Optional.of(getLogicalProperties()), child());
+ }
+
Review Comment:
unused function remove it
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateTableInfo.java:
##########
@@ -798,5 +824,209 @@ public CreateTableStmt translateToLegacyStmt() {
public void setIsExternal(boolean isExternal) {
this.isExternal = isExternal;
}
+
+ private void genreatedColumnCommonCheck() {
+ boolean hasGeneratedCol = false;
+ for (ColumnDefinition column : columns) {
+ if (column.getGeneratedColumnDesc().isPresent()) {
+ hasGeneratedCol = true;
+ break;
+ }
+ }
+ if (hasGeneratedCol && !engineName.equalsIgnoreCase("olap")) {
+ throw new AnalysisException("Tables can only have generated
columns if the olap engine is used");
+ }
+ if (hasGeneratedCol && keysType == KeysType.AGG_KEYS) {
+ throw new AnalysisException("Generated Column cannot be used in
the aggregate table");
+ }
+ }
+
+ private void generatedColumnCheck(ConnectContext ctx) {
+ genreatedColumnCommonCheck();
+ LogicalEmptyRelation plan = new LogicalEmptyRelation(
+ ConnectContext.get().getStatementContext().getNextRelationId(),
+ new ArrayList<>());
+ CascadesContext cascadesContext =
CascadesContext.initContext(ctx.getStatementContext(), plan,
+ PhysicalProperties.ANY);
+ Map<String, Slot> columnToSlotReference = Maps.newHashMap();
+ Map<String, ColumnDefinition> nameToColumnDefinition =
Maps.newHashMap();
+ Map<Slot, SlotRefAndIdx> translateMap = Maps.newHashMap();
+ for (int i = 0; i < columns.size(); i++) {
+ ColumnDefinition column = columns.get(i);
+ Slot slot = new SlotReference(column.getName(), column.getType());
+ columnToSlotReference.put(column.getName(), slot);
+ nameToColumnDefinition.put(column.getName(), column);
+ SlotRef slotRef = new SlotRef(null, column.getName());
+ slotRef.setType(column.getType().toCatalogDataType());
+ translateMap.put(slot, new SlotRefAndIdx(slotRef, i,
column.getGeneratedColumnDesc().isPresent()));
+ }
+ ExpressionToExpr.initializeslotRefMap(translateMap);
+ PlanTranslatorContext planTranslatorContext = new
PlanTranslatorContext(cascadesContext);
+ List<Slot> slots = Lists.newArrayList(columnToSlotReference.values());
+ List<GeneratedColumnUtil.ExprAndname> exprAndnames =
Lists.newArrayList();
+ for (int i = 0; i < columns.size(); i++) {
+ ColumnDefinition column = columns.get(i);
+ Optional<GeneratedColumnDesc> info =
column.getGeneratedColumnDesc();
+ if (!info.isPresent()) {
+ continue;
+ }
+ Expression parsedExpression = info.get().getExpression();
+ checkparsedExpressionInGeneratedColumn(parsedExpression);
Review Comment:
```suggestion
checkParsedExpressionInGeneratedColumn(parsedExpression);
```
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java:
##########
@@ -686,6 +686,17 @@ public String toSqlImpl() {
sb.append(((FunctionCallExpr) expr).fnName);
sb.append(" ");
sb.append(children.get(1).toSql());
+ } else if (fnName.getFunction().equalsIgnoreCase("encryptkeyref")) {
Review Comment:
if rely on tosql of Expr, we should check all subclass of Expr to ensure it
is right later
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateTableInfo.java:
##########
@@ -798,5 +824,209 @@ public CreateTableStmt translateToLegacyStmt() {
public void setIsExternal(boolean isExternal) {
this.isExternal = isExternal;
}
+
+ private void genreatedColumnCommonCheck() {
+ boolean hasGeneratedCol = false;
+ for (ColumnDefinition column : columns) {
+ if (column.getGeneratedColumnDesc().isPresent()) {
+ hasGeneratedCol = true;
+ break;
+ }
+ }
+ if (hasGeneratedCol && !engineName.equalsIgnoreCase("olap")) {
+ throw new AnalysisException("Tables can only have generated
columns if the olap engine is used");
+ }
+ if (hasGeneratedCol && keysType == KeysType.AGG_KEYS) {
+ throw new AnalysisException("Generated Column cannot be used in
the aggregate table");
Review Comment:
why? could be used in key column?
##########
regression-test/suites/ddl_p0/test_create_table_generated_column/test_generated_column_nereids.groovy:
##########
Review Comment:
add case `insert into tbl select ...`. we should throw exception when try to
insert data into generated column
##########
regression-test/suites/ddl_p0/test_create_table_generated_column/fault_tolerance_nereids.groovy:
##########
@@ -0,0 +1,159 @@
+// 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.
+
+suite("test_generated_column_fault_tolerance_nereids") {
+ sql "SET enable_nereids_planner=true;"
+ sql "SET enable_fallback_to_original_planner=false;"
+ test {
+ sql """
+ create table gencol_type_check(a int,b int, c array<int> generated
always as (abs(a+b,3)) not null)
+ DISTRIBUTED BY HASH(a)
+ PROPERTIES("replication_num" = "1");
+ """
+ exception "Can not found function 'abs' which has 2 arity."
+ }
+
+ // gencol_has_sum
+ test {
+ sql """
+ create table gencol_has_sum(a int,b int, c int generated always as
(sum(a)) not null)
+ DISTRIBUTED BY HASH(a)
+ PROPERTIES("replication_num" = "1");
+ """
+ exception "Expression of generated column 'c' contains a disallowed
function"
Review Comment:
print the disallowed function sql?
##########
regression-test/suites/ddl_p0/test_create_table_generated_column/fault_tolerance_nereids.groovy:
##########
Review Comment:
should print column name in error message
##########
fe/fe-core/src/main/java/org/apache/doris/common/proc/IndexSchemaProcNode.java:
##########
@@ -65,6 +65,9 @@ public static ProcResult createResult(List<Column> schema,
Set<String> bfColumns
if (column.isAutoInc()) {
extras.add("AUTO_INCREMENT");
}
+ if (column.getGeneratedColumnInfo() != null) {
+ extras.add("GENERATED");
Review Comment:
mysql print: STORED GENERATED or VIRTUAL GENERATED
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
