morrySnow commented on code in PR #35284:
URL: https://github.com/apache/doris/pull/35284#discussion_r1635933149


##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java:
##########
@@ -846,6 +862,10 @@ public void checkSchemaChangeAllowed(Column other) throws 
DdlException {
             return;
         }
         // TODO check cluster key
+
+        if (generatedColumnInfo != null || other.getGeneratedColumnInfo() != 
null) {
+            throw new DdlException("Temporarily not supporting alter table 
modify generated columns.");

Review Comment:
   not use Temporarily in error message
   ```suggestion
               throw new DdlException("not supporting alter table modify 
generated columns.");
   ```



##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java:
##########
@@ -1003,6 +1026,10 @@ private boolean addColumnInternal(OlapTable olapTable, 
Column newColumn, ColumnP
             }
         }
 
+        if (newColumn.getGeneratedColumnInfo() != null) {
+            throw  new DdlException("Temporarily not supporting alter table 
add generated columns.");

Review Comment:
   ```suggestion
               throw new DdlException("Temporarily not supporting alter table 
add generated columns.");
   ```



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java:
##########
@@ -711,4 +719,156 @@ public String toString() {
     public boolean needAuditEncryption() {
         return !engineName.equals("olap");
     }
+
+    private void generatedColumnCheck(Analyzer analyzer) throws 
AnalysisException {
+        generatedColumnCommonCheck();
+        Map<String, Pair<ColumnDef, Integer>> nameToColumnDef = 
Maps.newHashMap();
+        for (int i = 0; i < columnDefs.size(); i++) {
+            ColumnDef columnDef = columnDefs.get(i);
+            nameToColumnDef.put(columnDef.getName(), Pair.of(columnDef, i));
+        }
+        SlotRefRewriteRule.initializeslotRefMap(nameToColumnDef);
+        List<GeneratedColumnUtil.ExprAndname> exprAndnames = 
Lists.newArrayList();
+        for (int i = 0; i < columnDefs.size(); i++) {
+            ColumnDef columnDef = columnDefs.get(i);
+            if (!columnDef.getGeneratedColumnInfo().isPresent()) {
+                continue;
+            }
+            SlotRefRewriteRule slotRefRewriteRule = new SlotRefRewriteRule(i);
+            ExprRewriter rewriter = new ExprRewriter(slotRefRewriteRule);
+            GeneratedColumnInfo generatedColumnInfo = 
columnDef.getGeneratedColumnInfo().get();
+            Expr expr = rewriter.rewrite(generatedColumnInfo.getExpr(), 
analyzer);
+            expr.foreach(e -> {
+                if (e instanceof LambdaFunctionCallExpr) {
+                    throw new AnalysisException("Generated column does not 
support lambda.");
+                } else if (e instanceof Subquery) {
+                    throw new AnalysisException("Generated column does not 
support subquery.");
+                }
+            });
+            try {
+                expr.analyze(analyzer);
+            } catch (AnalysisException e) {
+                throw new AnalysisException("In generated column '" + 
columnDef.getName() + "', "
+                        + Utils.convertFirstChar(e.getDetailMessage()));
+            }
+            expr.foreach(e -> {
+                if (e instanceof VariableExpr) {
+                    throw new AnalysisException("Generated column expression 
cannot contain variable.");
+                } else if (e instanceof SlotRef && 
nameToColumnDef.containsKey(((SlotRef) e).getColumnName())) {
+                    ColumnDef refColumnDef = nameToColumnDef.get(((SlotRef) 
e).getColumnName()).first;
+                    if (refColumnDef.getAutoIncInitValue() != -1) {
+                        throw new AnalysisException("Generated column '" + 
columnDef.getName()
+                                + "' cannot refer to auto-increment column.");
+                    }
+                } else if (e instanceof FunctionCallExpr && 
!checkFunctionInGeneratedColumn((FunctionCallExpr) e)) {
+                    throw new AnalysisException(
+                            "Expression of generated column '"
+                                    + columnDef.getName() + "' contains a 
disallowed function:'"
+                                    + ((FunctionCallExpr) e).getFnName() + 
"'");
+                } else if (e instanceof AnalyticExpr) {
+                    throw new AnalysisException(
+                            "Expression of generated column '"
+                                    + columnDef.getName() + "' contains a 
disallowed expression:'"
+                                    + ((AnalyticExpr) e).toSqlWithoutTbl() + 
"'");
+                }
+            });
+            if (!Type.canCastTo(expr.getType(), columnDef.getType())) {
+                throw new AnalysisException("can not cast from origin type "
+                        + expr.getType() + " to target type=" + 
columnDef.getType());
+            }
+            exprAndnames.add(new GeneratedColumnUtil.ExprAndname(expr, 
columnDef.getName()));
+        }
+
+        // for alter drop column
+        Map<String, List<String>> 
columnToListOfGeneratedColumnsThatReferToThis = new HashMap<>();
+        for (ColumnDef column : columnDefs) {
+            Optional<GeneratedColumnInfo> info = 
column.getGeneratedColumnInfo();
+            if (!info.isPresent()) {
+                continue;
+            }
+            Expr generatedExpr = info.get().getExpr();
+            Set<Expr> slotRefsInGeneratedExpr = new HashSet<>();
+            generatedExpr.collect(e -> e instanceof SlotRef, 
slotRefsInGeneratedExpr);
+            for (Expr slotRefExpr : slotRefsInGeneratedExpr) {
+                String name = ((SlotRef) slotRefExpr).getColumnName();
+                columnToListOfGeneratedColumnsThatReferToThis
+                        .computeIfAbsent(name, k -> new ArrayList<>())
+                        .add(column.getName());
+            }
+        }
+        for (Map.Entry<String, List<String>> entry : 
columnToListOfGeneratedColumnsThatReferToThis.entrySet()) {
+            ColumnDef col = nameToColumnDef.get(entry.getKey()).first;
+            col.addGeneratedColumnsThatReferToThis(entry.getValue());
+        }
+
+        GeneratedColumnUtil.rewriteColumns(exprAndnames);
+        for (GeneratedColumnUtil.ExprAndname exprAndname : exprAndnames) {
+            if (nameToColumnDef.containsKey(exprAndname.getName())) {
+                ColumnDef columnDef = 
nameToColumnDef.get(exprAndname.getName()).first;
+                Optional<GeneratedColumnInfo> info = 
columnDef.getGeneratedColumnInfo();
+                info.ifPresent(columnInfo -> 
columnInfo.setExpandExprForLoad(exprAndname.getExpr()));
+            }
+        }
+    }
+
+    private boolean checkFunctionInGeneratedColumn(FunctionCallExpr funcExpr) {
+        if (!funcExpr.isScalarFunction()) {
+            return false;
+        }
+        if (funcExpr.fn.isUdf()) {
+            return false;
+        }
+        if (funcExpr instanceof GroupingFunctionCallExpr) {
+            return false;
+        }
+        return true;
+    }
+
+    public static final class SlotRefRewriteRule implements ExprRewriteRule {
+        private static Map<String, Pair<ColumnDef, Integer>> 
nameToColumnDefMap = new HashMap<>();
+        private final int index;
+
+        public SlotRefRewriteRule(int index) {
+            this.index = index;
+        }
+
+        public static void initializeslotRefMap(Map<String, Pair<ColumnDef, 
Integer>>  map) {
+            nameToColumnDefMap = map;
+        }
+
+        @Override
+        public Expr apply(Expr expr, Analyzer analyzer, 
ExprRewriter.ClauseType clauseType) throws AnalysisException {
+            if (!(expr instanceof SlotRef)) {
+                return expr;
+            }
+            SlotRef slotRef = (SlotRef) expr;
+            if (!nameToColumnDefMap.containsKey(slotRef.getColumnName())) {
+                throw new AnalysisException("Unknown column '" + 
slotRef.getColumnName()
+                        + "' in 'generated column function'");
+            }
+            Pair<ColumnDef, Integer> columnAndIdx = 
nameToColumnDefMap.get(slotRef.getColumnName());
+            ColumnDef columnDef = columnAndIdx.first;
+            if (columnDef.getGeneratedColumnInfo().isPresent() && 
columnAndIdx.second >= index) {
+                throw new AnalysisException(
+                        "Generated column can refer only to generated columns 
defined prior to it.");
+            }
+            slotRef.setType(columnDef.getType());
+            slotRef.setAnalyzed(true);
+            return slotRef;
+        }
+    }
+
+    private void generatedColumnCommonCheck() {
+        for (ColumnDef column : columnDefs) {
+            if (keysDesc != null && keysDesc.getKeysType() == KeysType.AGG_KEYS
+                    && column.getGeneratedColumnInfo().isPresent() && 
!column.isKey()) {
+                throw new 
org.apache.doris.nereids.exceptions.AnalysisException(
+                        "Generated Columns in aggregate table must be keys.");

Review Comment:
   do not throw nereids' exception in non-nereids package



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to