Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Question] Stance about key duplication #23

Open
kamoii opened this issue Nov 4, 2020 · 2 comments
Open

[Question] Stance about key duplication #23

kamoii opened this issue Nov 4, 2020 · 2 comments

Comments

@kamoii
Copy link
Contributor

kamoii commented Nov 4, 2020

I have a question about the stance of the library against key dupclication.

Currently, there is no key duplication check on construct, and the operators work even if there is a key duplication.
There is a metion in the README about fixing KeyDoesNotExist and RemoveAccessTo, which I guess you want to get rid of key duplication. But then there are operators like append that could bring in key duplication.
So which stance does the library have(or will have) against key duplication?

  1. Prevent key duplication by adding key check constraints on construct functions/instances and make sure operations don't bring in key duplication.
  2. Adding key check constraints are costly, so don't. Make the operation works even if there ia an accidentaly brought in key duplication.
@neongreen
Copy link
Contributor

Hey! The current thinking is that we will probably expose both operations that do try to prevent key duplication, and operations that do not. We might split them into several modules for convenience, eg. JRec.Lax and JRec.Strict or something along those lines.

It is important for Juspay's internal purposes that we have a way to avoid duplication checks at compile-time — we want fast compile times and happen to already have a guarantee that records have no duplicates. But I am not against having variants of functions that do not allow duplicates.

I think there are some details in #3.

@kamoii
Copy link
Contributor Author

kamoii commented Nov 5, 2020

@neongreen
Thanks for you reply!

I think I understand what you said. But I still have a concern about RemoveAccessTo.

RemoveAccessTo type family is only there to make operations work with key duplication.
But, if I understand correctly, it is causing RecApply, RecCopy, Eq and Ord instance to have compile cost O(n^2), which is as costly as key duplication check. Hense functions using these instance like inserting, JSON encoding, etc ... almost all of them except get, set and modify, has compile cost O(n^2). If we can get rid of RemoveAccessTo, we can improve compile time and also simplize the code greatly.

So are operators like inserting or JSON Encoding are going to have Lax and Strict version?
For example, Lax version of insert uses RecApply constraint which uses RemoveAccessTo type family, while Strict version of insert will use modified version of RecApply, like RecApplyStrict, which doesn't use RemoveAccessTo? If so, it gets kinda wiered since Lax version is meant to be less compile-costly version but other than constructing and appending, Strict version is less costly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants