debug-ito / greskell

Haskell binding for Gremlin graph query language
https://hackage.haskell.org/package/greskell
27 stars 4 forks source link

`liftWalk` hell #15

Closed gardspirito closed 1 year ago

gardspirito commented 1 year ago

This repo has a great DSL, but every time I write some function that returns a Walk (especially abstract over WalkType) it becomes a huge pain to match the walk type, because it's easy to get lost in all the ', liftWalk, >>>, etc, so I'm wondering if it's me doing something wrong.

Thank you.

debug-ito commented 1 year ago

Thanks for raising this issue. I understand. I think it's like we need a lot of liftIO and/or lift when we deal with a complicated monad stack.

Honestly, I don't use greskell so much. I'd like to hear feedback from other users, too.

gardspirito commented 1 year ago

Well, in my exact project I didn't find walk type annotation very useful, but, nonetheless, maybe there is a better way to express it in newer GHC versions. For example, I was thinking of:

class WalkType w => WalkTransform w
class WalkTransform w => WalkEffect w

instance WalkTransform Transform
instance WalkTransform SideEffect
instance WalkEffect SideEffect

# A possible replacement for Split
data Subwalk a = Subwalk a
instance WalkType a => WalkType (Subwalk a)
instance WalkType a => WalkTransform (Subwalk a)
instance WalkEffect a => WalkEffect (Subwalk a)

... and so:

someFilterWalk :: ... -> Walk Filter a b
-- Turns into:
someFilterWalk:: WalkType c => ... -> Walk c a b

someTransformWalk :: ... -> Walk Transform a b
-- Turns into:
someTransformWalk:: WalkTransform c => ... -> Walk c a b

someEffectfulWalk :: ... -> Walk SideEffect a b
-- Turns into:
someEffectfulWalk:: WalkEffect c => ... -> Walk c a b

someBranching :: Split c p => Walk c a b -> Walk p a b
-- Turns into:
someBranching :: WalkType p => Walk (Subwalk p) a b -> Walk p a b

I'm pretty sure that my design doesn't work, but something like this could help avoid lifts and splits.

debug-ito commented 1 year ago

Hmm, I think the constraint (WalkTransform w) is equivalent to (Lift Transform w), which is already defined in greskell.

I think the real cause of the issue is that most of the traversal functions in Data.Greskell.GTraversal are monomorphic on walk types. For example, gOrder has the signature

gOrder :: [ByComparator s] -> Walk Transform s s

instead of

gOrder :: (WalkType c, Lift Transform c) => [ByComparator s] -> Walk c s s

so you have to use liftWalk to combine it with SideEffect walks.

Well, some Filter-type traversals have both polymorphic and monomorphic versions (e.g. gIs and gIs'), but Transform-type traversals are almost all monomorphic. Sorry for such an inconsistent API.

Just changing monomorphic traversals into polymorphic ones would be a breaking change. Some code would fail to compile due to type ambiguity. I want to avoid that.

Maybe I should add a new module, say Data.Greskell.GTraversal.Gen, which contains generalized (polymorphic) versions of the traversals with the same names as in Data.Greskell.GTraversal. What do you think about it?

gardspirito commented 1 year ago

Maybe I should add a new module, say Data.Greskell.GTraversal.Gen

I think that would be great! But I'm not sure what others think.

debug-ito commented 1 year ago

I just released greskell-2.0.3.0, which includes the GTraversal.Gen module. Just try it and let me know if it solves your issue.

gardspirito commented 1 year ago

Sorry, deadlines suddenly hit, but now I finally have time to test it.

... And it's great. The complexity of the code is reduced by a huge margin. But now I had to use custom helper function tW :: Walk Transform a b -> Walk Transform a b in many places to resolve the inference issue. For my small DB interface of 480 sloc it took 18 uses of this tW helper. Which is pretty okay.

Problems for me were with gProject, gByL, gRepeat, gChoose3, gFrom. They weren't able to infer WalkType of a subwalk, but that's a minor issue.

debug-ito commented 1 year ago

Thanks for feedback. I'm glad it helped.