feat: add db.batch() for type-safe batched query execution#1619
feat: add db.batch() for type-safe batched query execution#1619WilcoKruijer wants to merge 1 commit intokysely-org:masterfrom
Conversation
|
@WilcoKruijer is attempting to deploy a commit to the Kysely Team Team on Vercel. A member of the Team first needs to authorize it. |
|
Hey 👋 Appreciate the effort, but I'm afraid we'll have to turn it down.
|
|
Thanks for your reaction. I understand why you closed the PR, and of course you're free to, but I'd still like to discuss now what it would take to land this PR. Issue #904 is stale when it comes to this subject. Your colleague posted a couple of questions, which I feel I've answered somewhat by implementing this functionality.
Yes, and we can do this through a builder pattern. That way it's not necessary to have type overload up til a certain number of queries.
The issue has a bunch of reactions, plus we have the Postgres documentation above that explains the benefits. I understand there are two different concepts. I decided to go for the term "batching" as I feel it's more generally accepted terminology to mean what Postgres supports in its pipeline mode. Your colleague also referenced to this process as batching. There's also prior art in other database libraries, such as Drizzle and the Neon Postgres driver. Lastly, I feel that "batching" is facing a chicken-and-egg type problem. Drivers point to query builders/ORMs to say batching is not supported, and ORMs point the other way. This is why I mentioned the currently open PR to I hoped to kickstart the discussion on batching by opening this PR, for me it's generally easier to discuss code when there is a PoC. My apologies for not following the process, but I hope you can reconsider at least opening the discussion on this subject. |
We don't have the concept of staleness. We don't rush into introducing things, especially core functionality. Kysely is widely used, and is at the core of some high-profile projects and libraries. Everything needs to be done right, and with care. The blast radius is bigger now.
Pause. This is not a corporate environment. This is not a company. This is not work. We're 2 lads doing this on our spare time for shits and giggles.
Doesn't matter if you answered questions 100%. You disregarded the process. Things moved around in the project since those questions were asked. There are things in the pipeline, that might conflict. There might have been work done on this already.
We don't care what Drizzle has or not.
That's how it goes. Databases have to support something. Then drivers have to support it. And only then abstractions can be made on top of drivers to support it.
The discussion is open, in the issue. GitHub has these dope things called gists, code blocks and permalinks that allow people to discuss with code snippets. Hopefully with more signal than noise (big PRs). |
Hi, thanks for the great project!
In this PR I've built a prototype for batch execution of queries as discussed in #904
The benefit of batching/pipelining as described in the Postgres manual:
This PR focuses on setting up the infrastructure to enable adapters to implement batching. Currently it does not actually implement batching for any driver. The main reason for this is that
node-postgresdoes not actually support pipelining curently. (See brianc/node-postgres#3357).Before this lands we need to remove batch support from the Postgres dialect in this codebase. I've added it now to aid testing. Postgres.js does actually support pipelining already, so after a version of this PR lands, we can add support there.
The API I've implemented looks like this:
In this example persons and pets are fully typed.
Main questions: Does this API look alright? Is everything implemented in the right places?
I would then need some guidance on actually implementing batching for the Postgres-js driver (in the other repo). In Postgres-js pipelining works as follows (see here):
How would we actually use this in the adapter?