Skip to content

db: replace UPDATE FROM syntax for SQLite compat #8246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

vincenzopalazzo
Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo commented Apr 22, 2025

Introduced the use of UPDATE FROM syntax in SQLite queries, which is not supported in versions prior to 3.33.0.

This causes issues on systems with older SQLite versions,
as reported in issue #8231. Rewrite the query in
migrate_convert_old_channel_keyidx() to use a subquery
with IN clause instead of UPDATE FROM, ensuring compatibility with
older SQLite versions.

Fixes 68f3649d6b9fc65ece3f1dc3e2f7e6a053170ea5
Signed-off-by: Vincenzo Palazzo [email protected]

Important

25.05 FREEZE MAY 12TH: Non-bugfix PRs not ready by this date will wait for 25.08.

RC1 is scheduled on May 23rd, RC2 on May 26th, ...

The final release is on MAY 29TH.

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

  • The changelog has been updated in the relevant commit(s) according to the guidelines.
  • Tests have been added or modified to reflect the changes.
  • Documentation has been reviewed and updated as needed.
  • Related issues have been listed and linked, including any that this PR closes.

@vincenzopalazzo vincenzopalazzo force-pushed the macros/sql-syntax branch 2 times, most recently from b68afd0 to 57e267d Compare April 22, 2025 21:38
@vincenzopalazzo vincenzopalazzo marked this pull request as ready for review April 22, 2025 21:38
@ShahanaFarooqui ShahanaFarooqui added this to the v25.02.2 milestone Apr 22, 2025
@vincenzopalazzo vincenzopalazzo requested review from rustyrussell and Copilot and removed request for rustyrussell April 22, 2025 21:45
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR rewrites an UPDATE query in wallet/db.c to replace the unsupported UPDATE FROM syntax with a subquery using the IN clause for better SQLite compatibility.

  • Changed the UPDATE query in migrate_convert_old_channel_keyidx() to remove the JOIN and use a subquery
  • Adjusted conditions to try maintaining identical filtering logic

Introduced the use of UPDATE FROM syntax in SQLite queries,
which is not supported in versions prior to 3.33.0.

This causes issues on systems with older SQLite versions,
 as reported in issue ElementsProject#8231. Rewrite the query in
 migrate_convert_old_channel_keyidx() to use a subquery
 with IN clause instead of UPDATE FROM, ensuring compatibility with
 older SQLite versions.

Changelog-Fixed: db: replace UPDATE FROM syntax for SQLite compat
Fixes 68f3649
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Vincenzo Palazzo <[email protected]>
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR replaces an unsupported SQLite UPDATE FROM syntax with a subquery using the IN clause to ensure compatibility with older SQLite versions.

  • Removed the UPDATE FROM clause.
  • Introduced a subquery using the IN clause to select the appropriate key indexes.
Comments suppressed due to low confidence (1)

wallet/db.c:2046

  • [nitpick] Consider qualifying 'keyidx' with the table name (e.g., addresses.keyidx) to improve clarity and avoid any ambiguity.
" WHERE keyidx IN (SELECT shutdown_keyidx_local FROM channels"

Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack e6c203b

Thankyou!!!

@rustyrussell rustyrussell merged commit 7198fec into ElementsProject:master Apr 23, 2025
37 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docker image broken due to sqlite not supporting a query.
3 participants