Add mupserts and table unions to the state machine tests #475
Conversation
jorisdral
requested review from
dcoutts,
mheinzel,
recursion-ninja and
wenkokke
as code owners
jorisdral
force-pushed
the
jdral/single-api-qls
branch
from
0bfbac3
to
d51871a
Compare
jorisdral
force-pushed
the
jdral/single-api
branch
3 times, most recently
from
1a8d287
to
986bec6
Compare
jorisdral
force-pushed
the
jdral/single-api-qls
branch
from
d51871a
to
5f61aee
Compare
jorisdral
force-pushed
the
jdral/single-api
branch
2 times, most recently
from
7bedfcc
to
36b4a77
Compare
jorisdral
force-pushed
the
jdral/single-api
branch
2 times, most recently
from
ea4dd27
to
669e7f5
Compare
|
#469 was merged, so I'll update this PR before undrafting it |
jorisdral
force-pushed
the
jdral/single-api-qls
branch
from
5f61aee
to
d98f5ad
Compare
| , (10, fmap Some $ Inserts <$> genInserts <*> genTableVar) | ||
| , (10, fmap Some $ Deletes <$> genDeletes <*> genTableVar) | ||
| , (8, fmap Some $ Lookups <$> genLookupKeys <*> genTableVar) | ||
| , (4, fmap Some $ RangeLookup <$> genRange <*> genTableVar) |
There was a problem hiding this comment.
| , (4, fmap Some $ RangeLookup <$> genRange <*> genTableVar) | |
| , (4, fmap Some $ RangeLookup <$> genRange <*> genTableVar) |
| Database.LSMTree.Class | ||
| Database.LSMTree.Class.Common | ||
| Database.LSMTree.Class.Monoidal | ||
| Database.LSMTree.Class.Normal |
There was a problem hiding this comment.
So we added the unified API in #469 to allow writing tests against it. Here we add a third version of IsTable, the IO model etc. for the the unified API. But why not unify the model/tests as well, resulting in a single IO model and single IsTable class? That would make the tests much leaner.
The only argument against it that I can think of is that we're not yet sure which API(s) we want to provide in the end. If we later remove the unified API again, we might have to re-duplicate the model/tests. 🤷
There was a problem hiding this comment.
Sure, it would be nice if we could eventually deduplicate the test stuff too, but it's more work, and not strictly necessary for the statemachine tests
There was a problem hiding this comment.
Okay, but would be nice to discuss and tackle soonish then.
| @@ -46,6 +46,7 @@ import Data.List (sort) | |||
| import qualified Data.Primitive.ByteArray as BA | |||
| import qualified Data.Vector.Primitive as VP | |||
| import Data.Word | |||
| import qualified Database.LSMTree as LSMTree | |||
There was a problem hiding this comment.
To fit the pattern of Normal/Monoidal, would it make sense to import it as Unified here and in other places?
| R.Insert _ Nothing -> (i+1, iwb , d , m ) | ||
| R.Insert _ Just{} -> (i , iwb+1, d , m ) | ||
| R.Delete{} -> (i , iwb , d+1, m ) | ||
| R.Mupsert{} -> (i , iwb , d , m + 1) |
There was a problem hiding this comment.
| R.Insert _ Nothing -> (i+1, iwb , d , m ) | |
| R.Insert _ Just{} -> (i , iwb+1, d , m ) | |
| R.Delete{} -> (i , iwb , d+1, m ) | |
| R.Mupsert{} -> (i , iwb , d , m + 1) | |
| R.Insert _ Nothing -> (i+1, iwb , d , m ) | |
| R.Insert _ Just{} -> (i , iwb+1, d , m ) | |
| R.Delete{} -> (i , iwb , d+1, m ) | |
| R.Mupsert{} -> (i , iwb , d , m+1) |
This is a follow-up to #469. This PR is put in draft mode until we've come to a conclusion about the proposal in #469.
In this PR, we first create a class and reference implementation for the "full" API that is introduced in #469. Then, we switch the state machine tests to use this class instead of the
Normalclass. Finally, we can add infrastructure for both mupserts and table unions to the state machine tests. We start testing mupserts immediately, but table union operations are not yet generated because table union does not have an implementation yet.