[
https://issues.apache.org/jira/browse/CALCITE-4347?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17270964#comment-17270964
]
Louis Kuang edited comment on CALCITE-4347 at 1/24/21, 7:42 PM:
----------------------------------------------------------------
Thanks for the detailed feedbacks!
{quote}but I suspect that there are several other functions that are similar. I
wonder whether it is possible for OperandTypeChecker to notice that all
overloads give the same type to operand i and therefore deduce the type of
operand i. By which I mean, can we make OperandTypeChecker implement
OperandTypeInference in some cases?
{quote}
You are right, there are at least a few functions that are in similar
situation: POSITION and OVERLAY. Their signatures are a bit different though
{noformat}
POSITION(STRING, STRING)
POSITION(STRING, STRING, INTEGER)
POSITION(BINARY, BINARY)
POSITION(BINARY, BINARY, INTEGER)
OVERLAY(STRING, STRING, INTEGER)
OVERLAY(STRING, STRING, INTEGER, INTEGER)
OVERLAY(BINARY, BINARY, INTEGER)
OVERLAY(BINARY, BINARY, INTEGER, INTEGER){noformat}
Not sure if I understand what you mean exactly but here is my interpretation:
{code:java}
public static final SqlOperandTypeChecker
BINARY_OR_STRING_INTEGER_OPTIONAL_INTEGER = new CompositeOperandTypeChecker(
CompositeOperandTypeChecker.Composition.SEQUENCE,
ImmutableList.of(
OperandTypes.or(BINARY, STRING),
family(ImmutableList.of(SqlTypeFamily.INTEGER), i -> i == 1)),
null,
null
) {
@Override public @Nullable SqlOperandTypeInference typeInference() {
return (callBinding, returnType, operandTypes) -> {
operandTypes[0] = returnType;
RelDataTypeFactory typeFactory = callBinding.getTypeFactory();
operandTypes[1] = typeFactory.createSqlType(SqlTypeName.INTEGER);
if (operandTypes.length == 3) {
operandTypes[2] = typeFactory.createSqlType(SqlTypeName.INTEGER);
}
};
}
};
{code}
This way the type inference can be obtained from the type checker. However,
this does not do as much as the checker at
SqlSubStringFunction#checkOperandTypes, which allow implicit type cast, e.g.,
from STRING to INTEGER. Because of this, I think for this JIRA making the
modification inplace as you suggest `inferOperandTypes(SqlCallBinding
callBinding, RelDataType returnType, RelDataType[] operandTypes)` will be
better.
{quote}Your test should check the inferred types of the parameters, not just
call .ok()
{quote}
Is there an example test that does this that I can reference? The `Sql` fluent
testing API does not seem to expose a convenient API to do this.
{quote}But you should test "WHERE deptno = SUBSTRING(? FROM 2 FOR 4)" and make
sure that it gives a useful error
{quote}
With `sql("select * from emp where deptno = \^substring(? from 0)\^")`, the
above implementation would yield
{noformat}
org.apache.calcite.sql.validate.SqlValidatorException: Cannot apply 'SUBSTRING'
to arguments of type 'SUBSTRING(<INTEGER> FROM <INTEGER>)'. Supported form(s):
'SUBSTRING(<CHAR> FROM <INTEGER>)'
'SUBSTRING(<CHAR> FROM <INTEGER> FOR <INTEGER>)'
'SUBSTRING(<VARCHAR> FROM <INTEGER>)'
'SUBSTRING(<VARCHAR> FROM <INTEGER> FOR <INTEGER>)'
'SUBSTRING(<BINARY> FROM <INTEGER>)'
'SUBSTRING(<BINARY> FROM <INTEGER> FOR <INTEGER>)'
'SUBSTRING(<VARBINARY> FROM <INTEGER>)'
'SUBSTRING(<VARBINARY> FROM <INTEGER> FOR <INTEGER>)'{noformat}
The first argument in the signature has been silently swapped to `INTEGER`
because of the return type deduction. The error message is not very clear to
me. Is there a good way to supply the extra context to the validator exception
that a deduction of the type of `?` based on the return type has been applied?
or do you think this is good enough for an error message.
was (Author: lllouiskuang):
Thanks for the detailed feedbacks!
{quote}but I suspect that there are several other functions that are similar. I
wonder whether it is possible for OperandTypeChecker to notice that all
overloads give the same type to operand i and therefore deduce the type of
operand i. By which I mean, can we make OperandTypeChecker implement
OperandTypeInference in some cases?
{quote}
You are right, there are at least a few functions that are in similar
situation: POSITION and OVERLAY. Their signatures are a bit different though
{noformat}
POSITION(STRING, STRING)
POSITION(STRING, STRING, INTEGER)
POSITION(BINARY, BINARY)
POSITION(BINARY, BINARY, INTEGER)
OVERLAY(STRING, STRING, INTEGER)
OVERLAY(STRING, STRING, INTEGER, INTEGER)
OVERLAY(BINARY, BINARY, INTEGER)
OVERLAY(BINARY, BINARY, INTEGER, INTEGER){noformat}
Not sure if I understand what you mean exactly but here is my interpretation:
{code:java}
public static final SqlOperandTypeChecker
BINARY_OR_STRING_INTEGER_OPTIONAL_INTEGER = new CompositeOperandTypeChecker(
CompositeOperandTypeChecker.Composition.SEQUENCE,
ImmutableList.of(
OperandTypes.or(BINARY, STRING),
family(ImmutableList.of(SqlTypeFamily.INTEGER), i -> i == 1)),
null,
null
) {
@Override public @Nullable SqlOperandTypeInference typeInference() {
return (callBinding, returnType, operandTypes) -> {
operandTypes[0] = returnType;
RelDataTypeFactory typeFactory = callBinding.getTypeFactory();
operandTypes[1] = typeFactory.createSqlType(SqlTypeName.INTEGER);
if (operandTypes.length == 3) {
operandTypes[2] = typeFactory.createSqlType(SqlTypeName.INTEGER);
}
};
}
};
{code}
This way the type inference can be obtained from the type checker. However,
this does not do as much as the checker at
SqlSubStringFunction#checkOperandTypes, which allow implicit type cast, e.g.,
from STRING to INTEGER. Because of this, I think for this JIRA making the
modification inplace as you suggest `inferOperandTypes(SqlCallBinding
callBinding, RelDataType returnType, RelDataType[] operandTypes)` will be
better.
{quote}Your test should check the inferred types of the parameters, not just
call .ok()
{quote}
Is there an example test that does this that I can reference? The `Sql` fluent
testing API does not seem to expose a convenient API to do this.
{quote}But you should test "WHERE deptno = SUBSTRING(? FROM 2 FOR 4)" and make
sure that it gives a useful error
{quote}
Regarding the error that is given from `sql("select * from emp where deptno =
^substring(? from 0)^")`, the above implementation would yield
{noformat}
org.apache.calcite.sql.validate.SqlValidatorException: Cannot apply 'SUBSTRING'
to arguments of type 'SUBSTRING(<INTEGER> FROM <INTEGER>)'. Supported form(s):
'SUBSTRING(<CHAR> FROM <INTEGER>)'
'SUBSTRING(<CHAR> FROM <INTEGER> FOR <INTEGER>)'
'SUBSTRING(<VARCHAR> FROM <INTEGER>)'
'SUBSTRING(<VARCHAR> FROM <INTEGER> FOR <INTEGER>)'
'SUBSTRING(<BINARY> FROM <INTEGER>)'
'SUBSTRING(<BINARY> FROM <INTEGER> FOR <INTEGER>)'
'SUBSTRING(<VARBINARY> FROM <INTEGER>)'
'SUBSTRING(<VARBINARY> FROM <INTEGER> FOR <INTEGER>)'{noformat}
The first argument in the signature has been silently swapped to `INTEGER`
because of the return type deduction. The error message is not very clear to
me. Is there a good way to supply the extra context to the validator exception
that a deduction of the type of `?` based on the return type has been applied?
or do you think this is good enough for an error message.
> Illegal use of dynamic parameter when use preparestatement
> ----------------------------------------------------------
>
> Key: CALCITE-4347
> URL: https://issues.apache.org/jira/browse/CALCITE-4347
> Project: Calcite
> Issue Type: Bug
> Components: core
> Reporter: yangzhang
> Priority: Major
> Labels: pull-request-available
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> hi , when query with preparestatement, validator throws exception.
> here is 2 sqls:
> 1、select LSTG_FORMAT_NAME
> from kylin_sales
> where LSTG_FORMAT_NAME=substring(?,1,6)
>
> 2、select LSTG_FORMAT_NAME
> from kylin_sales
> where LSTG_FORMAT_NAME=substring(?,?,6)
>
> trace is:
> Caused by: org.apache.calcite.runtime.CalciteContextException: At line 4,
> column 34: Illegal use of dynamic parameter
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
> at
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
> at org.apache.calcite.runtime.Resources$ExInstWithCause.ex(Resources.java:463)
> at org.apache.calcite.sql.SqlUtil.newContextException(SqlUtil.java:803)
> at org.apache.calcite.sql.SqlUtil.newContextException(SqlUtil.java:788)
> at
> org.apache.calcite.sql.validate.SqlValidatorImpl.newValidationError(SqlValidatorImpl.java:4708)
> at
> org.apache.calcite.sql.validate.SqlValidatorImpl.inferUnknownTypes(SqlValidatorImpl.java:1710)
> at
> org.apache.calcite.sql.validate.SqlValidatorImpl.inferUnknownTypes(SqlValidatorImpl.java:1785)
> at
> org.apache.calcite.sql.validate.SqlValidatorImpl.inferUnknownTypes(SqlValidatorImpl.java:1785)
> at
> org.apache.calcite.sql.validate.SqlValidatorImpl.validateWhereOrOn(SqlValidatorImpl.java:3906)
> at
> org.apache.calcite.sql.validate.SqlValidatorImpl.validateWhereClause(SqlValidatorImpl.java:3898)
> at
> org.apache.calcite.sql.validate.SqlValidatorImpl.validateSelect(SqlValidatorImpl.java:3224)
> at
> org.apache.calcite.sql.validate.SelectNamespace.validateImpl(SelectNamespace.java:60)
> at
> org.apache.calcite.sql.validate.AbstractNamespace.validate(AbstractNamespace.java:84)
> at
> org.apache.calcite.sql.validate.SqlValidatorImpl.validateNamespace(SqlValidatorImpl.java:949)
> at
> org.apache.calcite.sql.validate.SqlValidatorImpl.validateQuery(SqlValidatorImpl.java:930)
> at org.apache.calcite.sql.SqlSelect.validate(SqlSelect.java:226)
> at
> org.apache.calcite.sql.validate.SqlValidatorImpl.validateScopedExpression(SqlValidatorImpl.java:905)
> at
> org.apache.calcite.sql.validate.SqlValidatorImpl.validate(SqlValidatorImpl.java:615)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:576)
> at org.apache.calcite.prepare.Prepare.prepareSql(Prepare.java:269)
> at org.apache.calcite.prepare.Prepare.prepareSql(Prepare.java:235)
> at
> org.apache.calcite.prepare.CalcitePrepareImpl.prepare2_(CalcitePrepareImpl.java:804)
> at
> org.apache.calcite.prepare.CalcitePrepareImpl.prepare_(CalcitePrepareImpl.java:663)
> at
> org.apache.calcite.prepare.CalcitePrepareImpl.prepareSql(CalcitePrepareImpl.java:626)
> at
> org.apache.calcite.jdbc.CalciteConnectionImpl.parseQuery(CalciteConnectionImpl.java:232)
> at
> org.apache.calcite.jdbc.CalciteConnectionImpl.prepareStatement_(CalciteConnectionImpl.java:214)
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
