Open mqus opened 4 years ago
I have never used the:VVV
syntax before sorry!
You should be able to use positioned parameter https://www.sqlite.org/c3ref/bind_blob.html.
You can use ?NNN
to specify a parameter position:
expect(
await db.rawQuery(
'SELECT ?1 as item1, ?2 as item2, ?1 + ?2 as sum', [3, 4]),
[{'item1': 3, 'item2': 4, 'sum': 7}]);
I will add this in the doc...
Thank you! But i really would like to see the named parameters.
Maybe I should have provided a bit more background:
I'm currently contributing to the floor ORM, where we currently just replace all occurrences of :abc
with ?
, including all limitations this brings with it (https://github.com/vitusortner/floor/issues/224, https://github.com/vitusortner/floor/issues/142). But I just discovered that parameter binding is a feature of sqlite itself so it would be nice if we could provide our users the same capabilities without messing with their queries.
This would however require that we can use rawQuery
in the way I described. I don"t know what abstractions sqflite uses but the raw c api seems to provide some facilities(1 2) for finding and supplying named parameters, so it could be a relatively minor effort to add this.
But as always: this is just a request, feel free to ignore it if you think this is not feasible, we will then just have to work around it (and the ?NNN
syntax will probably useful for that) ;).
But i really would like to see the named parameters
Ok I was not even aware of that feature. Sounds useful indeed.
I'm not sure Android supports named parameters though (https://stackoverflow.com/questions/27233530/android-sqlite-named-parameters/27235389#27235389) but I have not looked much.
The swapping could be done at the dart level but I avoid parsing as much as possible on the dart side for performance so maybe this could be done upfront only when needed (since it has a cost that could be avoided). It should be fairly easy (like you have done) to write a command (sql/param) converter from named to positioned parameter. If someone has an implementation we could add it to sqflite.
There is no union type to allow arguments to be either a List or a Map and adding another optional parameter in every request might not be the best evolution.
I was thinking of adding a Command or Statement class to have something like that:
Statement convertNamedParameters(
{@required String sql, Map<String, dynamic> arguments}) {
// TODO Convert :key1, :key2 to ?1, ?2 parameters
return Statement(sql: sql, arguments: [arguments]);
}
/// SQLite Statement.
class Statement {
final String sql;
final List<dynamic> arguments;
/// A SQL command and its positioned parameters
Statement({@required this.sql, this.arguments});
/// A SQL command and its named parameters
factory Statement.named(
{@required String sql, Map<String, dynamic> arguments}) {
// Convert :key1, :key2 to ?1, ?2 parameters
return convertNamedParameters(sql: sql, arguments: arguments);
}
}
/// Database Statement extension.
extension SqfliteStatementExecutor on Statement {
/// run a query statement (SELECT)
Future<List<Map<String, dynamic>>> query(DatabaseExecutor db) {
return db.rawQuery(sql, arguments);
}
}
Future testDatabaseAccess(Database db) async {
var statement = Statement(sql: 'PRAGMA user_version');
var results = await statement.query(db);
print(results);
statement = Statement(sql: 'SELECT ?1', arguments: [1234]);
results = await statement.query(db);
print(results);
statement = Statement.named(sql: 'SELECT :a', arguments: {'a': 1234});
results = await statement.query(db);
print(results);
}
This would allow support for named parameters
I realize I read your issue too quickly. You have everything figured out already. If named parameters is not supported on all platforms (and as far as I know, it is not the case for Android but I could be wrong) we cannot do much better than what you do now by converting named parameters to positioned parameters. Whether we want to add support for named parameters in sqflite if we fake it by conversion could be a solution and I'd rather have a good API for it.
Well, if Android doesn't support it then it doesn't seem feasible to implement it here (in the way I expected). In floor, we would implement it in our code generation engine so it would not incur any runtime costs but that route is not available to sqflite and I think dealing with this should not be the task of sqflite (imho). I would prefer sqflite to be a relatively thin layer over the platform APIs.
So summarizing: If you document that parameter binding can be done by position but not by name this issue can be considered as closed to me :)
I would prefer sqflite to be a relatively thin layer over the platform APIs.
That is exactly its goal.
If you document that parameter binding can be done by position but not by name
After some testing, I found some minor issues to fix to have positionning support (currently ?
has better supported then ?NNN
)
The following test failed on Android:
expect(await db.rawQuery('SELECT ?1 as a', [2]), [{'a': 2}]);
although this one succeeds:
expect(await db.rawQuery('SELECT ? as a', [2]), [{'a': 2}]);
Android convert bind parameter using String (! so result is {'a':'2'}
) so I have already some parsing done on Android to replace int parameter directly in the result to avoid some unexpected String conversion. It seems I should handle ?NNN
too to have proper positioning supported; hoping it does not introduce other issues.
Once this is fixed, I'll document absolute positioning. In the meantime, stick with ?
alright! Thank you!
Currently I can do the following:
and also
and it will work because the list has the same positions as the parameters in the query. but if I want to do, e.g.
I can't tell sqflite which parameter is which, especially if I can't rely on ordering. I propose an optional additional list which provides the names to the parameter values. Another option would be to also accept maps for
arguments
, which would map parameter names to parameter values.But both options only cover the all-or-nothing approach, meaning they don't address cases in which named and positioned parameters intermingle and I don't know a good solution for that. But sqflite is surely not the first to have that issue and there are probably ways in which other sql libraries have solved this.