tanclary commented on code in PR #3343:
URL: https://github.com/apache/calcite/pull/3343#discussion_r1280774881
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -332,6 +332,11 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding
operatorBinding,
Static.RESOURCE.delimiterIsRequired(
operatorBinding.getOperator().getName(), type.toString()));
}
+ if (SqlTypeUtil.isBinary(type)) {
+ type =
operatorBinding.getTypeFactory().createSqlType(SqlTypeName.VARBINARY);
Review Comment:
Is it simpler to say
SqlTypeName typeName = SqlTypeUtil.isBinary(type) ? SqlTypeName.VARBINARY :
SqlTypeNameVARCHAR
type = operatorBinding.getTypeFactory().createSqlType(typeName)
I only ask because the creation of the sql type gets done twice with one
slight difference. Let me know what you think.
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -5090,6 +5091,19 @@ private HepProgram getTransitiveProgram() {
.checkUnchanged();
}
+ /** Test case for <a
href="https://issues.apache.org/jira/browse/CALCITE-5882";>
+ * [CALCITE-5882] Compile-time evaluation of SPLIT function returns
incorrect result</a>. */
+ @Test public void testSplit() {
Review Comment:
Is there a reason this test is in RelOptRules? My understanding is this test
class is for optimization rules, I am not sure if a function bug test
necessarily belongs here. Maybe the operator tests would work better? Let me
know if I am missing something.
--
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]
