search

agentm / project-m36

Project: M36 Relational Algebra Engine
The Unlicense
876 stars 47 forks source link

undefined when make a record type with two fields Atomable (improve error message) #234

Closed YuMingLiao closed 5 years ago

YuMingLiao commented 5 years ago

I was using Hair example to test if I can derive Atomable for record types.

data User = User
  { userFirstName :: Text
--  , userLastName :: Text
  } deriving (Eq, Ord, Show, Generic, NFData, Binary, Atomable)

a type with one field seems fine. But one with two fields cause this error: 

hair.hs: UnhandledExceptionError "Prelude.undefined\nCallStack (from HasCallStack):\n  error, called at libraries/base/GHC/Err.hs:79:14 in base:GHC.Err\n  undefined, called at ./src/lib/ProjectM36/Atomable.hs:59:43 in project-m36-0.6-9XTUV8wYedBJeDQmdZiQnD:ProjectM36.Atomable"
CallStack (from HasCallStack):
  error, called at hair.hs:33:23 in main:Main

data User = User
  { userFirstName :: Text
  , userLastName :: Text
--  , userEmail :: Text
u  } deriving (Eq, Ord, Show, Generic, NFData, Binary, Atomable)

 -- | Creates DatabaseContextExpr necessary to load the type constructor and data constructor into the database.
  toAddTypeExpr :: proxy a -> DatabaseContextExpr
  default toAddTypeExpr :: (Generic a, AtomableG (Rep a)) => proxy a -> DatabaseContextExpr
  toAddTypeExpr _ = toAddTypeExprG (from (undefined :: a)) (toAtomType (Proxy :: Proxy a))

It seems it crashes when toAddTypeExpr.

YuMingLiao commented 5 years ago

Okay, I guess that deriving Atomable isn't for record type since ,intuitively, record type is for Tupleable.

I was trying to make DbRecord a in project-m36-typed Tupeable. (a type that records createdtime, ...etc.) It seems its field dbRecordRecord :: a tries to store a record type in an atom if using deriving Tupleable. So that is why it requires a record type to be Atomable.

In the end, I use custom Tupleable typeclass to store fields of a and fields of DbRecord a in the same relation. It feels right to me. So I don't need record type to be Atomable now.

agentm commented 5 years ago

Nice- thanks for figuring out the issue.

The difference between Tupleable and Atomable may not always be obvious, so it would be good to improve the error message at least. I will re-open this ticket to replace the undefined failure with an error, at least.

agentm commented 5 years ago

I am not able to reproduce this. Note that test/Relation/Atomable.hs includes a record type test with two fields, so something else is going on here.

Could you provide the complete hair.hs with your changes?

agentm commented 5 years ago

Ah- I wasn't paying attention. The test does not cover toAddTypeExpr. I am able to reproduce this now.

YuMingLiao commented 5 years ago

Thanks for finding it out!

AgentM notifications@github.com 於 2019年3月28日 週四 上午10:22寫道：

Ah- I wasn't paying attention. The test does not cover toAddTypeExpr. I am able to reproduce this now.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/agentm/project-m36/issues/234#issuecomment-477419155, or mute the thread https://github.com/notifications/unsubscribe-auth/AcQgGRTyw_p_dtyb7XJ5PnBYmZv_5DBCks5vbCdPgaJpZM4cFb6f .