imay closed pull request #283: Fix truncation error in CastExpr
URL: https://github.com/apache/incubator-doris/pull/283
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/be/src/exprs/anyval_util.cpp b/be/src/exprs/anyval_util.cpp
index 4915fa63..515c7c2f 100755
--- a/be/src/exprs/anyval_util.cpp
+++ b/be/src/exprs/anyval_util.cpp
@@ -139,7 +139,7 @@ FunctionContext::TypeDesc
AnyValUtil::column_type_to_type_desc(const TypeDescrip
out.len = type.len;
break;
case TYPE_CHAR:
- out.type = FunctionContext::TYPE_FIXED_BUFFER;
+ out.type = FunctionContext::TYPE_CHAR;
out.len = type.len;
break;
case TYPE_DECIMAL:
diff --git a/be/src/exprs/anyval_util.h b/be/src/exprs/anyval_util.h
index 0b2a81a4..92917b3d 100755
--- a/be/src/exprs/anyval_util.h
+++ b/be/src/exprs/anyval_util.h
@@ -287,7 +287,8 @@ class AnyValUtil {
}
static void TruncateIfNecessary(const FunctionContext::TypeDesc& type,
StringVal *val) {
- if (type.type == FunctionContext::TYPE_VARCHAR) {
+ if (type.type == FunctionContext::TYPE_VARCHAR
+ || type.type == FunctionContext::TYPE_CHAR) {
DCHECK(type.len >= 0);
val->len = std::min(val->len, type.len);
}
diff --git a/be/src/exprs/cast_functions.cpp b/be/src/exprs/cast_functions.cpp
index 554fab1a..7898149c 100644
--- a/be/src/exprs/cast_functions.cpp
+++ b/be/src/exprs/cast_functions.cpp
@@ -170,9 +170,21 @@ StringVal
CastFunctions::cast_to_string_val(FunctionContext* ctx, const LargeInt
if (UNLIKELY(sv.is_null)) { \
return sv; \
} \
- sv.len = snprintf(reinterpret_cast<char*>(sv.ptr), sv.len, format,
val.val); \
- DCHECK_GT(sv.len, 0); \
- DCHECK_LE(sv.len, MAX_FLOAT_CHARS); \
+ const FunctionContext::TypeDesc& returnType = ctx->get_return_type(); \
+ if (returnType.len > 0) { \
+ sv.len = snprintf(reinterpret_cast<char*>(sv.ptr), sv.len, format,
val.val); \
+ DCHECK_GT(sv.len, 0); \
+ DCHECK_LE(sv.len, MAX_FLOAT_CHARS); \
+ AnyValUtil::TruncateIfNecessary(returnType, &sv); \
+ } else if (returnType.len == -1) { \
+ std::stringstream ss; \
+ ss << val.val; \
+ std::string str = ss.str(); \
+ sv.len = str.length(); \
+ memcpy(sv.ptr, str.c_str(), str.length()); \
+ } else { \
+ DCHECK(false); \
+ } \
return sv; \
}
diff --git a/fe/src/main/cup/sql_parser.cup b/fe/src/main/cup/sql_parser.cup
index 24d5e4c3..27660c43 100644
--- a/fe/src/main/cup/sql_parser.cup
+++ b/fe/src/main/cup/sql_parser.cup
@@ -24,6 +24,7 @@ import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.KeysType;
import org.apache.doris.catalog.PrimitiveType;
import org.apache.doris.catalog.ColumnType;
+import org.apache.doris.catalog.ScalarType;
import org.apache.doris.catalog.Type;
import org.apache.doris.catalog.View;
import org.apache.doris.catalog.AggregateType;
@@ -189,7 +190,7 @@ parser code {:
}
:};
-// Total keywords of palo
+// Total keywords of doris
terminal String KW_ADD, KW_ADMIN, KW_AFTER, KW_AGGREGATE, KW_ALL, KW_ALTER,
KW_AND, KW_ANTI, KW_AS, KW_ASC, KW_AUTHORS,
KW_BACKEND, KW_BACKUP, KW_BETWEEN, KW_BEGIN, KW_BIGINT, KW_BOOLEAN,
KW_BOTH, KW_BROKER, KW_BACKENDS, KW_BY,
KW_CANCEL, KW_CASE, KW_CAST, KW_CHAIN, KW_CHAR, KW_CHARSET, KW_CLUSTER,
KW_CLUSTERS,
@@ -302,6 +303,8 @@ nonterminal OrderByElement order_by_element;
nonterminal Boolean opt_order_param;
nonterminal Boolean opt_nulls_order_param;
nonterminal LimitElement limit_clause;
+nonterminal TypeDef type_def;
+nonterminal Type type;
nonterminal Expr cast_expr, case_else_clause, analytic_expr;
nonterminal LiteralExpr literal;
nonterminal CaseExpr case_expr;
@@ -3016,17 +3019,49 @@ limit_clause ::=
{: RESULT = new LimitElement(offset.longValue(), limit.longValue()); :}
;
+type ::=
+ KW_TINYINT
+ {: RESULT = Type.TINYINT; :}
+ | KW_SMALLINT
+ {: RESULT = Type.SMALLINT; :}
+ | KW_INT
+ {: RESULT = Type.INT; :}
+ | KW_BIGINT
+ {: RESULT = Type.BIGINT; :}
+ | KW_BOOLEAN
+ {: RESULT = Type.BOOLEAN; :}
+ | KW_FLOAT
+ {: RESULT = Type.FLOAT; :}
+ | KW_DOUBLE
+ {: RESULT = Type.DOUBLE; :}
+ | KW_DATE
+ {: RESULT = Type.DATE; :}
+ | KW_DATETIME
+ {: RESULT = Type.DATETIME; :}
+ | KW_VARCHAR LPAREN INTEGER_LITERAL:len RPAREN
+ {: RESULT = ScalarType.createVarcharType(len.intValue()); :}
+ | KW_VARCHAR
+ {: RESULT = ScalarType.createVarcharType(-1); :}
+ | KW_CHAR LPAREN INTEGER_LITERAL:len RPAREN
+ {: RESULT = ScalarType.createCharType(len.intValue()); :}
+ | KW_CHAR
+ {: RESULT = ScalarType.createCharType(-1); :}
+ | KW_DECIMAL LPAREN INTEGER_LITERAL:precision RPAREN
+ {: RESULT = ScalarType.createDecimalType(precision.intValue()); :}
+ | KW_DECIMAL LPAREN INTEGER_LITERAL:precision COMMA INTEGER_LITERAL:scale
RPAREN
+ {: RESULT = ScalarType.createDecimalType(precision.intValue(),
scale.intValue()); :}
+ | KW_DECIMAL
+ {: RESULT = ScalarType.createDecimalType(); :}
+ ;
+
+type_def ::=
+ type:t
+ {: RESULT = new TypeDef(t); :}
+ ;
+
cast_expr ::=
- KW_CAST LPAREN expr:e KW_AS KW_STRING RPAREN
- {: RESULT = new CastExpr(Type.VARCHAR, e, false); :}
- | KW_CAST LPAREN expr:e KW_AS primitive_type:targetType RPAREN
- {: RESULT = new CastExpr(Type.fromPrimitiveType((PrimitiveType) targetType),
e, false); :}
- | KW_CAST LPAREN expr:e KW_AS primitive_type:targetType LPAREN
non_pred_expr:e1 RPAREN RPAREN
- {: RESULT = new CastExpr(Type.fromPrimitiveType((PrimitiveType) targetType),
e, false); :}
- | KW_CAST LPAREN expr:e KW_AS KW_CHAR opt_charset_name RPAREN
- {: RESULT = new CastExpr(Type.fromPrimitiveType(PrimitiveType.VARCHAR), e,
false); :}
- | KW_CAST LPAREN expr:e KW_AS KW_CHAR LPAREN non_pred_expr:e1 RPAREN
opt_charset_name RPAREN
- {: RESULT = new CastExpr(Type.fromPrimitiveType(PrimitiveType.VARCHAR), e,
false); :}
+ KW_CAST LPAREN expr:e KW_AS type_def:targetType RPAREN
+ {: RESULT = new CastExpr(targetType, e); :}
;
case_expr ::=
diff --git a/fe/src/main/java/org/apache/doris/analysis/CastExpr.java
b/fe/src/main/java/org/apache/doris/analysis/CastExpr.java
index bbcc0665..96104b53 100644
--- a/fe/src/main/java/org/apache/doris/analysis/CastExpr.java
+++ b/fe/src/main/java/org/apache/doris/analysis/CastExpr.java
@@ -38,24 +38,25 @@
public class CastExpr extends Expr {
private static final Logger LOG = LogManager.getLogger(CastExpr.class);
- private final Type targetType;
- /**
- * true if this is a "pre-analyzed" implicit cast
- */
+ // Only set for explicit casts. Null for implicit casts.
+ private final TypeDef targetTypeDef;
+
+ // True if this is a "pre-analyzed" implicit cast.
private final boolean isImplicit;
// True if this cast does not change the type.
private boolean noOp = false;
- public CastExpr(Type targetType, Expr e, boolean isImplicit) {
+ public CastExpr(Type targetType, Expr e) {
super();
- Preconditions.checkArgument(targetType != Type.INVALID);
- this.targetType = targetType;
- this.isImplicit = isImplicit;
+ Preconditions.checkArgument(targetType.isValid());
Preconditions.checkNotNull(e);
+ type = targetType;
+ targetTypeDef = null;
+ isImplicit = true;
+
children.add(e);
if (isImplicit) {
- type = targetType;
try {
analyze();
} catch (AnalysisException ex) {
@@ -66,9 +67,20 @@ public CastExpr(Type targetType, Expr e, boolean isImplicit)
{
}
}
+ /**
+ * Copy c'tor used in clone().
+ */
+ protected CastExpr(TypeDef targetTypeDef, Expr e) {
+ Preconditions.checkNotNull(targetTypeDef);
+ Preconditions.checkNotNull(e);
+ this.targetTypeDef = targetTypeDef;
+ isImplicit = false;
+ children.add(e);
+ }
+
protected CastExpr(CastExpr other) {
super(other);
- targetType = other.targetType;
+ targetTypeDef = other.targetTypeDef;
isImplicit = other.isImplicit;
noOp = other.noOp;
}
@@ -94,42 +106,6 @@ public static void initBuiltins(FunctionSet functionSet) {
if ((fromType.isBoolean() || fromType.isDateType()) && toType
== Type.DECIMAL) {
continue;
}
-// if (fromType.getPrimitiveType() == PrimitiveType.CHAR
-// && toType.getPrimitiveType() == PrimitiveType.CHAR) {
-// // Allow casting from CHAR(N) to Char(N)
-// String beSymbol = "impala::CastFunctions::CastToChar";
-//
functionSet.addBuiltin(ScalarFunction.createBuiltin(getFnName(ScalarType.CHAR),
-// Lists.newArrayList((Type)
ScalarType.createCharType(-1)), false,
-// ScalarType.CHAR, beSymbol, null, null, true));
-// continue;
-// }
-// if (fromType.getPrimitiveType() == PrimitiveType.VARCHAR
-// && toType.getPrimitiveType() ==
PrimitiveType.VARCHAR) {
-// // Allow casting from VARCHAR(N) to VARCHAR(M)
-// String beSymbol =
"impala::CastFunctions::CastToStringVal";
-//
functionSet.addBuiltin(ScalarFunction.createBuiltin(getFnName(ScalarType.VARCHAR),
-// Lists.newArrayList((Type) ScalarType.VARCHAR),
false, ScalarType.VARCHAR,
-// beSymbol, null, null, true));
-// continue;
-// }
-// if (fromType.getPrimitiveType() == PrimitiveType.VARCHAR
-// && toType.getPrimitiveType() == PrimitiveType.CHAR) {
-// // Allow casting from VARCHAR(N) to CHAR(M)
-// String beSymbol = "impala::CastFunctions::CastToChar";
-//
functionSet.addBuiltin(ScalarFunction.createBuiltin(getFnName(ScalarType.CHAR),
-// Lists.newArrayList((Type) ScalarType.VARCHAR),
false, ScalarType.CHAR,
-// beSymbol, null, null, true));
-// continue;
-// }
-// if (fromType.getPrimitiveType() == PrimitiveType.CHAR
-// && toType.getPrimitiveType() ==
PrimitiveType.VARCHAR) {
-// // Allow casting from CHAR(N) to VARCHAR(M)
-// String beSymbol =
"impala::CastFunctions::CastToStringVal";
-//
functionSet.addBuiltin(ScalarFunction.createBuiltin(getFnName(ScalarType.VARCHAR),
-// Lists.newArrayList((Type) ScalarType.CHAR),
false, ScalarType.VARCHAR,
-// beSymbol, null, null, true));
-// continue;
-// }
// Disable no-op casts
if (fromType.equals(toType)) {
@@ -158,10 +134,10 @@ public String toSql() {
if (isImplicit) {
return getChild(0).toSql();
}
- if (targetType.isStringType()) {
+ if (type.isStringType()) {
return "CAST(" + getChild(0).toSql() + " AS " + "CHARACTER" + ")";
} else {
- return "CAST(" + getChild(0).toSql() + " AS " +
targetType.toString() + ")";
+ return "CAST(" + getChild(0).toSql() + " AS " + type.toString() +
")";
}
}
@@ -193,7 +169,7 @@ private void analyze() throws AnalysisException {
Type childType = getChild(0).getType();
// this cast may result in loss of precision, but the user requested it
- if (childType.equals(targetType)) {
+ if (childType.equals(type)) {
noOp = true;
return;
}
@@ -219,7 +195,9 @@ private void analyze() throws AnalysisException {
@Override
public void analyzeImpl(Analyzer analyzer) throws AnalysisException {
- type = targetType;
+ Preconditions.checkState(!isImplicit);
+ targetTypeDef.analyze(analyzer);
+ type = targetTypeDef.getType();
analyze();
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/Expr.java
b/fe/src/main/java/org/apache/doris/analysis/Expr.java
index 27281521..4d652924 100644
--- a/fe/src/main/java/org/apache/doris/analysis/Expr.java
+++ b/fe/src/main/java/org/apache/doris/analysis/Expr.java
@@ -1171,7 +1171,7 @@ public final Expr castTo(Type targetType) throws
AnalysisException {
* failure to convert a string literal to a date
literal
*/
protected Expr uncheckedCastTo(Type targetType) throws AnalysisException {
- return new CastExpr(targetType, this, true);
+ return new CastExpr(targetType, this);
}
/**
diff --git a/fe/src/main/java/org/apache/doris/analysis/TypeDef.java
b/fe/src/main/java/org/apache/doris/analysis/TypeDef.java
new file mode 100644
index 00000000..f6a51305
--- /dev/null
+++ b/fe/src/main/java/org/apache/doris/analysis/TypeDef.java
@@ -0,0 +1,124 @@
+// 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.analysis;
+
+import org.apache.doris.catalog.PrimitiveType;
+import org.apache.doris.catalog.ScalarType;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.common.AnalysisException;
+
+import com.google.common.base.Preconditions;
+
+/**
+ * Represents an anonymous type definition, e.g., used in DDL and CASTs.
+ */
+public class TypeDef implements ParseNode {
+ private boolean isAnalyzed;
+ private final Type parsedType;
+
+ public TypeDef(Type parsedType) {
+ this.parsedType = parsedType;
+ }
+
+ @Override
+ public void analyze(Analyzer analyzer) throws AnalysisException {
+ if (isAnalyzed) {
+ return;
+ }
+ // Check the max nesting depth before calling the recursive analyze() to
avoid
+ // a stack overflow.
+ if (parsedType.exceedsMaxNestingDepth()) {
+ throw new AnalysisException(String.format(
+ "Type exceeds the maximum nesting depth of %s:\n%s",
+ Type.MAX_NESTING_DEPTH, parsedType.toSql()));
+ }
+ analyze(parsedType);
+ isAnalyzed = true;
+ }
+
+ private void analyze(Type type) throws AnalysisException {
+ if (!type.isSupported()) {
+ throw new AnalysisException("Unsupported data type: " + type.toSql());
+ }
+ if (type.isScalarType()) {
+ analyzeScalarType((ScalarType) type);
+ }
+ }
+
+ private void analyzeScalarType(ScalarType scalarType)
+ throws AnalysisException {
+ PrimitiveType type = scalarType.getPrimitiveType();
+ switch (type) {
+ case CHAR:
+ case VARCHAR: {
+ String name;
+ int maxLen;
+ if (type == PrimitiveType.VARCHAR) {
+ name = "Varchar";
+ maxLen = ScalarType.MAX_VARCHAR_LENGTH;
+ } else if (type == PrimitiveType.CHAR) {
+ name = "Char";
+ maxLen = ScalarType.MAX_CHAR_LENGTH;
+ } else {
+ Preconditions.checkState(false);
+ return;
+ }
+ int len = scalarType.getLength();
+ // len is decided by child, when it is -1.
+ if (len < -1) {
+ throw new AnalysisException(name + " size must be > 0: " + len);
+ }
+ if (scalarType.getLength() > maxLen) {
+ throw new AnalysisException(
+ name + " size must be <= " + maxLen + ": " + len);
+ }
+ break;
+ }
+ case DECIMAL: {
+ int precision = scalarType.decimalPrecision();
+ int scale = scalarType.decimalScale();
+ if (precision > ScalarType.MAX_PRECISION) {
+ throw new AnalysisException("Decimal precision must be <= " +
+ ScalarType.MAX_PRECISION + ": " + precision);
+ }
+ if (precision == 0) {
+ throw new AnalysisException("Decimal precision must be > 0: " +
precision);
+ }
+ if (scale > precision) {
+ throw new AnalysisException("Decimal scale (" + scale + ") must be
<= " +
+ "precision (" + precision + ")");
+ }
+ }
+ default: break;
+ }
+ }
+
+ public Type getType() {
+ return parsedType;
+ }
+
+ @Override
+ public String toString() {
+ return parsedType.toSql();
+ }
+
+ @Override
+ public String toSql() {
+ return parsedType.toSql();
+ }
+}
diff --git a/fe/src/main/java/org/apache/doris/planner/BrokerScanNode.java
b/fe/src/main/java/org/apache/doris/planner/BrokerScanNode.java
index 8ade6c90..b773cf29 100644
--- a/fe/src/main/java/org/apache/doris/planner/BrokerScanNode.java
+++ b/fe/src/main/java/org/apache/doris/planner/BrokerScanNode.java
@@ -419,13 +419,11 @@ private Expr castToSlot(SlotDescriptor slotDesc, Expr
expr) throws AnalysisExcep
if (srcType.isStringType()) {
return expr;
} else {
- CastExpr castExpr = new CastExpr(Type.VARCHAR, expr, true);
- castExpr.analyze(analyzer);
+ CastExpr castExpr = (CastExpr)expr.castTo(Type.VARCHAR);
return castExpr;
}
} else if (destType != srcType) {
- CastExpr castExpr = new CastExpr(slotDesc.getType(), expr, true);
- castExpr.analyze(analyzer);
+ CastExpr castExpr = (CastExpr)expr.castTo(slotDesc.getType());
return castExpr;
}
diff --git a/fe/src/main/java/org/apache/doris/planner/StreamLoadScanNode.java
b/fe/src/main/java/org/apache/doris/planner/StreamLoadScanNode.java
index c56ac4f6..8d0a4c2a 100644
--- a/fe/src/main/java/org/apache/doris/planner/StreamLoadScanNode.java
+++ b/fe/src/main/java/org/apache/doris/planner/StreamLoadScanNode.java
@@ -336,13 +336,11 @@ private Expr castToSlot(SlotDescriptor slotDesc, Expr
expr) throws UserException
if (srcType.isStringType()) {
return expr;
} else {
- CastExpr castExpr = new CastExpr(Type.VARCHAR, expr, true);
- castExpr.analyze(analyzer);
+ CastExpr castExpr = (CastExpr)expr.castTo(Type.VARCHAR);
return castExpr;
}
} else if (dstType != srcType) {
- CastExpr castExpr = new CastExpr(slotDesc.getType(), expr, true);
- castExpr.analyze(analyzer);
+ CastExpr castExpr = (CastExpr)expr.castTo(slotDesc.getType());
return castExpr;
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
