Page MenuHomePhabricator

[web-db] implement function get all drafts from SQLite
ClosedPublic

Authored by kamil on Mar 21 2023, 8:22 AM.
Tags
None
Referenced Files
F3556378: D7123.id23985.diff
Thu, Dec 26, 11:39 PM
F3555714: D7123.id24364.diff
Thu, Dec 26, 11:22 PM
F3543141: D7123.diff
Thu, Dec 26, 10:02 AM
Unknown Object (File)
Tue, Dec 24, 10:25 PM
Unknown Object (File)
Tue, Dec 17, 4:49 AM
Unknown Object (File)
Mon, Dec 16, 12:16 PM
Unknown Object (File)
Sat, Dec 14, 8:49 PM
Unknown Object (File)
Sat, Dec 14, 2:25 AM
Subscribers

Details

Summary

Function to return all drafts, native version: link

Depends on D7122

Test Plan

Tests D7124

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 23 2023, 6:14 AM
ashoat added inline comments.
web/database/queries/draft-queries.js
43 ↗(On Diff #24022)
  1. I think it's more clear / readable to check dbResult.length === 0
  2. In what scenario would dbResult.length === 0? Looking at D7118, it seems like the first array here should have on result for each statement in the query. But the query always has precisely one statement, so wouldn't we expect dbResult.length === 1? Should we use an invariant instead of checking this here?
This revision is now accepted and ready to land.Mar 23 2023, 12:48 PM
web/database/queries/draft-queries.js
43 ↗(On Diff #24022)
  1. I think it's more clear / readable to check dbResult.length === 0

You're right, I'll address this before landing

  1. In what scenario would dbResult.length === 0? Looking at D7118, it seems like the first array here should have on result for each statement in the query. But the query always has precisely one statement, so wouldn't we expect dbResult.length === 1? Should we use an invariant instead of checking this here?

It's non-standard sql.js behavior, I added a comment in D7118 (above the function to warn developers in the future).
Imagine that the drafts table is empty, in that case, you would expect (because it's one query) the result to look like this: [[]] or [undefined].
However, sql.js skips empty results, so rawDBResult variable will be [], which implies dbResult will also be [].
I don't think invariant is suitable, dbResult.length === 0 it's sql.js quirk but it's a legal situation.

web/database/queries/draft-queries.js
43 ↗(On Diff #24022)

Thanks for explaining! That is weird behavior but your approach makes sense