Page MenuHomePhabricator

[SQLite] implement getting `MessageToDevice` items
ClosedPublic

Authored by kamil on Feb 26 2024, 2:39 AM.
Tags
None
Referenced Files
F2066542: D11171.id.diff
Fri, Jun 21, 12:41 PM
F2063107: D11171.id37814.diff
Fri, Jun 21, 3:55 AM
Unknown Object (File)
Wed, Jun 19, 10:20 PM
Unknown Object (File)
Tue, Jun 18, 2:45 AM
Unknown Object (File)
Tue, Jun 18, 2:06 AM
Unknown Object (File)
Wed, Jun 12, 11:17 PM
Unknown Object (File)
May 12 2024, 11:59 PM
Unknown Object (File)
Apr 4 2024, 6:43 PM
Subscribers

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Mar 1 2024, 12:24 AM
marcin requested changes to this revision.Mar 1 2024, 2:22 AM
marcin added inline comments.
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
1717–1719 ↗(On Diff #37691)

The convention is not to use sqlite3_stmt directly but to use SQLiteStatementWrapper that automatically handles preparing, finalizing and errors.

1730–1735 ↗(On Diff #37691)

This code doesn't perform necessary error handling for sqlite3_step and sqlite3_finalize but if you use SQLiteStatementWrapper it will do error handling automatically for you.

This revision now requires changes to proceed.Mar 1 2024, 2:22 AM
marcin added inline comments.
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
1721 ↗(On Diff #37786)

Nit: Error message you provide here suggest that SQLite function that failed was sqlite3_finalize which is not necessarily the case. If one of sqlite3_step fails the SQLiteStatementWrapper destructor will throw an error with error message associated with the failed sqlite3_step. In other words the destructor of SQLiteStatementWrapper throws error with message built from the error code of the first function that failed. That said we should aim for something more general like: "Failed to get all messages to device".

This revision is now accepted and ready to land.Mar 5 2024, 1:27 AM
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
1721 ↗(On Diff #37786)

make sense, thanks for suggestion