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

Reply via email to