Skip to content

Commit

Permalink
Fix union-types when aggregating on inline conversion function (elast…
Browse files Browse the repository at this point in the history
…ic#110652)

A query like:

```
FROM sample_data, sample_data_str
| STATS count=count(*) BY client_ip = TO_IP(client_ip)
| SORT count DESC, client_ip ASC
| KEEP count, client_ip
```

Failed due to unresolved aggregates from the union-type in the grouping key
  • Loading branch information
craigtaverner authored Jul 10, 2024
1 parent bdedced commit 4ddca13
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,21 @@ mc:l | count:l
7 | 2
;

multiIndexTsLongStatsStats
required_capability: union_types
required_capability: union_types_agg_cast

FROM sample_data, sample_data_ts_long
| EVAL ts = TO_STRING(@timestamp)
| STATS count = COUNT(*) BY ts
| STATS mc = COUNT(count) BY count
| SORT mc DESC, count ASC
;

mc:l | count:l
14 | 1
;

multiIndexTsLongRenameStats
required_capability: union_types

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,12 @@ public enum Cap {
* Fix to GROK validation in case of multiple fields with same name and different types
* https://github.com/elastic/elasticsearch/issues/110533
*/
GROK_VALIDATION;
GROK_VALIDATION,

/**
* Fix for union-types when aggregating over an inline conversion with conversion function. Done in #110652.
*/
UNION_TYPES_INLINE_FIX;

private final boolean snapshotOnly;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1088,15 +1088,19 @@ protected LogicalPlan doRule(LogicalPlan plan) {

// In ResolveRefs the aggregates are resolved from the groupings, which might have an unresolved MultiTypeEsField.
// Now that we have resolved those, we need to re-resolve the aggregates.
if (plan instanceof EsqlAggregate agg && agg.expressionsResolved() == false) {
if (plan instanceof EsqlAggregate agg) {
// If the union-types resolution occurred in a child of the aggregate, we need to check the groupings
plan = agg.transformExpressionsOnly(FieldAttribute.class, UnresolveUnionTypes::checkUnresolved);

// Aggregates where the grouping key comes from a union-type field need to be resolved against the grouping key
Map<Attribute, Expression> resolved = new HashMap<>();
for (Expression e : agg.groupings()) {
Attribute attr = Expressions.attribute(e);
if (attr != null && attr.resolved()) {
resolved.put(attr, e);
}
}
plan = agg.transformExpressionsOnly(UnresolvedAttribute.class, ua -> resolveAttribute(ua, resolved));
plan = plan.transformExpressionsOnly(UnresolvedAttribute.class, ua -> resolveAttribute(ua, resolved));
}

// Otherwise drop the converted attributes after the alias function, as they are only needed for this function, and
Expand Down Expand Up @@ -1222,9 +1226,8 @@ protected LogicalPlan rule(LogicalPlan plan) {
return plan.transformExpressionsOnly(FieldAttribute.class, UnresolveUnionTypes::checkUnresolved);
}

private static Attribute checkUnresolved(FieldAttribute fa) {
var field = fa.field();
if (field instanceof InvalidMappedField imf) {
static Attribute checkUnresolved(FieldAttribute fa) {
if (fa.field() instanceof InvalidMappedField imf) {
String unresolvedMessage = "Cannot use field [" + fa.name() + "] due to ambiguities being " + imf.errorMessage();
return new UnresolvedAttribute(fa.source(), fa.name(), fa.qualifier(), fa.id(), unresolvedMessage, null);
}
Expand Down

0 comments on commit 4ddca13

Please sign in to comment.