Feature/raw query

[dkaera]

[vitusortner]

@dkaera thanks for your contribution! Is there anything we can help you with to continue working on this feature?

[dkaera]

@dkaera thanks for your contribution! Is there anything we can help you with to continue working on this feature?

Currently not, thank you. I've to get time to finish work on MR this week. Sorry for the delay.

[codecov-commenter]

Codecov Report

Merging #517 (e3a0d23) into develop (7e74482) will decrease coverage by 1.53%. The diff coverage is 70.83%.

@@             Coverage Diff             @@
##           develop     #517      +/-   ##
===========================================
- Coverage    90.07%   88.54%   -1.54%     
===========================================
  Files           74       79       +5     
  Lines         1853     1911      +58     
===========================================
+ Hits          1669     1692      +23     
- Misses         184      219      +35     

Flag	Coverage Δ
floor	90.57% <ø> (ø)
floor_generator	88.31% <70.83%> (-1.70%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
.../processor/error/query_method_processor_error.dart	100.00% <ø> (ø)
...cessor/error/raw_query_method_processor_error.dart	0.00% <0.00%> (ø)
...ator/lib/processor/raw_query_method_processor.dart	0.00% <0.00%> (ø)
floor_generator/lib/value_object/query_method.dart	63.33% <ø> (ø)
floor_generator/lib/value_object/sqlite_query.dart	0.00% <0.00%> (ø)
floor_generator/lib/processor/dao_processor.dart	94.54% <70.00%> (-5.46%)	:arrow_down:
floor_generator/lib/misc/type_utils.dart	93.93% <77.77%> (-6.07%)	:arrow_down:
...loor_generator/lib/writer/query_method_writer.dart	96.00% <80.95%> (-4.00%)	:arrow_down:
...tor/lib/processor/base_query_method_processor.dart	100.00% <100.00%> (ø)
...essor/error/base_query_method_processor_error.dart	100.00% <100.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[dkaera]

@vitusortner @mqus @ThomasMiddel hey hey, It took a while to get back to it, but now the real working integration is done. All that's left is to finish updating the tests and documentation 😀

[MrCsabaToth]

Can we have a dedicated feature issue for this PR (I didn't find any, sorry if there is one)? The PR description is empty, as well, an issue ticket could explain what are the aims of the PR, how does it improve the current rawQuery. The current rawQuery takes string query and an array and substitutes the array values into the "?" marked places. One may wonder if this PR will be a breaking change, maybe preserve the current rawQuery for compatibility and a new one would be sqliteRawQuery or something, things like that.

[dkaera]

Can we have a dedicated feature issue for this PR (I didn't find any, sorry if there is one)? The PR description is empty, as well, an issue ticket could explain what are the aims of the PR, how does it improve the current rawQuery. The current rawQuery takes string query and an array and substitutes the array values into the "?" marked places. One may wonder if this PR will be a breaking change, maybe preserve the current rawQuery for compatibility and a new one would be sqliteRawQuery or something, things like that.

The main idea of this PR is to allow the generation of RawQuery inside DAO interfaces and support the whole Floor framework ecosystem (mapping, database-view, etc.). The current usage of rawQuery like

Future<int?> count() async {
    final floor = await AppDatabase.getInstance();
    var result = await floor!.database.rawQuery('SELECT COUNT(*) FROM GameResult');
    return result.first['COUNT(*)'] as int;
  }

is a workaround, which in my opinion should be encapsulated. Perhaps I'm missing something, and the issue has already been resolved in another way.

PS: and you're right, without description it's not clear what I suggest. My bad 😬

[MrCsabaToth]

@dkaera No worries and I didn't want to bash on you and I'm happy people are contributing. But I'm sure others may have questions.

I'm not sure about others, but in all of my DAOs only insert / update / delete functions don't have any annotations, all of my find functions have a Query annotation which is kind of a raw SQL query except that the code generator engine can pair up the references in that query with the parameters of the function. For example this is typical in all my DAOs for paging: https://github.com/TrackMyIndoorWorkout/TrackMyIndoorWorkout/blob/d97e6be59cee465ec20a94dd186af197f36076cf/lib/persistence/dao/workout_summary_dao.dart#L16

However (as a possible instigator for this PR as well) the COUNT type raw SQL queries were not be able to be processed by the analyzer #200. As far as I understand there was an effort to refactor the analyzer, but that got abandoned, it's definitely a huge work. However some issues may have been closed because that new analyzer promised solution? Not sure.

Anyways, I have COUNT queries and DISTINCT queries, which I had to elevate to the FloorDatabase / AppDatabase level. Such as https://github.com/TrackMyIndoorWorkout/TrackMyIndoorWorkout/blob/d97e6be59cee465ec20a94dd186af197f36076cf/lib/persistence/database.dart#L119 or https://github.com/TrackMyIndoorWorkout/TrackMyIndoorWorkout/blob/d97e6be59cee465ec20a94dd186af197f36076cf/lib/persistence/database.dart#L130 for DISTINCT and this core function for all COUNTs I use: https://github.com/TrackMyIndoorWorkout/TrackMyIndoorWorkout/blob/d97e6be59cee465ec20a94dd186af197f36076cf/lib/persistence/database.dart#L46

  Future<int> rowCount(String tableName, String deviceId, {String extraPredicate = ""}) async {
    var queryString = "SELECT COUNT(`id`) AS cnt FROM `$tableName` WHERE `mac` = ?";
    if (extraPredicate.isNotEmpty) {
      queryString += " AND $extraPredicate";
    }

    final result = await database.rawQuery(queryString, [deviceId]);

    if (result.isEmpty) {
      return 0;
    }

    return result[0]['cnt'] as int? ?? 0;
  }

So if I understand it correctly your PR will make it possible that the COUNT type row queries will be able to be at DAO level (where they should be)? You are modifying the analyzer for that. Will there be any breaking changes regarding the current database.rawQuery? Will it be able to handle the DISTINCT use-case by any chance? We should open an issue and reference this PR.

[MrCsabaToth]

Actually, if this focuses only on the COUNT use-case the issue could be #200 itself. If it's smarter / broader than that then maybe another issue.

[dkaera]

@dkaera No worries and I didn't want to bash on you and I'm happy people are contributing. But I'm sure others may have questions.

I'm not sure about others, but in all of my DAOs only insert / update / delete functions don't have any annotations, all of my find functions have a Query annotation which is kind of a raw SQL query except that the code generator engine can pair up the references in that query with the parameters of the function. For example this is typical in all my DAOs for paging: https://github.com/TrackMyIndoorWorkout/TrackMyIndoorWorkout/blob/d97e6be59cee465ec20a94dd186af197f36076cf/lib/persistence/dao/workout_summary_dao.dart#L16

However (as a possible instigator for this PR as well) the COUNT type raw SQL queries were not be able to be processed by the analyzer #200. As far as I understand there was an effort to refactor the analyzer, but that got abandoned, it's definitely a huge work. However some issues may have been closed because that new analyzer promised solution? Not sure.

Anyways, I have COUNT queries and DISTINCT queries, which I had to elevate to the FloorDatabase / AppDatabase level. Such as https://github.com/TrackMyIndoorWorkout/TrackMyIndoorWorkout/blob/d97e6be59cee465ec20a94dd186af197f36076cf/lib/persistence/database.dart#L119 or https://github.com/TrackMyIndoorWorkout/TrackMyIndoorWorkout/blob/d97e6be59cee465ec20a94dd186af197f36076cf/lib/persistence/database.dart#L130 for DISTINCT and this core function for all COUNTs I use: https://github.com/TrackMyIndoorWorkout/TrackMyIndoorWorkout/blob/d97e6be59cee465ec20a94dd186af197f36076cf/lib/persistence/database.dart#L46

  Future<int> rowCount(String tableName, String deviceId, {String extraPredicate = ""}) async {
    var queryString = "SELECT COUNT(`id`) AS cnt FROM `$tableName` WHERE `mac` = ?";
    if (extraPredicate.isNotEmpty) {
      queryString += " AND $extraPredicate";
    }

    final result = await database.rawQuery(queryString, [deviceId]);

    if (result.isEmpty) {
      return 0;
    }

    return result[0]['cnt'] as int? ?? 0;
  }

So if I understand it correctly your PR will make it possible that the COUNT type row queries will be able to be at DAO level (where they should be)? You are modifying the analyzer for that. Will there be any breaking changes regarding the current database.rawQuery? Will it be able to handle the DISTINCT use-case by any chance? We should open an issue and reference this PR.

Hey, thanks for the in-depth analysis of integrating this feature. I really appreciate it 👍 Opened a discussion to proceed😄