Closed simolus3 closed 1 year ago
I think we should have this one. I'm not sure yet how this should be exposed on the public API. Any suggestions?
I'll open a PR with an interface that would work for me :) Then we can see what needs to happen for this to be generally useful.
SGTM. Keep in mind that I am planning to refactor the public API a bit for a while (also merge with package:postgres_pool), so we could do bundled breaking change here...
I've opened a PR which doesn't really touch existing APIs all that much.
so we could do bundled breaking change here...
If we're fine with considerable breaking changes, I think it might be a good idea to separately expose high-level and low-level APIs. I just spent three hours trying to figure out why my changes were breaking something only to find out it was caused by automatic query caching in the end...
I would love a low-level API where you'd manually send the BEGIN
/COMMIT
statements for transactions, separate query preparation, execution and disposal and so on. The current high-level API could be built ontop of the lower-level stuff.
I would love a low-level API where you'd manually send the BEGIN/COMMIT statements for transactions, separate query preparation, execution and disposal and so on. The current high-level API could be built ontop of the lower-level stuff.
It would be nice if the PR could introduce something, without affecting the Connection or the ExecutionContext interface. I think the API should be either high-level or low-level, but not both at the same time. (Unfortunately the messages introduced in 2.5 are mixing it already, but it would be nicer if we could start separating these before introducing them).
What would it look like? queryDirect
seems to be a good starting point for separation...
I think the API should be either high-level or low-level, but not both at the same time.
At the moment, this package only exposes the raw messages as a low-level API and the postgres connection class (implementing the execution context interface). There's no class to do the basic connection management.
I'm not sure I fully understand the changes you had in mind. My idea is to introduce a completely new API (say package:postgres/direct.dart
) that would be fairly simple. My general idea is something like this (with nicer APIs here and there, just to get a sense of the structuring):
abstract class PostgresDatabase {
static Future<PostgresDatabase> connect(...);
Future<QueryResponse> sendSimple(String sql);
Future<PreparedStatement> prepare(String sql, {List<PostgreSQLDataType>? parameterTypes});
}
abstract class PreparedStatement {
Future<QueryResponse> executewith(List<Object?> parameters);
Future<void> dispose();
}
abstract class QueryResult implements List<List<Object?>> {
int get affectedRows;
List<ResultColumn> get schema;
}
abstract class ResultColumn {
PostgreSQLDataType get dataType;
int? get tableOid, columnOid;
}
I think provides is enough to implement the current API ontop of it. Do you agree with exposing something like this?
I've been fiddling with this a bit, while also trying to integrate @osaxma's messages API and the pooling API into a unified API. So far I am beginning to thing that we can't/shouldn't really hide these APIs from the pooling logic: it shouldn't really matter if you get a connection from a pool or connect using a known connection string - in both cases you should be able to access the "low-level" APIs too...
So on that thought, I wouldn't treat this as a direct/raw connection vs. a higher-level API, rather, I'd treat it as re-defining the API and its semantics (both could be a breaking change from the current design). We could start developing it as a separate library for now (e.g. lib/postgres_v3_experimental.dart
), with better consistency in naming things, and maybe stricter semantics (I think a sizeable amount of complexity could be reduced if we'd always wait for the previous command to complete and wouldn't send interleaving messages).
wdyt?
it shouldn't really matter if you get a connection from a pool or connect using a known connection string
I agree with this. But they still can't have the same API because low-level stuff like prepared statements are inherently bound to one connection: You can't prepare a statement on one connection and then use it on another.
Perhaps we could have a shared interface between connections and a connection pool? It can expose methods used to run entire queries (which is still fairly low-level if we're not integrating the text substitution mechanism right away). Stuff that's bordering on implementation details, like explicitly preparing and disposing statements, would be exposed on connections only.
with better consistency in naming things, and maybe stricter semantics
This sounds great to me!
I think we can close this since it has been addressed in the v3 prototype.
In all methods exposed from the high-level APIs exposed by this package, queries are scanned locally to format parameters. This is convenient for users. However, some packages like ORMs which auto-generate SQL don't need the local processing and can deal with providing the additional information themselves. Here, the indirection from
package:postgres
may even cause unintended effects or malformed queries.It would be great to have a method that takes a SQL and a list of
ParameterValue
that will be bound to it. It should not modify the SQL sent to the database in any way. This also means that the caller will have to figure out all the right parameter types, but exactly what I want :)