morrySnow commented on code in PR #58031:
URL: https://github.com/apache/doris/pull/58031#discussion_r2576040695
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Database.java:
##########
@@ -798,7 +798,12 @@ public synchronized void addFunction(Function function,
boolean ifNotExists) thr
function.checkWritable();
if (FunctionUtil.addFunctionImpl(function, ifNotExists, false,
name2Function)) {
Env.getCurrentEnv().getEditLog().logAddFunction(function);
- FunctionUtil.translateToNereids(this.getFullName(), function);
+ try {
+ FunctionUtil.translateToNereidsThrows(this.getFullName(),
function);
+ } catch (Exception e) {
+ name2Function.remove(function.getFunctionName().getFunction());
+ throw e;
+ }
Review Comment:
why throw exception here? we should not throw any exception after record
edit log
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/SlotReference.java:
##########
@@ -220,6 +220,17 @@ public String toString() {
}
return name.get() + "['" + String.join("']['", subPath) + "']" + "#" +
exprId;
}
+ // @Override
Review Comment:
? why comment these code blocks
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java:
##########
@@ -441,15 +441,15 @@ protected void addShadowIndexToCatalog(OlapTable tbl) {
partition.createRollupIndex(shadowIndex);
}
}
-
+ // ?这里需要传入session variables吗？think
Review Comment:
remove this comment
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java:
##########
@@ -441,15 +441,15 @@ protected void addShadowIndexToCatalog(OlapTable tbl) {
partition.createRollupIndex(shadowIndex);
}
}
-
+ // ?这里需要传入session variables吗？think
for (long shadowIdxId : indexIdMap.keySet()) {
MaterializedIndexMeta originalIndexMeta =
tbl.getIndexMetaByIndexId(indexIdMap.get(shadowIdxId));
tbl.setIndexMeta(shadowIdxId, indexIdToName.get(shadowIdxId),
indexSchemaMap.get(shadowIdxId),
indexSchemaVersionAndHashMap.get(shadowIdxId).schemaVersion,
indexSchemaVersionAndHashMap.get(shadowIdxId).schemaHash,
indexShortKeyMap.get(shadowIdxId), TStorageType.COLUMN,
tbl.getKeysTypeByIndexId(indexIdMap.get(shadowIdxId)),
originalIndexMeta.getDefineStmt(),
- indexChange ? indexes : originalIndexMeta.getIndexes());
+ indexChange ? indexes : originalIndexMeta.getIndexes(),
null);
Review Comment:
why null? add comment to explain it
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/memo/StructInfoMap.java:
##########
@@ -80,8 +81,9 @@ public class StructInfoMap {
}
if (groupExpressionMap.containsKey(tableMap)) {
Pair<GroupExpression, List<BitSet>> groupExpressionBitSetPair =
getGroupExpressionWithChildren(tableMap);
- structInfo = constructStructInfo(groupExpressionBitSetPair.first,
groupExpressionBitSetPair.second,
- originPlan, cascadesContext);
+ structInfo = MoreFieldsThread.keepFunctionSignature(() ->
Review Comment:
add a NOTICE comment to explain why need to use `keepFunctionSignature`
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/DecimalV3Literal.java:
##########
@@ -62,6 +62,14 @@ public DecimalV3Literal(DecimalV3Type dataType, BigDecimal
value) {
this.value = Objects.requireNonNull(adjustedValue);
}
+ public static DecimalV3Literal createWithCheck256(BigDecimal value) {
+ return new DecimalV3Literal(DecimalV3Type.createDecimalV3Type(value),
value);
+ }
+
+ public static DecimalV3Literal createWithoutCheck256(BigDecimal value) {
+ return new
DecimalV3Literal(DecimalV3Type.createDecimalV3TypeNotCheck256(value), value);
+ }
Review Comment:
add comment to explain these two functions
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/NeedSessionVarGuard.java:
##########
@@ -15,17 +15,10 @@
// specific language governing permissions and limitations
// under the License.
-suite("test_decimal_sum") {
- sql "set enable_agg_state=true"
- sql """ DROP TABLE IF EXISTS t01; """
- sql """
- create table t01(id int, decimal_col agg_state<sum(decimal(20,6))>
generic) properties ("replication_num" = "1");
- """
+package org.apache.doris.nereids.trees.expressions;
- sql """insert into t01 values (1, sum_state(10.1)), (1, sum_state(20.1)),
(2, sum_state(10.2)), (2, sum_state(11.0));
-"""
-
-
- qt_select """ select sum_merge(decimal_col) from t01 group by id order by
id;
- """
+/**
+ * expressions that need session variables guard
+ * */
+public interface NeedSessionVarGuard {
Review Comment:
add a comment to explain which type of function should add this trait when
add a new function
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinition.java:
##########
@@ -533,7 +537,9 @@ public Column translateToCatalogStyleForSchemaChange() {
defaultValue.map(DefaultValue::getRawValue).orElse(null),
onUpdateDefaultValue.isPresent(),
onUpdateDefaultValue.map(DefaultValue::getDefaultValueExprDef).orElse(null),
clusterKeyId,
generatedColumnDesc.map(GeneratedColumnDesc::translateToInfo).orElse(null),
- generatedColumnsThatReferToThis);
+ generatedColumnsThatReferToThis,
+ // 这里所有的列都加上了变量，以后改一下
+
ConnectContextUtil.getAffectQueryResultSessionVariables(ConnectContext.get()));
Review Comment:
ditto
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/MergeGuardExpr.java:
##########
@@ -0,0 +1,52 @@
+// 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.rules.expression;
+
+import org.apache.doris.nereids.exceptions.AnalysisException;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NeedSessionVarGuard;
+import org.apache.doris.nereids.trees.expressions.SessionVarGuardExpr;
+
+/**MergeGuardExpr*/
+public class MergeGuardExpr extends AbstractExpressionRewriteRule {
+ public static final MergeGuardExpr INSTANCE = new MergeGuardExpr();
+
+ @Override
+ public Expression visitSessionVarGuardExpr(SessionVarGuardExpr
sessionVarGuardExpr,
+ ExpressionRewriteContext context) {
+ Expression child = sessionVarGuardExpr.child();
+ Expression res;
+ if (child instanceof NeedSessionVarGuard) {
+ res = sessionVarGuardExpr;
+ return super.visit(res, context);
+ } else {
+ if (child instanceof SessionVarGuardExpr) {
+ if
(sessionVarGuardExpr.getSessionVars().equals(((SessionVarGuardExpr)
child).getSessionVars())) {
Review Comment:
why must equals to child?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/SessionVarGuardExpr.java:
##########
@@ -0,0 +1,147 @@
+// 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.trees.expressions;
+
+import org.apache.doris.nereids.exceptions.UnboundException;
+import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.DataType;
+import org.apache.doris.qe.AutoCloseSessionVariable;
+import org.apache.doris.qe.ConnectContext;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+/**
+ * A transparent wrapper expression that temporarily applies given session
variables
+ * during type computation and SQL rendering of its child expression. This is
used
+ * to preserve precision/scale decisions for generated columns and alias
functions
+ * that depend on affectQueryResult session variables (e.g. enable_decimal256,
decimalOverflowScale).
+ */
+public class SessionVarGuardExpr extends Expression implements UnaryExpression
{
+
+ private final Map<String, String> sessionVars;
+
+ public SessionVarGuardExpr(Expression child, Map<String, String>
sessionVars) {
+ super(ImmutableList.of(child));
+ Preconditions.checkNotNull(sessionVars, "sessionVars cannot be null in
SessionVarGuardExpr");
Review Comment:
should copyOf to ImmutableMap
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinition.java:
##########
@@ -518,7 +520,9 @@ public Column translateToCatalogStyle() {
defaultValue.map(DefaultValue::getValue).orElse(null),
onUpdateDefaultValue.isPresent(),
onUpdateDefaultValue.map(DefaultValue::getDefaultValueExprDef).orElse(null),
clusterKeyId,
generatedColumnDesc.map(GeneratedColumnDesc::translateToInfo).orElse(null),
- generatedColumnsThatReferToThis);
+ generatedColumnsThatReferToThis,
+ // 这里所有的列都加上了变量，以后改一下
+
ConnectContextUtil.getAffectQueryResultSessionVariables(ConnectContext.get()));
Review Comment:
finish this comment
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/CreateMaterializedViewCommand.java:
##########
@@ -172,7 +172,7 @@ public Column getWhereClauseItemColumn(OlapTable olapTable)
throws DdlException
if (whereClauseItem == null) {
return null;
}
- return whereClauseItem.toMVColumn(olapTable);
+ return whereClauseItem.toMVColumn(olapTable, null);
Review Comment:
why null? add comment to explain it
--
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]
