diesel-rs / diesel

A safe, extensible ORM and Query Builder for Rust
https://diesel.rs
Apache License 2.0
12.74k stars 1.07k forks source link

Allow selecting multiple aggregate columns #3

Closed sgrif closed 4 years ago

sgrif commented 9 years ago

I had hoped that this would just involve adding an Aggregate marker trait, and modifying the tuple impls to allow if all members are aggregate. This causes overlap since any type could implement both NonAggregate and Aggregate. I do not think there is a way to express this currently in the type system currently, without negative bounds (or maybe specialization)

sgrif commented 8 years ago

@aturon @nikomatsakis Was thinking about this recently, and thought you might be interested as it would have been helped by the lattice rule, but could be aided by other additions to the type system as well. Basically we have an impl that looks like this for every size tuple:

impl<T, U> Expression for (T, U) where
    T: Expression + NonAggregate,
    U: Expression + NonAggregate,
{
    // ...
}

What we need is to write something along the lines of:

impl<T, U> Expression for (T, U) where
    T: Expression + !NonAggregate,
    U: Expression + !NonAggregate,
{
    // ...
}

With the lattice rule we could have added an Aggregate trait, and then had an impl for T: Expression + Aggregate + NonAggregate with a body that would force it to fail to compile if it was ever used. Ultimately though, what we really need here is either negative reasoning, or mutual exclusion.

The main goal (in case there's another solution that I'm missing) is to make sure that we do not have an impl for T: Aggregate, U: NonAggregate.

sgrif commented 8 years ago

Also I know you had asked me to keep you in the loop when I find some concrete examples to point you guys at. Is there a preferred form of communication besides just pinging you on issues like this?

nikomatsakis commented 8 years ago

@sgrif this seems good. The hard part I guess might be finding these examples later -- I'll try to keep a log. For what it's worth, this would probably be addressed by the semi-proposal for negative reasoning I advanced in this comment, which itself is just a variant on rust-lang/rfcs#1148.

aturon commented 8 years ago

@sgrif Thanks for the heads up! In general issue comments like this are fine -- perhaps you could have a meta-issue with links to various examples as they come up, just so there's ultimately one place to track?

Pyriphlegethon commented 8 years ago

This problem could possibly be solved by using a PhantomData marker similar to these two examples: ebfull/pcap and hyperium/hyper.

weiznich commented 7 years ago

When supporting multiple aggregate columns we should consider that there could be queries that mix aggregate and non-aggregate fields. See for example the following query:

SELECT a.id, count(b::id) FROM a LEFT JOIN b ON a.id = b.a_id GROUP BY a.id;
weiznich commented 4 years ago

Closed by #2251