sql plugin #5679

rustyrussell · 2022-10-28T17:43:07Z

~~[ Rebased on #5825 which is in draft ]~~ Applied!

This plugin provides a fairly nice way of using SQL queries on our various list commands, without too much penalty (and, with great time savings if the alternative is to pull all the data remotely to query it).

It goes to great lengths to cache listnodes and listchannels.

Feedback welcome!

jb55 · 2022-10-28T22:41:05Z

whoa I needed this! testing.

jb55 · 2022-10-29T19:18:23Z

I was trying various inputs and it looks like this one breaks it:

$ lightning-cli sqlsimple '["*"]' 'from sendpays limit 5'

sql: FATAL SIGNAL 11 (version v0.12.0-327-gf17f467)
0x417ca0 send_backtrace
        common/daemon.c:33
0x417d36 crashdump
        common/daemon.c:46
0x7fee2cdb2bef ???
        ???:0
0x7fee2ced297d ???
        ???:0
0x4a0648 json_out_member_direct
        ccan/ccan/json_out/json_out.c:164
0x4a0785 json_out_addv
        ccan/ccan/json_out/json_out.c:222
0x41d39e json_add_primitive_fmt
        common/json_stream.c:158
0x41d570 json_add_u64
        common/json_stream.c:262
0x403ca5 refresh_complete
        plugins/sql.c:317
0x403fce refresh_tables
        plugins/sql.c:914
0x404fab one_refresh_done
        plugins/sql.c:395
0x405391 default_list_done
        plugins/sql.c:615
0x40a282 handle_rpc_reply
        plugins/libplugin.c:751
0x40a406 rpc_read_response_one
        plugins/libplugin.c:926
0x40a4a4 rpc_conn_read_response
        plugins/libplugin.c:950
0x49def2 next_plan
        ccan/ccan/io/io.c:59
0x49e376 do_plan
        ccan/ccan/io/io.c:407
0x49e40f io_ready
        ccan/ccan/io/io.c:417
0x49f5da io_loop
        ccan/ccan/io/poll.c:453
0x40a6de plugin_main
        plugins/libplugin.c:1774
0x4066e3 main
        plugins/sql.c:1409
0x7fee2cd9e24d ???
        ???:0
0x7fee2cd9e308 ???
        ???:0
0x4038e4 ???
        ???:0
0xffffffffffffffff ???
        ???:0

cdecker

Very interesting PR, but I am not quite convinced we want to a) make it non-experimental and b) provide any guarantees of backwards compatibility to it going forward. It is all new code, contains a number of antipatterns that need justifications for their safety (string concatenation for queries), the ergonomics seems rather dubious to me, and thus promising that it'll not change in future is strange. It also comes in very shortly before we're cutting an RC, not giving much time to figure out the edge cases.

For these reasons I'm tempted to either downgrade the guarantees by making it experimental or delay merging until after the release. What would you prefer?

common/json_parse_simple.h

cdecker · 2022-11-01T16:49:33Z

tools/flattenschema.py

@@ -0,0 +1,43 @@
+#! /usr/bin/env python3
+# Script to turn JSON schema into simple CSV describing fields.


More CSVs that aren't really CSVs?

They are separated by commas :)

Yeah, I'm tempted to rework this to include the raw json schemas instead (or maybe strip them down to just names and types, but leave them in JSON). Doing half the processing here and half at plugin init time is just awkward (esp. as the later patches make the plugin do more complex analysis).

tests/test_plugin.py

plugins/sql.c

rustyrussell · 2023-01-13T05:19:43Z

Rebased on master now all prerequisites have been merged. We now handle future deprecations correctly!

rustyrussell · 2023-01-15T05:56:54Z

Rebased on master for man page / generated rust conflicts, and fixed naked _ in new man page.

rustyrussell · 2023-01-16T03:28:27Z

Rebased on new CI system! Let's see if that's an improvement!

It's a core concept in the spec which isn't directly exposed. Signed-off-by: Rusty Russell <[email protected]> Changelog-Added: JSON-RPC: `listchannels` added a `direction` field (0 or 1) as per gossip specification.

