neondatabase / serverless

Connect to Neon PostgreSQL from serverless/worker/edge functions
https://www.npmjs.com/package/@neondatabase/serverless
MIT License
318 stars 11 forks source link

add batch query interface #38

Closed skyzh closed 11 months ago

skyzh commented 11 months ago

This is a very early draft of non-interactive txn batch query interface and might need some refactor before merging.

https://github.com/neondatabase/neon/issues/4703

TL;DR, we added a new interface:

  const { txn, stmt, sql } = neon(env.NEON_DB_URL);
  await txn([
    stmt`SELECT ${1}::int AS int`,
    stmt`SELECT ${"hello"} AS str`
  ], /* ...options */)

meanwhile, we are also compatible with the original way of handling a query:

  const sql = neon(env.NEON_DB_URL);
  await sql(`SELECT ${1}::int AS int`)

several challenges of the current architecture:

skyzh commented 11 months ago

I think we also need to support something like:

  const { txn, stmt, sql } = neon(env.NEON_DB_URL);
  await txn([
    stmt`SELECT ${1}::int AS int`,
    "SELECT 1"
  ], /* ...options */)

...?

kelvich commented 11 months ago

wonder if it is possible to introduce only one new thing instead of two, e.g. make it possible to write:

  const { txn, sql } = neon(env.NEON_DB_URL);
  await txn([
    sql`SELECT ${1}::int AS int`,
    sql`SELECT ${"hello"} AS str`
  ], /* ...options */)

Right away I don't have any good idea how to implement that except accessing some global state to detect txn context (which would be a mess to recover from errors).

skyzh commented 11 months ago

wonder if it is possible to introduce only one new thing instead of two, e.g. make it possible to write:

  const { txn, sql } = neon(env.NEON_DB_URL);
  await txn([
    sql`SELECT ${1}::int AS int`,
    sql`SELECT ${"hello"} AS str`
  ], /* ...options */)

Right away I don't have any good idea how to implement that except accessing some global state to detect txn context (which would be a mess to recover from errors).

Of course this is possible by attaching a property to the function object and then extract it. But I feel it would be too tricky for users to understand what's happening...?

skyzh commented 11 months ago

the way to do that, sql function returns a promise that resolves to the single-flight query result, as well as some properties that can be extracted later by txn call. If await is not placed before sql, then the promise will not be resolved immediately.

jawj commented 11 months ago

Agree with Stas that it would be good to be able to use the same sql function inside a transaction block as outside.

Also: we need a way to specify the transaction isolation level.

kelvich commented 11 months ago

But I feel it would be too tricky for users to understand what's happening...?

For me less imports is less burden to understand what each thing does. And txn block around is explicit enough. But that is subjective, right.

the way to do that, sql function returns a promise that resolves to the single-flight query result, as well as some properties that can be extracted later by txn call. If await is not placed before sql, then the promise will not be resolved immediately.

ah, right. forgot that it is async function

Also: we need a way to specify the transaction isolation level.

good point. Also there is read only qualifier for transaction.

skyzh commented 11 months ago

I just realized that JavaScript promise is different from Rust futures. That is to say, once the promise object is created, the runtime will execute it in the background. Therefore, attaching properties to the promise does not look like a good idea...


It seems that we can create a deferred promise as in https://masteringjs.io/tutorials/fundamentals/promise-create#:~:text=JavaScript%20promises%20are%20%22hot%22%20in,a%20new%20promise%20every%20time. but this will cause problems for error reporting.

nicksrandall commented 11 months ago

I took a stab at this in #39

The api looks like:

const sql = neon(...);
const results = await sql.transaction([
  sql`SELECT ${1}::int AS int`,
  sql`SELECT ${"hello"} AS str`
], /* options */);

Which (I think) achieves all the goals stated above. It's not a breaking change because the single queries worked as before!