walterddr commented on code in PR #11048:
URL: https://github.com/apache/pinot/pull/11048#discussion_r1257379055
##########
pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java:
##########
@@ -76,8 +89,13 @@ public enum TransformFunctionType {
CAST("cast"),
// string functions
- JSONEXTRACTSCALAR("jsonExtractScalar"),
- JSONEXTRACTKEY("jsonExtractKey"),
+ JSONEXTRACTSCALAR("jsonExtractScalar", ReturnTypes.cascade(
+ opBinding -> inferJsonExtractScalarExplicitTypeSpec(opBinding).orElse(
+ opBinding.getTypeFactory().createSqlType(SqlTypeName.VARCHAR,
2000)), SqlTypeTransforms.FORCE_NULLABLE),
Review Comment:
we dont need to limit varchar to 2000 here
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -187,7 +187,7 @@ public static Object jsonExtractScalar(String
jsonFieldName, String jsonPath, St
}
@ScalarFunction(names = {"jsonExtractKey", "json_extract_key"},
isPlaceholder = true)
- public static String jsonExtractKey(String jsonFieldName, String jsonPath)
{
+ public static Object jsonExtractKey(String jsonFieldName, String jsonPath)
{
Review Comment:
JsonExtractKey always returns String right? a JSON key cannot be any other
type?
##########
pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java:
##########
@@ -180,4 +210,46 @@ public String getName() {
public List<String> getAliases() {
return _aliases;
}
+
+ public SqlReturnTypeInference getSqlReturnTypeInference() {
+ return _sqlReturnTypeInference;
+ }
+
+ public SqlOperandTypeChecker getSqlOperandTypeChecker() {
+ return _sqlOperandTypeChecker;
+ }
+
+ /** Returns the optional explicit returning type specification. */
+ private static Optional<RelDataType>
inferJsonExtractScalarExplicitTypeSpec(SqlOperatorBinding opBinding) {
+ if (opBinding.getOperandCount() > 2
+ && opBinding.isOperandLiteral(2, false)) {
+ String operandType = opBinding.getOperandLiteralValue(2,
String.class).toUpperCase();
+ return Optional.of(inferExplicitTypeSpec(operandType,
opBinding.getTypeFactory()));
+ }
+ return Optional.empty();
+ }
+
+ private static RelDataType inferExplicitTypeSpec(String operandType,
RelDataTypeFactory typeFactory) {
+ switch (operandType) {
+ case "INT":
+ case "INTEGER":
Review Comment:
INTEGER is valid SQL type name
##########
pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java:
##########
@@ -180,4 +210,46 @@ public String getName() {
public List<String> getAliases() {
return _aliases;
}
+
+ public SqlReturnTypeInference getSqlReturnTypeInference() {
+ return _sqlReturnTypeInference;
+ }
+
+ public SqlOperandTypeChecker getSqlOperandTypeChecker() {
+ return _sqlOperandTypeChecker;
+ }
+
+ /** Returns the optional explicit returning type specification. */
+ private static Optional<RelDataType>
inferJsonExtractScalarExplicitTypeSpec(SqlOperatorBinding opBinding) {
+ if (opBinding.getOperandCount() > 2
+ && opBinding.isOperandLiteral(2, false)) {
+ String operandType = opBinding.getOperandLiteralValue(2,
String.class).toUpperCase();
+ return Optional.of(inferExplicitTypeSpec(operandType,
opBinding.getTypeFactory()));
+ }
+ return Optional.empty();
+ }
+
+ private static RelDataType inferExplicitTypeSpec(String operandType,
RelDataTypeFactory typeFactory) {
Review Comment:
```suggestion
private static RelDataType inferExplicitTypeSpec(String operandTypeStr,
RelDataTypeFactory typeFactory) {
```
##########
pinot-query-planner/src/main/java/org/apache/calcite/sql/fun/PinotOperatorTable.java:
##########
@@ -248,4 +316,16 @@ private boolean isPinotAggregationFunction(String name) {
}
return aggFunctionType != null &&
!aggFunctionType.isNativeCalciteAggregationFunctionType();
}
+
+ private boolean isPinotTransformFunction(String name) {
+ TransformFunctionType transformFunctionType = null;
+ if (isTransformFunctionRegisteredWithOperatorTable(name)) {
+ try {
+ transformFunctionType = TransformFunctionType.valueOf(name);
+ } catch (IllegalArgumentException e) {
+ // Ignore
Review Comment:
this error shouldn't occur if the isTransform... method is correctly capture
the name
##########
pinot-query-planner/src/main/java/org/apache/calcite/sql/fun/PinotOperatorTable.java:
##########
@@ -207,6 +222,49 @@ public final void initNoDuplicate() {
}
}
+ /**
+ * This registers transform functions defined in {@link
org.apache.pinot.common.function.TransformFunctionType}
+ * which are multistage enabled.
+ */
+ private void initTransformFunctions() {
Review Comment:
see my change in https://github.com/apache/pinot/pull/11068
this method can be greatly simplified
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java:
##########
@@ -298,24 +298,32 @@ public boolean isSuperTypeOf(ColumnDataType
subTypeCandidate) {
public DataType toDataType() {
switch (this) {
case INT:
+ case INT_ARRAY:
Review Comment:
why this change? let's dont mix array support in here if we can make that
separate
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
