Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defining own Queryable interface #158

Open
moltar opened this issue Aug 8, 2023 · 2 comments
Open

Defining own Queryable interface #158

moltar opened this issue Aug 8, 2023 · 2 comments

Comments

@moltar
Copy link

moltar commented Aug 8, 2023

I propose that this package defines it's own Queryable rather than using pg built-ins:

export type Queryable = pg.ClientBase | pg.Pool;

The advantage of this could potentially open up uses with other compatible packages or even developing adapter for other packages.

If this was possible, I could implement this narrow interface on my own, for example to implement better retry logic.

// pseudo-code
class PoolWithRetry implements Queryable {
  connect(): Promise<any> {
	// own implementation
  }
  query(): Promise<any> {
	// own implementation
  }
}

As a minimum, we could even just narrow the type by using Pick. But ideally, it should be more tightly defined, and only define the parts that zapatos needs.

diff --git a/src/db/core.ts b/src/db/core.ts
index d0f0271..f57b08a 100644
--- a/src/db/core.ts
+++ b/src/db/core.ts
@@ -203,8 +203,7 @@ export type GenericSQLExpression = SQLFragment<any, any> | Parameter | DefaultTy
 export type SQLExpression = Table | ColumnNames<Updatable | (keyof Updatable)[]> | ColumnValues<Updatable | any[]> | Whereable | Column | ParentColumn | GenericSQLExpression;
 export type SQL = SQLExpression | SQLExpression[];
 
-export type Queryable = pg.ClientBase | pg.Pool;
-
+export interface Queryable extends Pick<pg.ClientBase | pg.Pool, 'connect' | 'query'> { };
 
 // === SQL tagged template strings ===

Additionally, this type of queryable detection will need a rework:

if (queryable instanceof pg.Pool) return 'pool';
if (queryable instanceof pg.Client) return 'client';
if (pg.native !== null && queryable instanceof pg.native.Pool) return 'pool';
if (pg.native !== null && queryable instanceof pg.native.Client) return 'client';


Related: #77

@jawj
Copy link
Owner

jawj commented Aug 9, 2023

I think this is a good idea in principle — I'd actually like to drop the direct pg dependency, so Zapatos can work with @neondatabase/serverless without any trickery.

But, as you say, distinguishing between a Client and a Pool in the transaction function is critical, and I'm not sure how we're going to do that in this case?

@moltar
Copy link
Author

moltar commented Aug 9, 2023

I think the safest way is to actually check for the behaviour of the interface.

if ('connect' in queryable && typeof queryable.connect === 'function') {
  // pool
}

And perhaps have adapters as an additional layer. But this, ofc, would be a breaking change.

.run(ZapatosPgDriverAdapter.fromPool(pgPool))
.run(ZapatosPgDriverAdapter.fromClient(client))
.run(ZapatosNeonDriverAdapter.fromClient(client))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants