kgyrtkirk commented on code in PR #16279:
URL: https://github.com/apache/druid/pull/16279#discussion_r1567455274
##########
sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/SubstringOperatorConversion.java:
##########
@@ -45,7 +46,7 @@ public class SubstringOperatorConversion implements
SqlOperatorConversion
.operatorBuilder("SUBSTRING")
.operandTypes(SqlTypeFamily.CHARACTER, SqlTypeFamily.INTEGER,
SqlTypeFamily.INTEGER)
.functionCategory(SqlFunctionCategory.STRING)
- .returnTypeInference(ReturnTypes.ARG0)
+
.returnTypeInference(ReturnTypes.ARG0.andThen(SqlTypeTransforms.FORCE_NULLABLE))
Review Comment:
afaik these are appied during calcite planning ; I see some tests in
`ExpressionTest` ; aren't those only excercise the native layer?
##########
processing/src/main/java/org/apache/druid/math/expr/Function.java:
##########
@@ -3634,7 +3634,7 @@ ExprEval doApply(ExprEval arrayExpr, ExprEval scalarExpr)
final Object[] array = arrayExpr.asArray();
final int position = scalarExpr.asInt() - 1;
- if (array.length > position) {
+ if (array.length > position && position >= 0) {
Review Comment:
is this change was coverd with a test? if this is a real issue - I've
noticed the same mistake in `ArrayOffsetFunction` :)
##########
sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java:
##########
@@ -270,6 +270,16 @@ public void testSubstring()
makeExpression("substring(\"spacey\", (\"p\" - 1), \"p\")"),
"hey"
);
+ testHelper.testExpressionString(
Review Comment:
I've unpatched all non-test changes - and these cases still pass...did I
miss 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: commits-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
