morrySnow commented on code in PR #25386: URL: https://github.com/apache/doris/pull/25386#discussion_r1368173681
########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Multiply.java: ########## @@ -52,7 +53,12 @@ public DecimalV3Type getDataTypeForDecimalV3(DecimalV3Type t1, DecimalV3Type t2) int retPercision = t1.getPrecision() + t2.getPrecision(); int retScale = t1.getScale() + t2.getScale(); if (retPercision > DecimalV3Type.MAX_DECIMAL128_PRECISION) { - retPercision = DecimalV3Type.MAX_DECIMAL128_PRECISION; + boolean enableDecimal256 = ConnectContext.get().getSessionVariable().enableDecimal256(); + if (enableDecimal256) { + retPercision = DecimalV3Type.MAX_DECIMAL256_PRECISION; + } else { + retPercision = DecimalV3Type.MAX_DECIMAL128_PRECISION; + } Review Comment: this if statement means upper round precision if result precision larger than max precision for decimal type. so u should change ret precision to 256 only if enableDecimal256 and precision > MAX_DECIMAL256_PRECISION. if enableDecimal256 is false, u should keep original logic ########## fe/fe-common/src/main/java/org/apache/doris/catalog/PrimitiveType.java: ########## @@ -517,6 +556,7 @@ public static ImmutableSetMultimap<PrimitiveType, PrimitiveType> getImplicitCast builder.put(JSONB, DECIMAL32); builder.put(JSONB, DECIMAL64); builder.put(JSONB, DECIMAL128); + // builder.put(JSONB, DECIMAL256); Review Comment: why comment this line? if it is a temp comment, please add a TODO ########## fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java: ########## @@ -306,6 +307,10 @@ private void handleQuery(MysqlCommand mysqlCommand) { if (mysqlCommand == MysqlCommand.COM_QUERY && ctx.getSessionVariable().isEnableNereidsPlanner()) { try { stmts = new NereidsParser().parseSQL(originStmt); + } catch (NotSupportedException e) { Review Comment: why must add new exception type? ########## fe/fe-core/src/main/java/org/apache/doris/nereids/types/DecimalV3Type.java: ########## @@ -99,18 +103,49 @@ public static DecimalV3Type createDecimalV3Type(int precision) { /** createDecimalV3Type. */ public static DecimalV3Type createDecimalV3Type(int precision, int scale) { - Preconditions.checkArgument(precision > 0 && precision <= MAX_DECIMAL128_PRECISION, - "precision should in (0, " + MAX_DECIMAL128_PRECISION + "], but real precision is " + precision); + Preconditions.checkArgument(precision > 0 && precision <= MAX_DECIMAL256_PRECISION, + "precision should in (0, " + MAX_DECIMAL256_PRECISION + "], but real precision is " + precision); Preconditions.checkArgument(scale >= 0, "scale should not smaller than 0, but real scale is " + scale); Preconditions.checkArgument(precision >= scale, "precision should not smaller than scale," + " but precision is " + precision, ", scale is " + scale); - return new DecimalV3Type(precision, scale); + boolean enableDecimal256 = ConnectContext.get().getSessionVariable().enableDecimal256(); + if (precision > MAX_DECIMAL128_PRECISION && !enableDecimal256) { + throw new NotSupportedException("Datatype DecimalV3 with precision " + precision + + ", which is greater than 38 is disabled by default. set enable_decimal256 = true to enable it."); + } else { + return new DecimalV3Type(precision, scale); + } } public static DecimalV3Type createDecimalV3Type(BigDecimal bigDecimal) { int precision = org.apache.doris.analysis.DecimalLiteral.getBigDecimalPrecision(bigDecimal); int scale = org.apache.doris.analysis.DecimalLiteral.getBigDecimalScale(bigDecimal); - return createDecimalV3Type(precision, scale); + return createDecimalV3TypeLooseCheck(precision, scale); + } + + /** + * create DecimalV3Type, not throwing NotSupportedException. + */ + public static DecimalV3Type createDecimalV3TypeLooseCheck(int precision, int scale) { Review Comment: why need loose check and no check? ########## fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java: ########## @@ -1261,6 +1263,10 @@ public void setIgnoreShapePlanNodes(String ignoreShapePlanNodes) { "the plan node type which is ignored in 'explain shape plan' command"}) public String ignoreShapePlanNodes = ""; + @VariableMgr.VarAttr(name = ENABLE_DECIMAL256, description = { "控制是否在计算过程中使用Decimal256类型", + "Set to true to enable Decimal256 type" }) + public boolean enableDecimal256 = false; Review Comment: need forward? -- 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