Fix SQL parsing for RETURNING + Correct value handling in executeSet (Issues #670 and #671)#672
Fix SQL parsing for RETURNING + Correct value handling in executeSet (Issues #670 and #671)#672xEverth wants to merge 3 commits intocapacitor-community:masterfrom
Conversation
account for INSERT statements w/ ON CONFLICT clauses with parentheses Previously the stmt for insert was being split at the last closing parenthesis, leaving unvalid statements in some circumstances. Unified the logic for INSERT, UPDATE and DELETE statements as per docs (the "returning-clause" has the same syntax for all of them). Removed extra if branch for retMode in prepareSQL (it was always taking a path whether retMode started with "no" or not) Added utils to parse Common Table Expressions Using the util to get statement type Using the same CTE in the getUpdDelReturnedValues if "returning" Added utils to parse Common Table Expressions Using the util to get statement type Using the same CTE in the getUpdDelReturnedValues if "returning"
replacePlaceholders is now redundant getUpdDelReturnedValues accepts the bindingValues
… database It previously got the last row id from any table of the DB, depending on which insert happened last Changes the function to retrieve inserted values
|
Hello dear maintainers, I just wanted to gently follow up on this draft PR, as it’s been a few months since I opened it. I completely understand how busy things can get, but I’d really appreciate any feedback whenever you have the time. Since this PR introduces changes to SQL parsing and statement preparation (including RETURNING handling, parameter binding, and ROWID retrieval), it would be especially helpful to know whether the overall direction aligns with the project’s expectations before I continue refining it. I’m very happy to adapt this however needed — whether that means splitting it into smaller PRs, adjusting the approach, or adding more tests and documentation. Even brief or high-level feedback would go a long way. Thanks again for your time and for maintaining the project! Best regards, |
This Draft Pull Request proposes fixes for multiple issues in the plugin’s SQL parsing and statement preparation logic in the Java/Android implementation.
The goal is to align behavior with SQLite grammar, ensure correctness when handling RETURNING clauses, and eliminate string-based parameter substitution that causes data corruption in multi-row inserts.
This PR is not ready for merging — it is intended to gather feedback and align on the expected behavior before finalizing the implementation. The code has been linted, formatted and verified (verified:android) using the npm scripts in the package.json. I would be glad to implement the same changes with a parallel PR on the repo https://github.com/jepiqueau/jeep-sqlite/tree/master which shares the same API, but I am currently unable to test these changes on iOS.
Fixes Included
BETWEEN a AND bwith b<a, resulting in an error.Rationale
SQLite’s grammar guarantees that RETURNING appears last, so parsing should only modify that clause.
Manual string substitution cannot safely reproduce SQLite’s parameter binding behavior, especially with complex string data.
Aligning the behavior across Android and Web improves consistency and reduces surprising behavior for end users.
Fixes #670 and fixes #671 on Android (API 26 and up). May also resolve #605 but can't say for sure without additional information from the submitter.