`hash` is a tighter requirement than simply `hex`. Signed-off-by: Rusty Russell <[email protected]>

We actually had a partid allowed (in the oneOf clauses), but didn''t document it. Signed-off-by: Rusty Russell <[email protected]>

It's actually two separate u16 fields, so actually treat it as such! Cleans up zombie handling code a bit too. Signed-off-by: Rusty Russell <[email protected]>

This is useful when you're writing routines to scan it. Signed-off-by: Rusty Russell <[email protected]>

connectd is the only one who uses these routines now. The rest can be linked into a plugin. Signed-off-by: Rusty Russell <[email protected]>

We only handle top-level objects with an array of objects: make sure it is one before we call the routines. Signed-off-by: Rusty Russell <[email protected]>

I don't like it, but we do expose some times like this :( Signed-off-by: Rusty Russell <[email protected]>

If we have a specific type (not just "hex") the length is implied. Signed-off-by: Rusty Russell <[email protected]>

We have "secret" and "hash" types which are often more appropriate. Signed-off-by: Rusty Russell <[email protected]>

This is designed to allow you to perform complex server-side queries. Signed-off-by: Rusty Russell <[email protected]>

Rather than two arrays "columns" (for names) and "fieldtypes" (for types), use a struct. This makes additions easier for successive patches. Also pull process_json_obj() out of the loop in list_done(). Signed-off-by: Rusty Russell <[email protected]>

This requires us to rename "index" fields, rename fields if we have a sub-object, and create sub-tables if we have an array, and handle the fact that some listX commands don't contain array X (listsendpays contains "payments"). Signed-off-by: Rusty Russell <[email protected]>

Signed-off-by: Rusty Russell <[email protected]>

Painfully created by hand from the source. Signed-off-by: Rusty Russell <[email protected]>

Signed-off-by: Rusty Russell <[email protected]>

…sip_store. It's quite a lot of code, but these are the most expensive commands we do so it's worth it. Signed-off-by: Rusty Russell <[email protected]>

`"deprecated": true` is obsolete; we don't document them anyway. Where it would have otherwise changed the GRPC wrappers, I actually put the version number in. We allow "listchannels" to have "satoshis" since we have some tests that run in deprecated-api mode. Signed-off-by: Rusty Russell <[email protected]>

For now, we ignore every deprecated field, but put in the logic so that future deprecations will work as expected. Signed-off-by: Rusty Russell <[email protected]>

We now add tables to the strmap as we allocate them, since we don't want to call "finish_td" when we're merely invoked for the documentation, and don't need a database. Signed-off-by: Rusty Russell <[email protected]>

In particular, we generate the schema part from the plugin itself. Signed-off-by: Rusty Russell <[email protected]> Changelog-Added: Plugins: `sql` plugin command to perform server-side complex queries.

And document them! Signed-off-by: Rusty Russell <[email protected]>

@niftynei

This *would* be a 1-line change (add it to Makefile) except that we previously assumed a "list" prefix on commands. These use the default refreshing, but they could be done better using the time-range parameters. Suggested-by: @niftynei Signed-off-by: Rusty Russell <[email protected]>

Good for detection of what fields are present. Signed-off-by: Rusty Russell <[email protected]>

@ShahanaFarooqui

Prompted by @ShahanaFarooqui's playing with examples and finding common errors. Signed-off-by: Rusty Russell <[email protected]>

Also, put the "added" lines in the request schemas for new commands: this doesn't do anything (yet?) but it keeps `make schema-added-check` happy. Signed-off-by: Rusty Russell <[email protected]>

endothermicdev

The gossip store rebase and zombie update looks solid, as do the corrections.

ACK c2face4

endothermicdev · 2023-01-30T18:09:31Z

gossipd/gossip_store.c

 	u32 index = bcast->index;

+	/* We assume flags is the first field! */
+	BUILD_ASSERT(offsetof(struct gossip_hdr, flags) == 0);


I like this method of validation.

