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