Skip to content
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

plugins/sql: allow datetime functions #7467

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

JosephGoulden
Copy link
Contributor

Hi. I'm using the SQL plugin to perform analytics on my node. It would be very useful to be able to use the date/time functions so that only the required data is returned.

I've written some queries to test this change and it works as expected.

Copy link
Collaborator

@niftynei niftynei left a comment

Choose a reason for hiding this comment

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

Please remove keyword args from the functions list.

Also remove the unnecessary Merge commit 797cdc8.

plugins/sql.c Outdated Show resolved Hide resolved
@rustyrussell
Copy link
Contributor

These look safe, thanks!

Two changes, if you could:

  1. Can you add a line to the commit message so this gets mentioned in the CHANGELOG for the release? Like so:
Changelog-Changed: Plugins: `sql` now allows date and time sqlite functions.
  1. Can you edit doc/schemas/lightning-sql-template.json to include these in the list of allowed functions? That's the basis for our documentation generation.

Thanks!

@JosephGoulden
Copy link
Contributor Author

JosephGoulden commented Jul 17, 2024

CI failed so I generated contrib/msggen/msggen/schema.json and checked that in as well.

Changelog-Changed: Plugins:  now allows date and time sqlite functions.
@rustyrussell
Copy link
Contributor

CI failed so I generated contrib/msggen/msggen/schema.json and checked that in as well.

That's what CI is for :) Good catch. Changelog is a bit weird, but Release Captain will fix it up when compiling CHANGELOG.md.

Ack dc0ae31

@rustyrussell rustyrussell enabled auto-merge (rebase) July 19, 2024 03:56
@rustyrussell rustyrussell merged commit 89ede8a into ElementsProject:master Jul 19, 2024
34 of 36 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.

3 participants