rustyrussell added plugin Optech Make Me Famous! Look! Look! Look! COOL NEW FEATURE! labels Oct 28, 2022

rustyrussell added this to the v22.11 milestone Oct 28, 2022

rustyrussell requested review from niftynei and ShahanaFarooqui October 28, 2022 17:43

rustyrussell requested a review from cdecker as a code owner October 28, 2022 17:43

rustyrussell force-pushed the guilt/sql-plugin branch from 0fddb06 to f640b59 Compare October 28, 2022 17:59

rustyrussell force-pushed the guilt/sql-plugin branch from f640b59 to 31f62e1 Compare October 29, 2022 05:05

cdecker reviewed Nov 1, 2022

View reviewed changes

rustyrussell force-pushed the guilt/sql-plugin branch 2 times, most recently from b859c0c to 5c746b2 Compare November 2, 2022 03:28

cdecker modified the milestones: v22.11, v23.02 Nov 3, 2022

rustyrussell force-pushed the guilt/sql-plugin branch from 5c746b2 to 1632cbe Compare November 28, 2022 03:05

rustyrussell force-pushed the guilt/sql-plugin branch 2 times, most recently from a01ff90 to 2ca0c5d Compare December 15, 2022 06:21

rustyrussell mentioned this pull request Jan 3, 2023

Add deprecation / addition information to schemas #5867

Merged

cdecker mentioned this pull request Jan 6, 2023

Add ability to fetch only changes for incremental sync to device Blockstream/greenlight#68

Open

rustyrussell force-pushed the guilt/sql-plugin branch from 2ca0c5d to 70df95d Compare January 13, 2023 05:18

rustyrussell requested a review from cdecker January 13, 2023 05:20

rustyrussell force-pushed the guilt/sql-plugin branch 2 times, most recently from 3dc5b46 to 2847a6d Compare January 15, 2023 05:56

rustyrussell force-pushed the guilt/sql-plugin branch 2 times, most recently from cfb6116 to f80a8e3 Compare January 16, 2023 03:28

rustyrussell force-pushed the guilt/sql-plugin branch from f80a8e3 to c6eada8 Compare January 17, 2023 04:24

rustyrussell added 27 commits January 30, 2023 16:37

plugins/topology: add direction field to listchannels.

a880b60

It's a core concept in the spec which isn't directly exposed. Signed-off-by: Rusty Russell <[email protected]> Changelog-Added: JSON-RPC: `listchannels` added a `direction` field (0 or 1) as per gossip specification.

lightningd: fix type of listhtlcs payment_hash.

90bb11d

`hash` is a tighter requirement than simply `hex`. Signed-off-by: Rusty Russell <[email protected]>

doc: fix listsendpays man page.

c74bc54

We actually had a partid allowed (in the oneOf clauses), but didn''t document it. Signed-off-by: Rusty Russell <[email protected]>

common/gossip_store: clean up header.

c826513

It's actually two separate u16 fields, so actually treat it as such! Cleans up zombie handling code a bit too. Signed-off-by: Rusty Russell <[email protected]>

common/gossip_store: expose routine to read one header.

22afc18

This is useful when you're writing routines to scan it. Signed-off-by: Rusty Russell <[email protected]>

common/gossip_store: move subdaemon-only routines to connectd.

71c033b

connectd is the only one who uses these routines now. The rest can be linked into a plugin. Signed-off-by: Rusty Russell <[email protected]>

tools/fromschema.py: don't try to handle more complex cases.

abb0564

We only handle top-level objects with an array of objects: make sure it is one before we call the routines. Signed-off-by: Rusty Russell <[email protected]>

common: add routine to get double from JSON.

4418172

I don't like it, but we do expose some times like this :( Signed-off-by: Rusty Russell <[email protected]>

doc/schemas: remove unnecessary length restrictions.

1f2f1b7

If we have a specific type (not just "hex") the length is implied. Signed-off-by: Rusty Russell <[email protected]>

doc: use specific types in schema rather than "hex".

f1efb65

