You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "macroguo-ghy (via GitHub)" <gi...@apache.org> on 2026/03/27 17:13:08 UTC
[PR] [CALCITE-7316] The POSITION function in SQLite is missing the FROM clause [calcite]
macroguo-ghy opened a new pull request, #4854:
URL: https://github.com/apache/calcite/pull/4854
## Jira Link
[CALCITE-7316](https://issues.apache.org/jira/browse/CALCITE-7316)
## Changes Proposed
This PR fixes SQLite SQL generation for the 3-argument `POSITION` form so that the `FROM` start position is preserved instead of being dropped.
Reproduction:
```sql
select position(''C'' IN ''ABCABC'' FROM 4) from "product";
```
Before this change, the generated SQLite SQL lost the `FROM 4` semantics.
This change rewrites the SQLite form to an equivalent expression based on `SUBSTR` and `INSTR`, and adds SQLite rel-to-sql tests covering both a hit and a miss case for `POSITION(... FROM ...)`.
## Testing
- Ran `./gradlew :core:test --tests "org.apache.calcite.rel.rel2sql.RelToSqlConverterTest.testPositionFunctionForSqlite"`
- Ran `./gradlew :core:test --tests "org.apache.calcite.rel.rel2sql.RelToSqlConverterTest.testPosition*"`
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7316] The POSITION function in SQLite is missing the FROM clause [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4854:
URL: https://github.com/apache/calcite/pull/4854#discussion_r3004351584
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -3692,11 +3692,30 @@ private SqlDialect nonOrdinalDialect() {
sql(query1).withClickHouse().ok(expected1);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-7316">[CALCITE-7316]
+ * The POSITION function in SQLite is missing the FROM clause</a>. */
@Test void testPositionFunctionForSqlite() {
final String query = "select position('A' IN 'ABC') from \"product\"";
final String expected = "SELECT INSTR('ABC', 'A')\n"
+ "FROM \"foodmart\".\"product\"";
sql(query).withSQLite().ok(expected);
+
+ final String query1 = "select position('C' IN 'ABCABC' FROM 4) from \"product\"";
+ final String expected1 =
Review Comment:
I’m not sure if we need to make such complex rewrites. Honestly, I haven’t even figured out how to approach the rewrite in the Jira issue.
##########
core/src/main/java/org/apache/calcite/sql/dialect/SqliteSqlDialect.java:
##########
@@ -32,6 +41,17 @@
* A <code>SqliteSqlDialect</code> implementation for the SQLite database.
*/
public class SqliteSqlDialect extends SqlDialect {
+ // Use plain function nodes here so the SQLite POSITION rewrite does not
+ // re-enter dialect-specific operator handling during unparsing.
+ private static final SqlFunction INSTR =
Review Comment:
Why do we need to add additional function definitions in the dialect?
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7316] The POSITION function in SQLite is missing the FROM clause [calcite]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7316] The POSITION function in SQLite is missing the FROM clause [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #4854:
URL: https://github.com/apache/calcite/pull/4854#discussion_r3004552293
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -3692,11 +3692,30 @@ private SqlDialect nonOrdinalDialect() {
sql(query1).withClickHouse().ok(expected1);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-7316">[CALCITE-7316]
+ * The POSITION function in SQLite is missing the FROM clause</a>. */
@Test void testPositionFunctionForSqlite() {
final String query = "select position('A' IN 'ABC') from \"product\"";
final String expected = "SELECT INSTR('ABC', 'A')\n"
+ "FROM \"foodmart\".\"product\"";
sql(query).withSQLite().ok(expected);
+
+ final String query1 = "select position('C' IN 'ABCABC' FROM 4) from \"product\"";
+ final String expected1 =
Review Comment:
+1. Additionally, there is another issue: for the query `select * from db.table where d=position(a IN b FROM c)`, if this expression is placed within the `WHERE` clause, we still need to enclose it in parentheses.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7316] The POSITION function in SQLite is missing the FROM clause [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4854:
URL: https://github.com/apache/calcite/pull/4854#issuecomment-4144255963
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4854) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4854&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4854&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4854&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [94.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4854&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4854&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4854)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7316] The POSITION function in SQLite is missing the FROM clause [calcite]
Posted by "macroguo-ghy (via GitHub)" <gi...@apache.org>.
macroguo-ghy merged PR #4854:
URL: https://github.com/apache/calcite/pull/4854
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7316] The POSITION function in SQLite is missing the FROM clause [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4854:
URL: https://github.com/apache/calcite/pull/4854#issuecomment-4151761969
@macroguo-ghy Please remember to update the Jira information. Thanks!
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7316] The POSITION function in SQLite is missing the FROM clause [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4854:
URL: https://github.com/apache/calcite/pull/4854#issuecomment-4147445446
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4854) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4854&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4854&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4854&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [93.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4854&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4854&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4854)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7316] The POSITION function in SQLite is missing the FROM clause [calcite]
Posted by "macroguo-ghy (via GitHub)" <gi...@apache.org>.
macroguo-ghy commented on code in PR #4854:
URL: https://github.com/apache/calcite/pull/4854#discussion_r3004917573
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -3692,11 +3692,30 @@ private SqlDialect nonOrdinalDialect() {
sql(query1).withClickHouse().ok(expected1);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-7316">[CALCITE-7316]
+ * The POSITION function in SQLite is missing the FROM clause</a>. */
@Test void testPositionFunctionForSqlite() {
final String query = "select position('A' IN 'ABC') from \"product\"";
final String expected = "SELECT INSTR('ABC', 'A')\n"
+ "FROM \"foodmart\".\"product\"";
sql(query).withSQLite().ok(expected);
+
+ final String query1 = "select position('C' IN 'ABCABC' FROM 4) from \"product\"";
+ final String expected1 =
Review Comment:
I think implementing a complex SQL is better than rejecting translations. And if we consider the case when statement as a whole, we can also assume that it is not a complex query
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -3692,11 +3692,30 @@ private SqlDialect nonOrdinalDialect() {
sql(query1).withClickHouse().ok(expected1);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-7316">[CALCITE-7316]
+ * The POSITION function in SQLite is missing the FROM clause</a>. */
@Test void testPositionFunctionForSqlite() {
final String query = "select position('A' IN 'ABC') from \"product\"";
final String expected = "SELECT INSTR('ABC', 'A')\n"
+ "FROM \"foodmart\".\"product\"";
sql(query).withSQLite().ok(expected);
+
+ final String query1 = "select position('C' IN 'ABCABC' FROM 4) from \"product\"";
+ final String expected1 =
Review Comment:
I add a new case to test the `position` in the `where` statement, and the output SQL is valid. I don't think it needs parentheses
##########
core/src/main/java/org/apache/calcite/sql/dialect/SqliteSqlDialect.java:
##########
@@ -32,6 +41,17 @@
* A <code>SqliteSqlDialect</code> implementation for the SQLite database.
*/
public class SqliteSqlDialect extends SqlDialect {
+ // Use plain function nodes here so the SQLite POSITION rewrite does not
+ // re-enter dialect-specific operator handling during unparsing.
+ private static final SqlFunction INSTR =
Review Comment:
It is a helper variable used for unparsed SQL. I originally intended to use `ReloSqlConverterUtil.specificalOperatorByName`, but due to its low precedence, it would produce extra parentheses. So, I have chosen the current solution.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org