Add COUNT(DISTINCT ...), SUM(DISTINCT ...), AVG(DISTINCT ...)

[vivekmenezes]

closes #2248

[tamird]

pretty sure in this case, you're just as well passing nil

[tamird]

typo in commit message: s/ADD/SUM/

[petermattis]

Rather than a separate seen slice that parallels the funcs slice, could you place this map inside aggregateFunc?

[petermattis]

LGTM

Seems like there are some opportunities to optimize distinct processing given the ordering, similar to what is done for SELECT DISTINCT.

[vivekmenezes]

I've incorporated most of your feedback and added another commit that encodes tuples for SELECT DISTINCT. Thanks.

[tamird]

This unnecessarily allocates a map even when it's unused. How about:

f := &aggregateFunc{
    val: aggregateValue{
        FuncExpr: t,
    },
    impl: impl.New(),
}
if t.Distinct {
    f.seen = make(map[string]struct{})
}

[vivekmenezes]

sounds good

On Mon, Sep 21, 2015 at 3:12 PM Tamir Duberstein notifications@github.com
wrote:

In sql/group.go
#2568 (comment):

@@ -196,11 +196,17 @@ func (v *extractAggregatesVisitor) Visit(expr parser.Expr, pre bool) (parser.Vis
break
}
if impl, ok := aggregates[strings.ToLower(string(t.Name.Base))]; ok {

•       seen := make(map[string]struct{})
  

This unnecessarily allocates a map even when it's unused. How about:

f := &aggregateFunc{
val: aggregateValue{
FuncExpr: t,
},
impl: impl.New(),
}if t.Distinct {
f.seen = make(map[string]struct{})
}

—
Reply to this email directly or view it on GitHub
https://github.com/cockroachdb/cockroach/pull/2568/files#r40012456.

[tamird]

There's still a typo in the commit message.

Whoops, no there isn't!

LGTM modulo the extra map allocation.

[petermattis]

LGTM

[vivekmenezes]

I was struggling to find a query that will return a tuple and not return an error. The test I have used to return a different error before the change.

SELECT (2, 3) IN (SELECT DISTINCT (y, z) FROM xyz) doesn't work
nor does
SELECT ((2, 3)) IN (SELECT DISTINCT (y, z) FROM xyz)

If we don't have a query available, I'll create a bug report and fix it in a different PR.

[petermattis]

Hmm, SELECT (2, 3) IN (SELECT DISTINCT (y, z) FROM xyz) should work. I'll take a look once this is merged.

[vivekmenezes]

Looks like I messed up the commits. Let me put the right changes in the right commits

[tamird]

I think you can just squash these to one.