We have "secret" and "hash" types which are often more appropriate. Signed-off-by: Rusty Russell <[email protected]>

plugins/sql: initial commit of new plugin.

c8a173b

This is designed to allow you to perform complex server-side queries. Signed-off-by: Rusty Russell <[email protected]>

plugins/sql: make tables for non-object arrays.

1897b3d

Signed-off-by: Rusty Russell <[email protected]>

plugins/sql: add listpeerchannels support.

1b58d37

Signed-off-by: Rusty Russell <[email protected]>

pytest: perform more thorough testing.

b0c2560

Painfully created by hand from the source. Signed-off-by: Rusty Russell <[email protected]>

plugins/sql: include the obvious indexes.

4bf9d21

Signed-off-by: Rusty Russell <[email protected]>

plugins/sql: refresh listnodes and listchannels by monitoring the gos…

0a6cd62

…sip_store. It's quite a lot of code, but these are the most expensive commands we do so it's worth it. Signed-off-by: Rusty Russell <[email protected]>

plugins/sql: pay attention to deprecated in schema.

4885ef6

For now, we ignore every deprecated field, but put in the logic so that future deprecations will work as expected. Signed-off-by: Rusty Russell <[email protected]>

plugins/sql: print out part of man page referring to schemas.

472d1b2

We now add tables to the strmap as we allocate them, since we don't want to call "finish_td" when we're merely invoked for the documentation, and don't need a database. Signed-off-by: Rusty Russell <[email protected]>

doc: document the sql command.

224383b

In particular, we generate the schema part from the plugin itself. Signed-off-by: Rusty Russell <[email protected]> Changelog-Added: Plugins: `sql` plugin command to perform server-side complex queries.

plugins/sql: allow some simple functions.

212c859

And document them! Signed-off-by: Rusty Russell <[email protected]>

plugins/sql: listsqlschemas command to retrieve schemas.

28cf990

Good for detection of what fields are present. Signed-off-by: Rusty Russell <[email protected]>

doc: add examples for sql plugin.

cf3f295

Prompted by @ShahanaFarooqui's playing with examples and finding common errors. Signed-off-by: Rusty Russell <[email protected]>

typo fixes found by @niftynei

c2face4

Also, put the "added" lines in the request schemas for new commands: this doesn't do anything (yet?) but it keeps `make schema-added-check` happy. Signed-off-by: Rusty Russell <[email protected]>

rustyrussell force-pushed the guilt/sql-plugin branch from acf6a0c to c2face4 Compare January 30, 2023 06:27

endothermicdev approved these changes Jan 30, 2023

View reviewed changes

endothermicdev merged commit fea7368 into ElementsProject:master Jan 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sql plugin #5679

sql plugin #5679

rustyrussell commented Oct 28, 2022 •

edited

Loading

jb55 commented Oct 28, 2022

jb55 commented Oct 29, 2022

cdecker left a comment

cdecker Nov 1, 2022

rustyrussell Nov 1, 2022

rustyrussell commented Jan 13, 2023

rustyrussell commented Jan 15, 2023

rustyrussell commented Jan 16, 2023

endothermicdev left a comment

endothermicdev Jan 30, 2023

		@@ -0,0 +1,43 @@
		#! /usr/bin/env python3
		# Script to turn JSON schema into simple CSV describing fields.

sql plugin #5679

sql plugin #5679

Conversation

rustyrussell commented Oct 28, 2022 • edited Loading

jb55 commented Oct 28, 2022

jb55 commented Oct 29, 2022

cdecker left a comment

Choose a reason for hiding this comment

cdecker Nov 1, 2022

Choose a reason for hiding this comment

rustyrussell Nov 1, 2022

Choose a reason for hiding this comment

rustyrussell commented Jan 13, 2023

rustyrussell commented Jan 15, 2023

rustyrussell commented Jan 16, 2023

endothermicdev left a comment

Choose a reason for hiding this comment

endothermicdev Jan 30, 2023

Choose a reason for hiding this comment

rustyrussell commented Oct 28, 2022 •

edited

Loading