karlseguin / pg.zig

Native PostgreSQL driver / client for Zig
MIT License
158 stars 9 forks source link

Concurrent queries on the same connection #1

Closed zigster64 closed 8 months ago

zigster64 commented 8 months ago

Coming from Go here ... so have some preconceptions about the meaning of "connection" vs "query"

Im doing this in Zig, and experiencing this issue

my_db = db_pool.aquire(); 
orders = my_db.query("select id from orders", .{});
while (orders.next()) |row| {
   ... do stuff
   my_db.row("update some_other_table set field1=$1 where keyfield=$2", .{value1, value2});  // bad idea !!!
}

The re-use of the connection to launch another query, whilst the outer query is still being used == a bad idea

You can see that the 1st Parse msg emitted by the update statement returns a packet of type 'D' instead of '1' which suggests that the conversation between the outer query and the PG server is getting messed up with the inner query.

Simple enough fix on my side

outer_db = db_pool.aquire();  // outer query
inner_db = db.pool.acquire(); // update query
orders = outer_db.query("select id from orders", .{});
while (orders.next()) |row| {
   ... do stuff
  // use a separate connection for the update
  inner_db.row("update some_other_table set field1=$1 where keyfield=$2", .{value1, value2});  
}

That's all good. I don't know if that is the intention of how "connection" and "query" should be related in this lib ?

If so, then maybe return an error .ConnectionInUse or something similar ?

Personal preference from me would be to keep pg.zig as low level as possible, and put any ergonomic higher level trickery into a pgx.zig wrapper lib

zigster64 commented 8 months ago

Note that pipelining multiple operations over the same connection works fine, as long as the previous operation has completed before the next one starts.

outer_db = db_pool.aquire();  // outer query
inner_db = db.pool.acquire(); // update query
orders = outer_db.query("select id from orders", .{});
while (orders.next()) |row| {
   ... do stuff
  // pipeline multiple updates over the same inner_db connection
  inner_db.exec("update some_other_table set field1=$1 where keyfield=$2", .{value1, value2});  
  inner_db.exec("insert into yet_other_table values ($1, $2, ...)", .{value1, value2});  
  inner_db.row("delete from lock_table where keyfield=$1 returning keyfield", .{value1});  
}

Again, keeping it low level allows the developer to make choices about re-using existing connections vs creating new ones.

The above code seems to have a pretty decent performance gain vs creating new connections on demand for example.

karlseguin commented 8 months ago

I made it return error.ConnectionBusy. I don't think there's an alternative...I can't see how you could possibly have 2 concurrent queries on the same connection at the same time. Do you know a driver that actually allows this? I tried with Go's pgx and got an error, which is what I'd expect:

package main

import (
    "context"
    "fmt"

    "github.com/jackc/pgx/v5"
)

func main() {
    // urlExample := "postgres://username:password@localhost:5432/database_name"
    conn, err := pgx.Connect(context.Background(), "postgres://localhost:5432/postgres")
    if err != nil {
        panic(err)

    }
    defer conn.Close(context.Background())
    rows, err := conn.Query(context.Background(), "select 1 union all select 2")

    if err != nil {
        panic(err)
    }

    defer rows.Close()
    for rows.Next() {
        var id, t int
        rows.Scan(&id)
        fmt.Println(id)
        if err := conn.QueryRow(context.Background(), "select 3").Scan(&t); err != nil {
            panic(err)
        }
        fmt.Println(t)
    }
}
zigster64 commented 8 months ago

Ah yes, of course .. you a right.

im porting from sqlx code, where the select() call slurps the entire result into a slice of struct. So underneath that call, it is reading every row from the original query .. and the range statement loops over the results, not the query result.

my bad .. I should have looked closer :(

karlseguin commented 8 months ago

No problem. I think the added explicit ConnectionBusy error is a good addition!

zigster64 commented 8 months ago

For what it’s worth… there is a daily batch job in prod now using this lib in anger on some financial data, patching info together from 3 separate db servers with 3 different versions of Postgres running.

good real world test