Copilot commented on code in PR #4854:
URL: https://github.com/apache/calcite/pull/4854#discussion_r3003946082


##########
core/src/main/java/org/apache/calcite/sql/dialect/SqliteSqlDialect.java:
##########
@@ -85,16 +105,42 @@ public SqliteSqlDialect(SqlDialect.Context context) {
       RelToSqlConverterUtil.unparseTrimLR(writer, call, leftPrec, rightPrec);
       break;
     case POSITION:
-      final SqlWriter.Frame frame = writer.startFunCall("INSTR");
-      writer.sep(",");
-      call.operand(1).unparse(writer, leftPrec, rightPrec);
-      writer.sep(",");
-      call.operand(0).unparse(writer, leftPrec, rightPrec);
-      writer.endFunCall(frame);
+      switch (call.operandCount()) {
+      case 2:
+        INSTR.createCall(call.getParserPosition(), call.operand(1), 
call.operand(0))
+            .unparse(writer, leftPrec, rightPrec);
+        break;
+      case 3:
+        positionWithFromCall(call).unparse(writer, leftPrec, rightPrec);
+        break;
+      default:
+        super.unparseCall(writer, call, leftPrec, rightPrec);
+      }
       break;
     default:
       super.unparseCall(writer, call, leftPrec, rightPrec);
     }
   }
 
+  private static SqlNode positionWithFromCall(SqlCall call) {
+    final SqlParserPos pos = call.getParserPosition();
+    final SqlNode source = call.operand(1);
+    final SqlNode search = call.operand(0);
+    final SqlNode start = call.operand(2);
+    final SqlNode relativePosition =
+        INSTR.createCall(pos, SUBSTR.createCall(pos, source, start), search);
+    final SqlNode startAdjustment =
+        SqlStdOperatorTable.MINUS.createCall(pos,
+            SqlNode.clone(start),
+            SqlLiteral.createExactNumeric("1", pos));
+    final SqlNode whenCondition =
+        SqlStdOperatorTable.GREATER_THAN.createCall(pos,
+            SqlNode.clone(relativePosition),
+            SqlLiteral.createExactNumeric("0", pos));
+    final SqlNode thenResult =
+        SqlStdOperatorTable.PLUS.createCall(pos, relativePosition, 
startAdjustment);
+    return new SqlCase(pos, null, SqlNodeList.of(whenCondition),
+        SqlNodeList.of(thenResult), SqlLiteral.createExactNumeric("0", pos));

Review Comment:
   In the 3-arg POSITION rewrite, the CASE expression uses `ELSE 0`. If any 
operand is NULL, `INSTR(...)` (and therefore `relativePosition`) becomes NULL; 
the `WHEN relativePosition > 0` condition then evaluates to NULL and the CASE 
falls through to `ELSE 0`, changing the result from NULL to 0. To preserve 
POSITION/INSTR null-propagation semantics, make the ELSE branch return 
`relativePosition` (so it yields 0 on “not found” and NULL on NULL inputs), or 
add an explicit `WHEN relativePosition IS NULL THEN NULL` branch.
   ```suggestion
           SqlNodeList.of(thenResult), SqlNode.clone(relativePosition));
   ```



-- 
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]

Reply via email to