You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jc...@apache.org on 2016/05/06 10:22:19 UTC
calcite git commit: [CALCITE-1199] Incorrect trimming of CHAR when
performing cast to VARCHAR
Repository: calcite
Updated Branches:
refs/heads/master 19361b94a -> dd51b5375
[CALCITE-1199] Incorrect trimming of CHAR when performing cast to VARCHAR
Close apache/calcite#221
Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/dd51b537
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/dd51b537
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/dd51b537
Branch: refs/heads/master
Commit: dd51b53756a42f03dd89b51e0d7b575db5ca7209
Parents: 19361b9
Author: Jesus Camacho Rodriguez <jc...@apache.org>
Authored: Sat Apr 23 00:58:52 2016 +0100
Committer: Jesus Camacho Rodriguez <jc...@apache.org>
Committed: Fri May 6 11:10:24 2016 +0100
----------------------------------------------------------------------
.../adapter/enumerable/RexToLixTranslator.java | 47 +++++++++---------
.../java/org/apache/calcite/rex/RexBuilder.java | 50 +++++++++++++------
.../apache/calcite/runtime/SqlFunctions.java | 51 +++++++++++++++++---
.../org/apache/calcite/util/BuiltInMethod.java | 1 +
.../calcite/sql/test/SqlOperatorBaseTest.java | 38 ++++++++++++++-
.../java/org/apache/calcite/test/JdbcTest.java | 2 +-
.../org/apache/calcite/test/RelOptRulesTest.xml | 2 +-
7 files changed, 143 insertions(+), 48 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/calcite/blob/dd51b537/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
index da3a62d..baba915 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
@@ -354,30 +354,16 @@ public class RexToLixTranslator {
if (convert == null) {
convert = convert(operand, typeFactory.getJavaClass(targetType));
}
- // Going from CHAR(n), trim.
- switch (sourceType.getSqlTypeName()) {
- case CHAR:
- switch (targetType.getSqlTypeName()) {
- case VARCHAR:
- convert = Expressions.call(
- BuiltInMethod.RTRIM.method, convert);
- }
- break;
- case BINARY:
- switch (targetType.getSqlTypeName()) {
- case VARBINARY:
- convert = Expressions.call(
- BuiltInMethod.RTRIM.method, convert);
- }
- break;
- }
// Going from anything to CHAR(n) or VARCHAR(n), make sure value is no
// longer than n.
- truncate:
+ boolean pad = false;
+ boolean truncate = true;
switch (targetType.getSqlTypeName()) {
case CHAR:
- case VARCHAR:
case BINARY:
+ pad = true;
+ // fall through
+ case VARCHAR:
case VARBINARY:
final int targetPrecision = targetType.getPrecision();
if (targetPrecision >= 0) {
@@ -391,14 +377,25 @@ public class RexToLixTranslator {
if (sourcePrecision < 0
|| sourcePrecision >= 0
&& sourcePrecision <= targetPrecision) {
- break truncate;
+ truncate = false;
+ }
+ // If this is a widening cast, no need to pad.
+ if (sourcePrecision < 0
+ || sourcePrecision >= 0
+ && sourcePrecision >= targetPrecision) {
+ pad = false;
}
+ // fall through
default:
- convert =
- Expressions.call(
- BuiltInMethod.TRUNCATE.method,
- convert,
- Expressions.constant(targetPrecision));
+ if (truncate || pad) {
+ convert =
+ Expressions.call(
+ pad
+ ? BuiltInMethod.TRUNCATE_OR_PAD.method
+ : BuiltInMethod.TRUNCATE.method,
+ convert,
+ Expressions.constant(targetPrecision));
+ }
}
}
break;
http://git-wip-us.apache.org/repos/asf/calcite/blob/dd51b537/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
index b0cc27e..1796784 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
@@ -494,21 +494,8 @@ public class RexBuilder {
RexLiteral literal = (RexLiteral) exp;
Comparable value = literal.getValue();
SqlTypeName typeName = literal.getTypeName();
- if (RexLiteral.valueMatchesType(value, sqlType, false)
- && (type.getSqlTypeName() == typeName
- || !SqlTypeFamily.DATETIME.getTypeNames().contains(typeName))
- && (!(value instanceof NlsString)
- || (type.getPrecision()
- >= ((NlsString) value).getValue().length()))
- && (!(value instanceof ByteString)
- || (type.getPrecision()
- >= ((ByteString) value).length()))) {
+ if (canRemoveCastFromLiteral(type, value, typeName)) {
switch (typeName) {
- case CHAR:
- if (value instanceof NlsString) {
- value = ((NlsString) value).rtrim();
- }
- break;
case TIMESTAMP:
case TIME:
assert value instanceof Calendar;
@@ -561,6 +548,41 @@ public class RexBuilder {
return makeAbstractCast(type, exp);
}
+ private boolean canRemoveCastFromLiteral(RelDataType toType, Comparable value,
+ SqlTypeName fromTypeName) {
+ final SqlTypeName sqlType = toType.getSqlTypeName();
+ if (!RexLiteral.valueMatchesType(value, sqlType, false)) {
+ return false;
+ }
+ if (toType.getSqlTypeName() != fromTypeName
+ && SqlTypeFamily.DATETIME.getTypeNames().contains(fromTypeName)) {
+ return false;
+ }
+ if (value instanceof NlsString) {
+ final int length = ((NlsString) value).getValue().length();
+ switch (toType.getSqlTypeName()) {
+ case CHAR:
+ return toType.getPrecision() == length;
+ case VARCHAR:
+ return toType.getPrecision() >= length;
+ default:
+ throw new AssertionError(toType);
+ }
+ }
+ if (value instanceof ByteString) {
+ final int length = ((ByteString) value).length();
+ switch (toType.getSqlTypeName()) {
+ case BINARY:
+ return toType.getPrecision() == length;
+ case VARBINARY:
+ return toType.getPrecision() >= length;
+ default:
+ throw new AssertionError(toType);
+ }
+ }
+ return true;
+ }
+
private RexNode makeCastExactToBoolean(RelDataType toType, RexNode exp) {
return makeCall(toType,
SqlStdOperatorTable.NOT_EQUALS,
http://git-wip-us.apache.org/repos/asf/calcite/blob/dd51b537/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
index eed92f5..7e2d84d 100644
--- a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
+++ b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
@@ -19,6 +19,7 @@ package org.apache.calcite.runtime;
import org.apache.calcite.DataContext;
import org.apache.calcite.avatica.util.ByteString;
import org.apache.calcite.avatica.util.DateTimeUtils;
+import org.apache.calcite.avatica.util.Spaces;
import org.apache.calcite.linq4j.AbstractEnumerable;
import org.apache.calcite.linq4j.CartesianProductEnumerator;
import org.apache.calcite.linq4j.Enumerable;
@@ -1248,16 +1249,54 @@ public class SqlFunctions {
/** Helper for CAST(... AS VARCHAR(maxLength)). */
public static String truncate(String s, int maxLength) {
- return s == null ? null
- : s.length() > maxLength ? s.substring(0, maxLength)
- : s;
+ if (s == null) {
+ return null;
+ } else if (s.length() > maxLength) {
+ return s.substring(0, maxLength);
+ } else {
+ return s;
+ }
+ }
+
+ /** Helper for CAST(... AS CHAR(maxLength)). */
+ public static String truncateOrPad(String s, int maxLength) {
+ if (s == null) {
+ return null;
+ } else {
+ final int length = s.length();
+ if (length > maxLength) {
+ return s.substring(0, maxLength);
+ } else {
+ return length < maxLength ? Spaces.padRight(s, maxLength) : s;
+ }
+ }
}
/** Helper for CAST(... AS VARBINARY(maxLength)). */
public static ByteString truncate(ByteString s, int maxLength) {
- return s == null ? null
- : s.length() > maxLength ? s.substring(0, maxLength)
- : s;
+ if (s == null) {
+ return null;
+ } else if (s.length() > maxLength) {
+ return s.substring(0, maxLength);
+ } else {
+ return s;
+ }
+ }
+
+ /** Helper for CAST(... AS BINARY(maxLength)). */
+ public static ByteString truncateOrPad(ByteString s, int maxLength) {
+ if (s == null) {
+ return null;
+ } else {
+ final int length = s.length();
+ if (length > maxLength) {
+ return s.substring(0, maxLength);
+ } else if (length < maxLength) {
+ return s.concat(new ByteString(new byte[maxLength - length]));
+ } else {
+ return s;
+ }
+ }
}
/** SQL {@code POSITION(seek IN string)} function. */
http://git-wip-us.apache.org/repos/asf/calcite/blob/dd51b537/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java b/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
index c069b91..94356a4 100644
--- a/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
+++ b/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
@@ -253,6 +253,7 @@ public enum BuiltInMethod {
int.class),
POSITION(SqlFunctions.class, "position", String.class, String.class),
TRUNCATE(SqlFunctions.class, "truncate", String.class, int.class),
+ TRUNCATE_OR_PAD(SqlFunctions.class, "truncateOrPad", String.class, int.class),
TRIM(SqlFunctions.class, "trim", boolean.class, boolean.class, String.class,
String.class),
TRANSLATE3(SqlFunctions.class, "translate3", String.class, String.class, String.class),
http://git-wip-us.apache.org/repos/asf/calcite/blob/dd51b537/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
index 6741b00..6c24eee 100644
--- a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
@@ -484,6 +484,8 @@ public abstract class SqlOperatorBaseTest {
@Test public void testCastToString() {
tester.setFor(SqlStdOperatorTable.CAST);
+ checkCastToString("cast(cast('abc' as char(4)) as varchar(6))", null,
+ "abc ");
// integer
checkCastToString("123", "CHAR(3)", "123");
@@ -547,6 +549,26 @@ public abstract class SqlOperatorBaseTest {
checkCastToString("'abc'", "CHAR(1)", "a");
checkCastToString("'abc'", "CHAR(3)", "abc");
checkCastToString("cast('abc' as varchar(6))", "CHAR(3)", "abc");
+ checkCastToString("cast(' abc ' as varchar(10))", null, " abc ");
+ checkCastToString("cast(cast('abc' as char(4)) as varchar(6))", null,
+ "abc ");
+ tester.checkString("cast(cast('a' as char(2)) as varchar(3)) || 'x' ",
+ "a x", "VARCHAR(4) NOT NULL");
+ tester.checkString("cast(cast('a' as char(3)) as varchar(5)) || 'x' ",
+ "a x", "VARCHAR(6) NOT NULL");
+ tester.checkString("cast('a' as char(3)) || 'x'", "a x",
+ "CHAR(4) NOT NULL");
+
+ tester.checkScalar("char_length(cast(' x ' as char(4)))", 4,
+ "INTEGER NOT NULL");
+ tester.checkScalar("char_length(cast(' x ' as varchar(3)))", 3,
+ "INTEGER NOT NULL");
+ tester.checkScalar("char_length(cast(' x ' as varchar(4)))", 3,
+ "INTEGER NOT NULL");
+ tester.checkScalar("char_length(cast(cast(' x ' as char(4)) as varchar(5)))",
+ 4, "INTEGER NOT NULL");
+ tester.checkScalar("char_length(cast(' x ' as varchar(3)))", 3,
+ "INTEGER NOT NULL");
// date & time
checkCastToString("date '2008-01-01'", "CHAR(10)", "2008-01-01");
@@ -579,10 +601,12 @@ public abstract class SqlOperatorBaseTest {
checkCastToString(
"interval '60' day",
"CHAR(8)",
- "+60");
+ "+60 ");
// boolean
checkCastToString("True", "CHAR(4)", "TRUE");
+ checkCastToString("True", "CHAR(6)", "TRUE ");
+ checkCastToString("True", "VARCHAR(6)", "TRUE");
checkCastToString("False", "CHAR(5)", "FALSE");
tester.checkFails(
"cast(true as char(3))", INVALID_CHAR_MESSAGE,
@@ -1969,6 +1993,18 @@ public abstract class SqlOperatorBaseTest {
Boolean.TRUE);
tester.checkBoolean(
"cast('a ' as varchar(30))=cast('a' as varchar(30))",
+ Boolean.FALSE);
+ tester.checkBoolean(
+ "cast(' a' as varchar(30))=cast(' a' as varchar(30))",
+ Boolean.TRUE);
+ tester.checkBoolean(
+ "cast('a ' as varchar(15))=cast('a ' as varchar(30))",
+ Boolean.TRUE);
+ tester.checkBoolean(
+ "cast(' ' as varchar(3))=cast(' ' as varchar(2))",
+ Boolean.TRUE);
+ tester.checkBoolean(
+ "cast('abcd' as varchar(2))='ab'",
Boolean.TRUE);
tester.checkBoolean(
"cast('a' as varchar(30))=cast('b' as varchar(30))",
http://git-wip-us.apache.org/repos/asf/calcite/blob/dd51b537/core/src/test/java/org/apache/calcite/test/JdbcTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
index 21c8ce1..eeac5d2 100644
--- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
+++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
@@ -755,7 +755,7 @@ public class JdbcTest {
// The call to "View('(10), (2)')" expands to 'values (1), (3), (10), (20)'.
assertThat(CalciteAssert.toString(resultSet),
equalTo("N={'a'=1, 'baz'=2}\n"
- + "N=[3, 4, null]\n"));
+ + "N=[3, 4, null] \n"));
connection.close();
}
http://git-wip-us.apache.org/repos/asf/calcite/blob/dd51b537/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
----------------------------------------------------------------------
diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
index 779b1fd..20c5c0f 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -930,7 +930,7 @@ LogicalProject(U=[$0], S=[$1])
</Resource>
<Resource name="planAfter">
<![CDATA[
-LogicalCalc(expr#0=[{inputs}], expr#1=['TABLE'], expr#2=['t'], U=[$t1], S=[$t2])
+LogicalCalc(expr#0=[{inputs}], expr#1=['TABLE '], expr#2=['t'], U=[$t1], S=[$t2])
LogicalValues(tuples=[[{ true }]])
]]>
</Resource>