[POC]Support type checker For PPL functions #3562
Conversation
Signed-off-by: Heng Qian <[email protected]>
qianheng-aws
requested review from
ps48,
kavithacm,
derek-ho,
joshuali925,
dai-chen,
YANG-DB,
mengweieric,
Swiddis,
penghuo,
seankao-az,
MaxKsyunz,
Yury-Fridlyand,
anirudha,
forestmvey,
acarbonetto,
GumpacG,
ykmr1224,
LantaoJin and
noCharger
as code owners
qianheng-aws
changed the title
| import java.util.List; | ||
| import org.apache.calcite.rel.type.RelDataType; | ||
|
|
||
| /** Function signature is composed by function name and arguments list. */ | ||
| public record CalciteFuncSignature(FunctionName functionName, List<RelDataType> funcArgTypes) { | ||
| public record CalciteFuncSignature(FunctionName functionName, PPLTypeChecker typeChecker) { |
There was a problem hiding this comment.
PPLTypeChecker -> FunctionParamsTypeChecker could be more readable.
| private static boolean validateOperands( | ||
| List<SqlTypeFamily> funcTypeFamilies, List<RelDataType> operandTypes) { | ||
| if (funcTypeFamilies.size() != operandTypes.size()) { | ||
| return true; // Skip checking if sizes do not match because some arguments may be optional |
There was a problem hiding this comment.
how to handle the mismatch case with optional argument
There was a problem hiding this comment.
It won't do check for the case that has optional arguments, which has the same behavior as the type checker in Calcite. It should throw exception until execution as before.
| } | ||
| } | ||
|
|
||
| class PPLFamilyTypeCheckerWrapper implements PPLTypeChecker { |
There was a problem hiding this comment.
What is this PPLFamilyTypeCheckerWrapper used for?
There was a problem hiding this comment.
It supports build a PPLTypeChecker from a PPLFamilyTypeCheckerWrapper of Calcite. The latter one only supports SqlNode level check, and we use this wrapper to make it support RelNode level checking per our requirements
There was a problem hiding this comment.
Maybe sqlOperandTypeChecker.isOptional(int i) combined with sqlOperandTypeChecker.or(SqlOperandTypeChecker checker) can be used to check functions with multiple signatures?
| } | ||
| } | ||
|
|
||
| public interface FunctionImp2 extends FunctionImp { | ||
| List<RelDataType> ANY_TYPE_2 = List.of(ANY_TYPE, ANY_TYPE); | ||
| PPLTypeChecker IGNORE_2 = family(IGNORE, IGNORE); |
There was a problem hiding this comment.
Why does the checker name to IGNORE_N?
There was a problem hiding this comment.
In Calcite, IGNORE is a type like ANY but don't even validate the operand, while ANY accepts all types except CURSOR.
So IGNORE is a more suitable type here than ANY.
| return true; // Skip checking if sizes do not match because some arguments may be optional | ||
| } | ||
| for (int i = 0; i < operandTypes.size(); i++) { | ||
| SqlTypeName paramType = operandTypes.get(i).getSqlTypeName(); |
There was a problem hiding this comment.
relDataType.getSqlTypeName() does not handle UDT. A function like the following may help:
public static SqlTypeName convertRelDataTypeToSqlTypeName(RelDataType type) {
if (type instanceof ExprSqlType exprSqlType) {
return switch (exprSqlType.getUdt()) {
case EXPR_DATE -> SqlTypeName.DATE;
case EXPR_TIME -> SqlTypeName.TIME;
case EXPR_TIMESTAMP -> SqlTypeName.TIMESTAMP;
default -> type.getSqlTypeName();
};
}
return type.getSqlTypeName();
}
There was a problem hiding this comment.
These UDT should maps to SqlTypeName.VARCHAR actually. And we have the API to return the related SqlTypeName of our UDT here:
Description
Calcite has its type checker mechanism to do validation for its function calls. However, this process happens in SqlNode level and its API only accepts SqlNode level objects like
SqlCallBinding,SqlNodeand etc. Since we use RelNode directly, this validation step has been ignored by our current process.In order to reuse the definition of Calcite's builtin operators, we have to do validation by ourself with an approach to reuse the definition of Calcite's operators.
Luckily, in Calcite, almost all operators use
FamilyOperandTypeCheckeras their type checker with APIgetOperandSqlTypeFamilyexposed to us to make use of. We can use this API to do validation conveniently.Note
This is a draft PR with only one test cases to verify the functionality.
Before this PR, the test
CalcitePPLFunctionTypeTest::testLowerWithIntegerTypewill return a incorrect plan for its PPL:The above plan is incorrect because $0(i.e. EMPNO) has type of integer, so it shouldn't be passed into function
LOWERwhich only allowsSqlTypeFamily.CHARACTOR. The error will only be thrown until execution step.With this PR, the ppl will throw exception in building logical plan because of unsupported function.
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
Check List
--signoff.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.