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

Request to reintroduce Key/Keys interfaces #93

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

sGy1980de
Copy link

Hey guys,

we at esome really love your work. When we wanted to update this lib to v7, we stumbled over one problem. In our scenario, we use the data-loader to optimize our SQL queries also for parametrized GraphQL edges.
Therefore, our current keys do not fulfill the comparable constraint. With the Key interface, this wasn't a problem, since our keys simply exported a unique identifier according to their parameters.

That's why I took a step back, and reintroduced the Key interface backed with type parameters this time. So, basically the best from both worlds. Please let me know what you think about it. I'm not sure, about the parameter in the BatchFunc though, but I decided to keep the API clean (Key/Keys everywhere) in the first place. But this might be subject to change, since there is no real need for the Key there.

@eaglemoor
Copy link

eaglemoor commented Apr 3, 2023

Вad idea, in go v1.18+ comparable is just used to describe key types that you can compare. All other libraries support this paradigm and if you rewrite the library to [any, any] you can get a lot of integration problems.

We have the same problem too, but we go to another way. U can use some comparable interface and struct (go v1.20).

type Key[K comparable] interface{
    Raw() K
    String() string
}

You can see my code eaglemoor#1 . In it, I'm trying to fix 2 problems:

  1. Forward real context for each key to save tracing
  2. Name possibility to use non-memory cache

@sGy1980de
Copy link
Author

sGy1980de commented May 12, 2023

Вad idea, in go v1.18+ comparable is just used to describe key types that you can compare.

And exactly this is my problem. My identifier contain types, considered not comparable by the compiler. With the interface, this not a problem. But I cannot come up with a data-structure to fulfill the comparable constraint in my scenario. The Key interface solves this problem by placing the responsibility for comparability in the implementing types.

@pavelnikolov
What do you think, if I would come up with this Key stuff as an option? So basically a parallel interface/implementation, keeping the original v7 implementation, but also providing the old Key semantics, but using generics this time, basically what this PR is currently.

This would for sure mean some major code-duplication, which could be somewhat addressed by static code generation. But I would not like to invest the effort, if this has zero chance, to be merged upstream.

@pavelnikolov
Copy link
Member

I'm happy to go with an interface or other solution but I want to understand the problem first. Could you, please, give me a very minimalistic example which is broken or doesn't work.

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

Successfully merging this pull request may close these issues.

3 participants