Skip to content

Commit ab46f40

Browse files
committed
Fix post-merge compilation and test failures
- Add OPTIONAL_ANY to PPLOperandTypes for BaseConversionUDF - Revert registerExternalOperator to HEAD's simpler version (origin/main used PPLTypeChecker/CalciteFuncSignature not available on this branch) - Remove duplicate TOSTRING registerOperator (conflicts with custom register) - Update streamstats and timechart test expectations for sort/hint changes Signed-off-by: Yuanchun Shen <yuanchu@amazon.com>
1 parent 2070792 commit ab46f40

4 files changed

Lines changed: 13 additions & 15 deletions

File tree

core/src/main/java/org/opensearch/sql/calcite/utils/PPLOperandTypes.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ public class PPLOperandTypes {
2828
private PPLOperandTypes() {}
2929

3030
public static final UDFOperandMetadata NONE = UDFOperandMetadata.wrap(OperandTypes.family());
31+
public static final UDFOperandMetadata OPTIONAL_ANY =
32+
UDFOperandMetadata.wrap(
33+
(CompositeOperandTypeChecker)
34+
OperandTypes.family(SqlTypeFamily.ANY).or(OperandTypes.family()));
3135

3236
public static final UDFOperandMetadata OPTIONAL_INTEGER =
3337
UDFOperandMetadata.wrap(OperandTypes.INTEGER.or(OperandTypes.family()));

core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -401,16 +401,11 @@ private PPLFuncImpTable(Builder builder, AggBuilder aggBuilder) {
401401
* @param operator a SqlOperator representing an externally implemented function
402402
*/
403403
public void registerExternalOperator(BuiltinFunctionName functionName, SqlOperator operator) {
404-
PPLTypeChecker typeChecker =
405-
wrapSqlOperandTypeChecker(
406-
operator.getOperandTypeChecker(),
407-
functionName.name(),
408-
operator instanceof SqlUserDefinedFunction);
409-
CalciteFuncSignature signature = new CalciteFuncSignature(functionName.getName(), typeChecker);
410-
externalFunctionRegistry.put(
411-
functionName,
412-
List.of(
413-
Pair.of(signature, (FunctionImp) (builder, args) -> builder.makeCall(operator, args))));
404+
if (externalFunctionRegistry.containsKey(functionName)) {
405+
logger.warn(
406+
String.format(Locale.ROOT, "Function %s is registered multiple times", functionName));
407+
}
408+
externalFunctionRegistry.put(functionName, (builder, args) -> builder.makeCall(operator, args));
414409
}
415410

416411
/**
@@ -760,7 +755,6 @@ void populate() {
760755

761756
registerOperator(INTERNAL_PATTERN_PARSER, PPLBuiltinOperators.PATTERN_PARSER);
762757
registerOperator(TONUMBER, PPLBuiltinOperators.TONUMBER);
763-
registerOperator(TOSTRING, PPLBuiltinOperators.TOSTRING);
764758

765759
// Register PPL Convert command functions
766760
registerOperator(AUTO, PPLBuiltinOperators.AUTO);

ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLStreamstatsTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,15 +237,15 @@ public void testStreamstatsWithReverse() {
237237
+ " LogicalSort(sort0=[$8], dir0=[DESC])\n"
238238
+ " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],"
239239
+ " SAL=[$5], COMM=[$6], DEPTNO=[$7], __stream_seq__=[$8], max(SAL)=[MAX($5) OVER"
240-
+ " (PARTITION BY $7 ROWS UNBOUNDED PRECEDING)])\n"
240+
+ " (PARTITION BY $7 ORDER BY $0 NULLS LAST ROWS UNBOUNDED PRECEDING)])\n"
241241
+ " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],"
242242
+ " SAL=[$5], COMM=[$6], DEPTNO=[$7], __stream_seq__=[ROW_NUMBER() OVER ()])\n"
243243
+ " LogicalTableScan(table=[[scott, EMP]])\n";
244244
verifyLogical(root, expectedLogical);
245245

246246
String expectedSparkSql =
247247
"SELECT `EMPNO`, `ENAME`, `JOB`, `MGR`, `HIREDATE`, `SAL`, `COMM`, `DEPTNO`,"
248-
+ " MAX(`SAL`) OVER (PARTITION BY `DEPTNO` ROWS BETWEEN UNBOUNDED"
248+
+ " MAX(`SAL`) OVER (PARTITION BY `DEPTNO` ORDER BY `EMPNO` NULLS LAST ROWS BETWEEN UNBOUNDED"
249249
+ " PRECEDING AND CURRENT ROW) `max(SAL)`\n"
250250
+ "FROM (SELECT `EMPNO`, `ENAME`, `JOB`, `MGR`, `HIREDATE`, `SAL`, `COMM`, `DEPTNO`,"
251251
+ " ROW_NUMBER() OVER () `__stream_seq__`\n"

ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ public void testTimechartWithReverse() {
396396
verifyLogical(root, expectedLogical);
397397

398398
String expectedSparkSql =
399-
"SELECT SPAN(`@timestamp`, 1, 'm') `@timestamp`, COUNT(*) `count()`\n"
399+
"SELECT\n/*+ `AGG_ARGS`(`ignoreNullBucket` = 'true') */\nSPAN(`@timestamp`, 1, 'm') `@timestamp`, COUNT(*) `count()`\n"
400400
+ "FROM `scott`.`events`\n"
401401
+ "WHERE `@timestamp` IS NOT NULL\n"
402402
+ "GROUP BY SPAN(`@timestamp`, 1, 'm')\n"
@@ -422,7 +422,7 @@ public void testTimechartWithCustomTimefieldAndReverse() {
422422
verifyLogical(root, expectedLogical);
423423

424424
String expectedSparkSql =
425-
"SELECT SPAN(`created_at`, 1, 'M') `created_at`, COUNT(*) `count()`\n"
425+
"SELECT\n/*+ `AGG_ARGS`(`ignoreNullBucket` = 'true') */\nSPAN(`created_at`, 1, 'M') `created_at`, COUNT(*) `count()`\n"
426426
+ "FROM `scott`.`events`\n"
427427
+ "WHERE `created_at` IS NOT NULL\n"
428428
+ "GROUP BY SPAN(`created_at`, 1, 'M')\n"

0 commit comments

Comments
 (0)