lovasoa / SQLpage

SQL-only webapp builder, empowering data analysts to build websites and applications quickly
https://sql.ophir.dev
MIT License
882 stars 62 forks source link

on_disconnect.sql ? #373

Open lyderic opened 1 month ago

lyderic commented 1 month ago

Introduction

I am trying to use functions in SQLite and I found the define SQLite extension from the reputable sqlean set.

In the documentation, they say: "_Scalar functions are compiled into prepared statements. SQLite requires these statements to be freed before the connection is closed. Unfortunately, there is no way to free them automatically. Therefore, always execute define_free() before disconnecting_".

When I call define_free() at the end of a .sql page though, sqlpage crashes after a page refresh.

To Reproduce

  1. Download the sqlean zip file for your OS from the sqlean release page and uncompress it somewhere, e.g. I am on Linux and I chose /usr/local/lib/sqlext
$ wget https://github.com/nalgeon/sqlean/releases/download/0.23.0/sqlean-linux-x86.zip
$ sudo mkdir -p /usr/local/lib/sqlext
$ sudo unzip sqlean-linux-x86.zip define.so -d /usr/local/lib/sqlext
  1. Create a sqlpage project with the following configuration file:

sqlpage/sqlpage.yaml

port: 4004
database_url: "sqlite://:memory:"
sqlite_extensions:
  - "/usr/local/lib/sqlext/define.so"
  1. Create this migration script to define a function:

sqlpage/migrations/01_functions.sql

SELECT define('eurof', 'format(''%,.2f €'', :n)');
  1. Create a basic home page:

index.sql

SELECT 'text' AS component, eurof(3000.3892) AS contents;
SELECT define_free();

Actual behavior

Open a browser tab to http://localhost:4004. The page loads normally and the expected value is displayed, as formatted by the eurof function, i.e. 3,000.39 €. So far, so good.

However, if the page is refreshed (hit Ctrl-R once or twice), sqlpage crashes and doesn't display anything else than:

Segmentation fault (core dumped)

Expected behavior

I expected no crash, of course. It is subtle, as one has to reload the page to make the sqlpage service crash.

It doesn't look like a bug in sqlean (I can't reproduce it when using the sqlite3 interpreter).

Maybe I don't put the call to define_free() in the right place. If this is the case, where shall I put it? In other words, as I am supposed to call define_free() before disconnecting, when does sqlpage "disconnect" from sqlite?

Maybe the call to define_free() is not necessary at all! If I comment is out, there is no crash: in this case though, is this not a potential memory leak?

If it helps, I attach a log of the the full interaction. I produced it with the following command:

RUST_LOG=sqlpage=debug sqlpage > log.txt 2>&1

Version information

Additional context

I am not sure this is a bug, or me missing something. I apologize in advance if this is misleading.

lovasoa commented 1 month ago

Hello and thank you for the report!

SQLPage uses connection pools; it creates and drops connections with a more sophisticated logic than just one connection per request.

Unfortunately, SQLPage allows running code on connection, but currently does not allow executing custom code on deconnection.

Until such a mechanism is implemented, I recommend you just never call define_free, and set max_database_pool_connections to 1 in order not to leak memory.

lyderic commented 1 month ago

Fair enough. Many thanks.

lovasoa commented 1 month ago

An your SELECT define('eurof', 'format(''%,.2f €'', :n)') should go to on_connect.sql, not to the page itself or the migrations

lyderic commented 1 month ago

Yes. That makes sense. Thanks!

lyderic commented 1 month ago

I followed all your suggestions:

  1. My configuration file sqlpage/sqlpage.yaml now looks like this:
port: 4004
database_url: "sqlite://:memory:"
sqlite_extensions:
  - "/usr/local/lib/sqlext/define.so"
max_database_pool_connections: 1
  1. I removed the sqlpage/migrations directory and all its content

  2. I created a new file: sqlpage/on_connect.sql:

SELECT define('eurof', 'format(''%,.2f €'', :n)');
  1. I commented out the define_free() line in index.sql:
SELECT 'text' AS component, eurof(3000.3892) AS contents;
--SELECT define_free();

Now, all looks ok. I don't have any live traffic on this site yet, as I am only testing for myself. I don't know how things will be when/if I get concurrent connections though.

Also, when I Ctrl-C out of sqlpage, I get this error now:

^Cthread 'sqlx-sqlite-worker-0' panicked at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/sqlx-core-oldapi-0.6.22/src/sqlite/connection/handle.rs:103:17:
(code: 5) unable to close due to unfinalized statements or unfinished backups
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Aborted (core dumped)

Running RUST_BACKTRACE=1 sqlpage instead of just sqlpage didn't yield more output.

Maybe it's not a problem, just untidy ;-) However, now, the exit code of the sqlpage process is 134 instead of 0 and I guess this will be an annoyance in prod, when sqlpage is deployed with things like docker or systemd.

This is very low priority, but on_disconnect.sql would be handy indeed.

For the moment, I will try to avoid sqlean's define as it doesn't look that it's playing well with sqlpage.

lovasoa commented 1 month ago

Maybe it's not a problem, just untidy

Yes, we'll need to implement on_disconnect.sql to allow running the define_free cleanly.

lyderic commented 1 month ago

Excellent. Thanks!

On Tue, 4 Jun 2024, 18:29 Ophir LOJKINE, @.***> wrote:

Maybe it's not a problem, just untidy

Yes, we'll need to implement on_disconnect.sql to allow running the define_free cleanly.

— Reply to this email directly, view it on GitHub https://github.com/lovasoa/SQLpage/issues/373#issuecomment-2147940400, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGTK5LMT7KQYTLZPW6NMR3ZFXS3NAVCNFSM6AAAAABIYBGTQKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBXHE2DANBQGA . You are receiving this because you authored the thread.Message ID: @.***>