Aggregation Planner doesn't handle Literals properly

[vmg]

Here's an interesting bug that I found. For a SQL query like:

SELECT t1.name, CAST(SUM(b.dec) AS DECIMAL(20,6)) AS dec FROM t2 as b LEFT JOIN t1 ON b.exid = t1.t1_id GROUP BY b.exid`)

The CAST expression results in the wrong return value: the result of the SUM aggregation is casted to DECIMAL(0, 0), so it has no decimal precision after the period.

Why? Well, the planner aggregator in extractAggr is being too eager with the 20 and 6 literals inside the CAST.

vitess/go/vt/vtgate/planbuilder/operators/queryprojection.go

Lines 512 to 536 in bcfc5fb

	return func(node, parent sqlparser.SQLNode) bool {
	ex, isExpr := node.(sqlparser.Expr)
	if !isExpr {
	return true
	}
	if aggr, isAggr := node.(sqlparser.AggrFunc); isAggr {
	ae := aeWrap(aggr)
	if aggr == aliasedExpr.Expr {
	ae = aliasedExpr
	}
	aggrFunc := createAggrFromAggrFunc(aggr, ae)
	aggrFunc.Index = &idx
	addAggr(aggrFunc)
	return false
	}
	if containsAggr(node) {
	makeComplex()
	return true
	}
	if !qp.isExprInGroupByExprs(ctx, ex) {
	aggr := NewAggr(opcode.AggregateAnyValue, nil, aeWrap(ex), "")
	aggr.Index = &idx
	addAggr(aggr)
	}
	return false

The planner finds a *CastExpr and iterates it, but when recursing into it, it considers 20 and 6, which are both *sqlparser.Literal, as part of the aggregation, and generates an AggregateAnyValue for each of the literals.

This behavior makes sense for a compound expression like SELECT 3 * SUM(b.dec), but generating a new Any Aggregation for these literals is not correct. It results in very unexpected results: in this specific bug, since we now have a literal 20 as an aggregated expression for the query, when the planner performs the offset rewrites, it'll try to replace CAST(... AS DECIMAL(20,6)) with CAST(... AS DECIMAL(:offset_5,:offset_6)), but since the *CastExpr only holds *Literal and not sqlparser.Expr, it'll result in CAST(... AS DECIMAL(nil, nil)), and remove the decimal precision.

Ouchie!

@systay: I don't know how to fix this! We could special-case *CastExpr to ensure we don't consider its *Literal fields and extract them into aggregated expressions, but surely this is not scalable. There are other SQL functions that can be applied on top of aggregations and that contain literals that should not be extracted.