agentm / project-m36

Project: M36 Relational Algebra Engine
The Unlicense
895 stars 48 forks source link

add Atomable NonEmpty #206

Closed YuMingLiao closed 6 years ago

YuMingLiao commented 6 years ago

hi @agentm

Thanks for the advice. I've add this feature.

I put a haskell file in example folder in case you want to try Atomable NonEmpty and see. You can delete it if not needed.

binary package with specific new version is needed for offering binary NonEmpty.

Take a look.

agentm commented 6 years ago

This looks good. I made some changes on a related branch. I added a test and renamed the type to NonEmptyList to be explicit. One we support modules, the names can better mimic the Haskell naming scheme.

Also, I changed the ADT to effectively be data NonEmptyList = NonEmptyList a (List a) which is inline with how Data.List.NonEmpty works. What do you think of that change?

YuMingLiao commented 6 years ago

Those details are important. Thanks for the help. That's great!

agentm commented 6 years ago

Great! I found a minor bug in the internal type resolution which causes the test to fail, so I need to find some time to work on that. Otherwise, this patch is ready-to-merge.

YuMingLiao commented 6 years ago

I fetched your branch, checkout and stack test. I saw the bug.

project-m36-0.4: test (suite: test-tutoriald)

### Failure in: 91                          
test/TutorialD/Interpreter.hs:610
non-empty list type construction
expected: Right (Relation [Attribute "a" (ConstructedAtomType "NonEmptyList" (fromList [("a",IntegerAtomType)])),Attribute "x" IntegerAtomType] (RelationTupleSet {asList = [RelationTuple [Attribute "a" (ConstructedAtomType "NonEmptyList" (fromList [("a",IntegerAtomType)])),Attribute "x" IntegerAtomType] [ConstructedAtom "NECons" (ConstructedAtomType "NonEmptyList" (fromList [("a",IntegerAtomType)])) [IntegerAtom 3,ConstructedAtom "Cons" (ConstructedAtomType "List" (fromList [("a",IntegerAtomType)])) [IntegerAtom 4,ConstructedAtom "Empty" (ConstructedAtomType "List" (fromList [("a",IntegerAtomType)])) []]],IntegerAtom 3]]}))
 but got: Right (Relation [Attribute "a" (ConstructedAtomType "NonEmptyList" (fromList [("a",IntegerAtomType)])),Attribute "x" IntegerAtomType] (RelationTupleSet {asList = [RelationTuple [Attribute "a" (ConstructedAtomType "NonEmptyList" (fromList [("a",IntegerAtomType)])),Attribute "x" IntegerAtomType] [ConstructedAtom "NECons" (ConstructedAtomType "NonEmptyList" (fromList [("a",IntegerAtomType)])) [IntegerAtom 3,ConstructedAtom "Cons" (ConstructedAtomType "List" (fromList [("a",IntegerAtomType)])) [IntegerAtom 4,ConstructedAtom "Empty" (ConstructedAtomType "List" (fromList [])) []]],IntegerAtom 3]]}))
Cases: 92  Tried: 92  Errors: 0  Failures: 1

Empty in "x:=relation{tuple{a NECons 3 (Cons 4 Empty)}} : {x:=nonEmptyListHead(@a)}" creates ConstructedAtom "Empty" (ConstructedAtomType "List" (fromList [("a",IntegerAtomType)])) []

It seems we can't infer Empty' in NECons 3 (Cons 4 Empty) to be ListAtomType IntegerAtomType

agentm commented 6 years ago

Thanks for this patch- it helped me to identify some additional bugs in the type system which allowed unresolved types to be used in relation creation.

YuMingLiao commented 6 years ago

My pleasure! And thanks for helping me completing this patch!