diesel icon indicating copy to clipboard operation
diesel copied to clipboard

Experiment with a Queryable implementation that does take field names in account

Open weiznich opened this issue 5 years ago • 4 comments

So first the important things: I'm totally aware that code is currently not nicely structured or documented, that needs to be fixed before we think about merging this into diesel. Currently this is only a experiment to test some things out and maybe get some feedback if we really want something like this.

Why?

A commonly mistake seems to be that people assume that deriving Queryable maps field by name and not by indices. This experiment is a try to provide something that is more in line with that expectation.

How?

The idea is basically the same as that one behind frunk's labelled generics implementation, so I just link their description . There are a few modifications to adapt that into something that could be used to implement this feature in such a way that actual loading continues to use indices internally. The mapping is done entirely at type after that.

What is missing

Probably a lot

Open Questions:

  • [ ] How do we handle boxed queries?
  • [ ] This currently incoperates only field names, not table names. So this will fail as soon as a query involve fields with the same name from multiple tables. Maybe this is fixable
  • [ ] Do we event want this thing?
  • [ ] Is it ok to depend on frunk that way and convert forth and back to hlists
  • [ ] How to incorporate this with LoadDsl?
  • [ ] How to verify that this really optimized away?

Additional points to fix if we want this:

  • [ ] Implement FromHlist for bigger Hlist's
  • [ ] Cleanup
  • [ ] Documentation
  • [ ] More tests (specifically compile tests)

weiznich avatar Apr 17 '19 22:04 weiznich

This is very nice. I am assuming this will fix the issue of out of order fields with same type.

mmrath avatar Apr 24 '19 10:04 mmrath

@diesel-rs/core Any opinions about this?

weiznich avatar May 06 '19 22:05 weiznich

I kinda like the idea behind it. My main problem with it is how do we handle the fact that a column is misnamed ? Also Queryable can usually lie and the same structure could be used with multiple tables that have different names (like postfixed or prefixed).

Eijebong avatar May 06 '19 22:05 Eijebong

My main problem with it is how do we handle the fact that a column is misnamed ?

I'm not sure what you mean exactly by "a column is misnamed".

  • Do you mean the field name in the struct does not match the name of the field in the query? That results in a compile error (that does not look that nice, but contains the expect and provided name in some gabbled form).
  • Do you mean that the struct field should have a different name than the returned column? That could probably be solved by some attribute added to the custom derive.

Also Queryable can usually lie and the same structure could be used with multiple tables that have different names (like postfixed or prefixed).

I don't think removing Queryable as it exists today is a good idea. This approach should be added as an additional option in my option. Queryable has clear use cases and should continue to exist as independent option. To only thing that may change is that some documentation like the beginner guide may point to this new trait/derive first.

weiznich avatar May 07 '19 09:05 weiznich