Handle multi-statement queries

[bartlomieju]

We need to be able to handle queries like this:

const query = `
    CREATE TABLE ids(id integer); 
    INSERT INTO ids(id) values(1); 
    INSERT INTO ids(id) values(2);
`;

await client.query(query);

I'm not sure how to nicely handle results, currently QueryResult has rows property that stores data returned from query. One idea is to subclass QueryResult and create MultiQueryResult that has slight different API and leave it up to user to handle that properly.

Comments welcome

[bartlomieju]

After giving it some thought I'm leaning towards prohibiting multi-statements queries in query method. Instead there will be separate method for such cases:

const query = `
    CREATE TABLE ids(id integer); 
    INSERT INTO ids(id) values(1); 
    INSERT INTO ids(id) values(2);
`;

// throws error
await client.query(query);
// returns QueryResult[]
await client.multiStatementQuery(query);`

Still multiStatementQuery is not the best name. queryMulti?

[hayd]

What about client.transaction ?

We could conveniently wrap in a begin/commit/rollback try/catch block.

---

pseudocode:

await c.query("BEGIN")
const results: Array<QueryResult> = [];
try {
  for (const q of query) {
    results.push(await c.query(q));
  }
  await c.query("COMMIT");
  return results;
} catch (e) {
  await c.query("ROLLBACK");
  throw e;
}

[bartlomieju]

That's a separate issue, I just want to prevent using multi-statement queries in query because I don't want to give it QueryResult | QueryResult[] return type. 95% of queries will be single queries and there's really no need for devs to check every time if they got array or single result.

[hayd]

Agree it should be QueryResult, I don't think this even needs a helper function (they can use an explicit for loop or map)...

[halvardssm]

Hi! I am using this driver for a migration tool that i am creating, and I coud use the posibility of a multi statement query. What is the status of this issue? 🙂

[bartlomieju]

@halvardssm I don't plan to work on it in foreseeable future; feel free to open a PR if you want

[halvardssm]

@bartlomieju Ok, I will take a look 💪 could you point me to the file I should add this in?

[bartlomieju]

@halvardssm that should be query.ts

[halvardssm]

@bartlomieju So I am working on this right now and I was hoping to discuss with you if it is any purpose in supporting all of these types (string | string[] | QueryConfig | QueryConfig[]) on the input? I personally think the approach for this would be to use the interface QueryConfig as the blueprint for the method arguments, e.g.

query(text: string, args?: unknown[], name?: string, encoder?: (arg: unknown) => EncodedArg){...}
transaction(text: string | string[], args?: unknown[], name?: string, encoder?: (arg: unknown) => EncodedArg){...}

This way we don't have to do the check for the text input in multiple methods, and transaction can simply be a for ... of wrapper for query. What do you think about this? Do you prefer if we keep it as it?

[bartlomieju]

@halvardssm I'm open to change of interface, maybe open a PR so we can discuss on living organism?