Inconsistent/incorrect savepoint results in remote and remote_replica connections

[sveltespot]

Observing inconsistent results/error when SAVEPOINT is executed outside of a transaction in local vs remote vs remote_replica. Savepoint executed outside of a transaction on a local database succeeds, where as when executed in both remote and remote_replica connections it fails, albeit for different reasons. For remote_replica connections, it fails with the reason that savepoint cannot be started outside a transaction, which according to sqlite manual is incorrect. It seems to have been disabled explicitly in code (incorrectly?). See here:

libsql/libsql/src/replication/connection.rs

Line 62 in cf43aa2

(_, StmtKind::Savepoint) => State::Invalid,

The fix for this being simple enough, just adding the below line seems to fix this issue:

(State::Init, StmtKind::Savepoint) => State::Txn,

For remote connections however, savepoint execution doesn't fail, but rollback to savepoint fails (when savepoint is started outside of a transaction), with the error SQLite error: no such savepoint: ....

Adding sqlite docs explanation for savepoint below (https://www.sqlite.org/draft/lang_savepoint.html) :

A SAVEPOINT can be started either within or outside of a BEGIN...COMMIT. When a SAVEPOINT is the outer-most savepoint and it is not within a BEGIN...COMMIT then the behavior is the same as BEGIN DEFERRED TRANSACTION.

Below is a reproducer for this issue:

use libsql::{Builder, Connection, Result};
use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt};

#[tokio::main]
async fn main() {
    let filter = tracing_subscriber::EnvFilter::try_from_default_env()
        .unwrap_or_else(|_| "savepoint_transaction=debug,libsql=info".into());
    tracing_subscriber::registry()
        .with(filter)
        .with(tracing_subscriber::fmt::layer())
        .init();
    // tracing_subscriber::fmt()
    //     .with_max_level(tracing::Level::TRACE)
    //     .init();

    let db_url = "http://localhost:8080";
    let local = Builder::new_local(":memory:").build().await.unwrap();
    let remote = Builder::new_remote(db_url.to_string(), String::new())
        .build()
        .await
        .unwrap();
    let replica = Builder::new_remote_replica(
        "/tmp/embedded_transaction.db",
        db_url.to_string(),
        String::new(),
    )
    .build()
    .await
    .unwrap();
    let replica_conn_1 = replica.connect().unwrap();
    let replica_conn_2 = replica.connect().unwrap();
    let remote_conn = remote.connect().unwrap();
    let local_conn = local.connect().unwrap();

    setup_db(local_conn.clone()).await.unwrap();

    // For local db, everything works as expected.

    let local_task_1 = savepoint_within_tx(local_conn.clone(), "local").await;
    if local_task_1.is_err() {
        tracing::error!("Local savepoint within tx failed: {:?}", local_task_1.err());
    }

    let local_task_2 = savepoint_outside_tx(local_conn.clone(), "local").await;
    if local_task_2.is_err() {
        tracing::error!(
            "Local savepoint outside tx failed: {:?}",
            local_task_2.err()
        );
    }

    setup_db(remote_conn.clone()).await.unwrap();

    let remote_task_1 = savepoint_within_tx(remote_conn.clone(), "remote").await;
    if remote_task_1.is_err() {
        tracing::error!(
            "Remote connection savepoint within tx failed: {:?}",
            remote_task_1.err()
        );
    }

    // This fails with error:
    // `Some(Hrana(StreamError(Error { message: "SQLite error: no such savepoint: 123", code: "SQLITE_UNKNOWN" })))`
    let remote_task_2 = savepoint_outside_tx(remote_conn.clone(), "remote").await;
    if remote_task_2.is_err() {
        tracing::error!(
            "Remote connection savepoint outside tx failed: {:?}",
            remote_task_2.err()
        );
    }

    setup_db(replica_conn_1.clone()).await.unwrap();

    // This works as expected.
    let replica_task_1 = savepoint_within_tx(replica_conn_1, "replica").await;
    if replica_task_1.is_err() {
        tracing::error!(
            "Remote replica savepoint within tx failed: {:?}",
            replica_task_1.err()
        );
    }

    // This fails with error: `Error: Some(InvalidParserState("Init"))`
    let replica_task_2 = savepoint_outside_tx(replica_conn_2, "replica").await;
    if replica_task_2.is_err() {
        tracing::error!(
            "Remote replica savepoint outside tx failed: {:?}",
            replica_task_2.err()
        );
    }
}

async fn savepoint_within_tx(conn: Connection, caller: &str) -> Result<()> {
    tracing::info!("Executing savepoint_within_tx: {caller}");

    let tx = conn.transaction().await?;
    tx.execute("SAVEPOINT '123'", ()).await?;
    tx.execute("INSERT INTO test (name) VALUES (?1)", ["somename"])
        .await?;
    tracing::debug!("Inserted a row into the test table");

    tracing::debug!("Savepoint created. Executing a query...");
    let mut rows = tx.query("SELECT name from test", ()).await?;
    while let Some(row) = rows.next().await? {
        let name: String = row.get(0)?;
        tracing::debug!("Name: {}", name);
    }
    tracing::debug!("Rolling back to the savepoint..");
    tx.execute("ROLLBACK TO '123'", ()).await?;
    tx.commit().await?;
    Ok(())
}

async fn savepoint_outside_tx(conn: Connection, caller: &str) -> Result<()> {
    tracing::info!("Executing savepoint_outside_tx: {caller}");

    conn.execute("SAVEPOINT '123'", ()).await?;
    conn.execute("INSERT INTO test (name) VALUES (?1)", ["somename"])
        .await?;
    tracing::debug!("Inserted a row into the test table");

    let mut rows = conn.query("SELECT name from test", ()).await?;
    while let Some(row) = rows.next().await? {
        let name: String = row.get(0)?;
        tracing::debug!("Name: {}", name);
    }
    tracing::debug!("Rolling back to the savepoint..");
    conn.execute("ROLLBACK TO '123'", ()).await?;
    Ok(())
}
async fn setup_db(conn: Connection) -> Result<()> {
    conn.execute("DROP TABLE IF EXISTS test", ()).await?;
    conn.execute(
        "CREATE TABLE IF NOT EXISTS test (id INTEGER PRIMARY KEY, name TEXT)",
        (),
    )
    .await?;
    Ok(())
}