graph-gophers / dataloader

Implementation of Facebook's DataLoader in Golang
MIT License
1.2k stars 75 forks source link

Request to reintroduce Key/Keys interfaces #93

Open sGy1980de opened 2 years ago

sGy1980de commented 2 years ago

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 commented 1 year ago

В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 https://github.com/eaglemoor/dataloader/pull/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 commented 1 year ago

В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 commented 8 months ago

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.