[CALCITE-6042] Add test cases for ARRAY-related functions by using spark array function
https://issues.apache.org/jira/browse/CALCITE-6042
Add test cases by using spark array function rather than calcite array constructor. these cases only work in spark library.
I have wrote a array-expr-generator, it looks like:
the usage:
// array_append(array(1), 2) & array_append(array[1], 2)
f.checkScalarArray(ArrayType.ALL, "[1, 2]", "INTEGER NOT NULL ARRAY NOT NULL",
"array_append(%A%0, %1)", "1", "2");
// array_append(array(), 1)
f.checkScalarArray(ArrayType.FUNCTION, "[1]", "INTEGER NOT NULL ARRAY NOT NULL",
"array_append(%A%0, %1)", "", "1");
// array_append(array(array(1,2)), array(3,4)) & array_append(array[array[1,2]], array[3,4])
// -> It's a nested array case.
f.checkScalarArray(ArrayType.ALL, "[[1, 2], [3, 4]]",
"INTEGER NOT NULL ARRAY NOT NULL ARRAY NOT NULL",
"array_append(%A1%0, %A%1)", "1, 2", "3, 4");
the implemented method:
default void checkScalarArray(
ArrayType syntaxType,
String result,
String resultType,
String exprPattern,
String... exprArgs) {
if (syntaxType == ArrayType.ALL) {
String constructorExpr = generateArrayExpr(true, exprPattern, exprArgs);
String functionExpr = generateArrayExpr(false, exprPattern, exprArgs);
checkScalar(constructorExpr, result, resultType);
checkScalar(functionExpr, result, resultType);
} else {
boolean useConstructor = syntaxType == ArrayType.CONSTRUCTOR;
String arrayExpr = generateArrayExpr(useConstructor, exprPattern, exprArgs);
checkScalar(arrayExpr, result, resultType);
}
}
generateArrayExpr generate array() or array[] with given syntaxType, and it supports nested array.
-
exprPattern %A[n]%i placeholders are used for array syntax, where
nis the optional nested level andiis the index of the argument to be inserted. %i placeholders without %A will be replaced without array syntax. -
exprArgs Arguments to replace the placeholders in the expression pattern. They will replace %0, %1, etc.
I feel it will be less readable, but it does remove duplicate code. I want to know what everyone thinks of this plan? If possible, I will update all array cases to this form.
@mihaibudiu what do you think?
There is actually an even better way to do it, I should have thought about it sooner. The idea is to use Calcite itself to rewrite the code. The way this is done is by implementing a SqlShuttle which replaces calls to one of the operators with calls to the other one. Then for each test you:
- run the test as is
- parse the Sql, run the shuttle, unparse the sql
- run the resulting sql
Let me know if you need help implementing this solution.
thanks for suggestions. @mihaibudiu
Frankly speaking, I'm not sure if this is the best solution, it may be over-optimized, and the duplication in the PR may just be sonar's strategy (since I handle all array functions)
Try to see if others have other opinions, I might be leaning towards good readability.
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.







