Skip to content

Commit 377328c

Browse files
astefanastefan
committed
More fixes and tests
1 parent fa22575 commit 377328c

File tree

5 files changed

+145
-32
lines changed

5 files changed

+145
-32
lines changed

‎x-pack/plugin/esql/qa/testFixtures/src/main/resources/inlinestats.csv-spec‎

Lines changed: 115 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ emp_no:integer |avg_worked_seconds:long|avg_avg_worked_seconds:double|languages:
218218
10007 |393084805 |2.863684210555556E8 |4 |5 |F
219219
;
220220

221-
three-Ignore
221+
three
222222
required_capability: inlinestats_v8
223223
// fails with AssertionError at org.elasticsearch.xpack.esql.plan.logical.Limit.writeTo(Limit.java:70)
224224

@@ -312,16 +312,28 @@ FROM airports
312312
// end::extreme-airports-result[]
313313
;
314314

315-
brokenwhy-Ignore
315+
mvMinMvExpand
316316
required_capability: join_planning_v1
317+
required_capability: inlinestats_v8
317318

318319
FROM airports
320+
| EVAL original_type = type
319321
| INLINESTATS min_scalerank=MIN(scalerank) BY type
320322
| MV_EXPAND type
321-
| WHERE scalerank == MV_MIN(scalerank);
323+
| EVAL mvMin_scalerank = MV_MIN(scalerank)
324+
| WHERE scalerank == MV_MIN(scalerank)
325+
| SORT abbrev DESC NULLS LAST
326+
| LIMIT 7
327+
;
322328

323-
abbrev:keyword | type:keyword | scalerank:integer | min_scalerank:integer
324-
GWL | [mid, military] | 9 | [2, 4]
329+
abbrev:keyword |city:keyword |city_location:geo_point |country:keyword| location:geo_point | name:text |scalerank:integer|original_type:keyword |min_scalerank:integer|type:keyword|mvMin_scalerank:integer
330+
ZRH |Zürich |POINT (8.5411 47.3744) |Switzerland |POINT (8.56221279534765 47.4523895064915) |Zurich Int'l |3 |major |2 |major |3
331+
ZNZ |Zanzibar |POINT (39.199 -6.165) |Tanzania |POINT (39.2223319841558 -6.21857034620282)|Zanzibar |4 |mid |2 |mid |4
332+
ZLO |Cihuatlán |POINT (-104.5667 19.25) |Mexico |POINT (-104.560095200097 19.1480860285854)|Playa de Oro Int'l |7 |mid |2 |mid |7
333+
ZHHH |Wuhan |POINT (114.2881 30.5872)|China |POINT (114.24694737615 30.6017141196702) |Wang-Chia Tun Airbase|6 |[mid, military] |[2, 4] |mid |6
334+
ZHHH |Wuhan |POINT (114.2881 30.5872)|China |POINT (114.24694737615 30.6017141196702) |Wang-Chia Tun Airbase|6 |[mid, military] |[2, 4] |military |6
335+
ZGC |Lanzhou |POINT (103.8318 36.0617)|China |POINT (103.615415363043 36.5078842461237) |Lanzhou Zhongchuan |6 |mid |2 |mid |6
336+
ZAR |Zaria |POINT (7.7 11.0667) |Nigeria |POINT (7.68726764310577 11.1352958601071) |Zaria |8 |mid |2 |mid |8
325337
;
326338

327339
afterStats
@@ -627,11 +639,10 @@ abbrev:keyword | scalerank:integer | location:geo_point
627639
ZLO | 7 | POINT (-104.560095200097 19.1480860285854) | 2 | 20 | -100
628640
;
629641

630-
byTwoCalculatedSecondOverwrites-Ignore
642+
byTwoCalculatedSecondOverwrites
631643
required_capability: join_planning_v1
632644
required_capability: stats_alias_collision_warnings
633-
// fails with
634-
// line 1:78: Plan [InlineJoin[LEFT,[x{r}#20, x{r}#23],[x{r}#20, x{r}#23],[x{r}#23, x{r}#23]]] optimized incorrectly due to missing references from left hand side [x{r}#20]
645+
required_capability: inlinestats_v8
635646

636647
FROM airports
637648
| WHERE abbrev IS NOT NULL
@@ -644,18 +655,16 @@ FROM airports
644655
;
645656
warning:Line 5:4: Field 'x' shadowed by field at line 6:3
646657

647-
abbrev:keyword | scalerank:integer | location:geo_point | x:double | min_sl:integer
648-
ZRH | 3 | POINT(8.56221279534765 47.4523895064915) | 10 | 2
649-
ZNZ | 4 | POINT (39.2223319841558 -6.21857034620282) | 40 | 2
650-
ZLO | 7 | POINT (-104.560095200097 19.1480860285854) | -100 | 2
658+
abbrev:keyword | scalerank:integer | location:geo_point | min_sl:integer| x:double
659+
ZRH | 3 | POINT (8.56221279534765 47.4523895064915) | 2 | 10
660+
ZNZ | 4 | POINT (39.2223319841558 -6.21857034620282) | 2 | 40
661+
ZLO | 7 | POINT (-104.560095200097 19.1480860285854) | 2 | -100
651662
;
652663

653-
byTwoCalculatedSecondOverwritesReferencingFirst-Ignore
664+
byTwoCalculatedSecondOverwritesReferencingFirst
654665
required_capability: join_planning_v1
655666
required_capability: stats_alias_collision_warnings
656-
// fails with
657-
// line 1:105: Plan [Aggregate[[x{r}#45],[MIN(scalerank{f}#50,true[BOOLEAN]) AS min_sl, x{r}#45]]] optimized incorrectly due to missing references [x{r}#45]
658-
// line 1:105: Plan [InlineJoin[LEFT,[x{r}#42, x{r}#45],[x{r}#42, x{r}#45],[x{r}#45, x{r}#45]]] optimized incorrectly due to missing references from left hand side [x{r}#42]
667+
required_capability: inlinestats_v8
659668

660669
FROM airports
661670
| WHERE abbrev IS NOT NULL
@@ -669,35 +678,33 @@ FROM airports
669678
;
670679
warning:Line 6:4: Field 'x' shadowed by field at line 7:3
671680

672-
abbrev:keyword | scalerank:integer | location:geo_point | x:double | min_sl:integer
673-
ZRH | 3 | POINT(8.56221279534765 47.4523895064915) | 10 | 2
674-
ZNZ | 4 | POINT (39.2223319841558 -6.21857034620282) | 40 | 2
675-
ZLO | 7 | POINT (-104.560095200097 19.1480860285854) | -100 | 2
681+
abbrev:keyword | scalerank:integer | location:geo_point | min_sl:integer| x:double
682+
ZRH | 3 | POINT (8.56221279534765 47.4523895064915) | 2 | 10
683+
ZNZ | 4 | POINT (39.2223319841558 -6.21857034620282) | 2 | 40
684+
ZLO | 7 | POINT (-104.560095200097 19.1480860285854) | 2 | -100
676685
;
677686

678687

679-
groupShadowsAgg-Ignore
688+
groupShadowsAgg
680689
required_capability: join_planning_v1
681690
required_capability: stats_alias_collision_warnings
682-
// fails with
683-
// line 1:134: column [location] cannot be used as an aggregate once declared in the STATS BY grouping key [lat_10 = ROUND(ST_Y(location), -1)]
691+
required_capability: inlinestats_v8
684692

685693
FROM airports
686694
| WHERE abbrev IS NOT NULL
687695
| KEEP abbrev, scalerank, location
688696
| INLINESTATS min_sl=MIN(scalerank)
689-
, lat_10 = ROUND(ST_Y(location), -1)
690-
BY lat_10 = ROUND(ST_Y(location), -1)
697+
BY min_sl = ROUND(ST_Y(location), -1)
691698
, lon_10 = ROUND(ST_X(location), -1)
692699
| SORT abbrev DESC
693700
| LIMIT 3
694701
;
695-
warning:Line 5:3: Field 'lat_10' shadowed by field at line 6:4
702+
warning:Line 4:15: Field 'min_sl' shadowed by field at line 5:4
696703

697-
abbrev:keyword | scalerank:integer | location:geo_point | lat_10:double | lon_10:double | min_sl:integer
698-
ZRH | 3 | POINT(8.56221279534765 47.4523895064915) | 50 | 10 | 2
699-
ZNZ | 4 | POINT (39.2223319841558 -6.21857034620282) | -10 | 40 | 4
700-
ZLO | 7 | POINT (-104.560095200097 19.1480860285854) | 20 | -100 | 2
704+
abbrev:keyword | scalerank:integer | location:geo_point | min_sl:double | lon_10:double
705+
ZRH | 3 | POINT(8.56221279534765 47.4523895064915) | 50 | 10
706+
ZNZ | 4 | POINT (39.2223319841558 -6.21857034620282) | -10 | 40
707+
ZLO | 7 | POINT (-104.560095200097 19.1480860285854) | 20 | -100
701708
;
702709

703710
groupShadowsField
@@ -1106,3 +1113,81 @@ null |25324
11061113
F |25324
11071114
M |25324
11081115
;
1116+
1117+
renamingGroupingWithItself
1118+
required_capability: inlinestats_v8
1119+
1120+
FROM employees
1121+
| EVAL x = gender
1122+
| INLINESTATS min_sl = MIN(salary) BY x = x
1123+
| SORT salary DESC
1124+
| KEEP salary, x, gender, min_sl, emp_no
1125+
| LIMIT 5
1126+
;
1127+
1128+
salary:integer |x:keyword|gender:keyword |min_sl:integer |emp_no:integer
1129+
74999 |M |M |25945 |10029
1130+
74970 |M |M |25945 |10045
1131+
74572 |F |F |25976 |10007
1132+
73851 |F |F |25976 |10027
1133+
73717 |null |null |25324 |10019
1134+
;
1135+
1136+
overridingGroupings
1137+
required_capability: inlinestats_v8
1138+
1139+
FROM employees
1140+
| INLINESTATS min_sl = MIN(salary) BY x = gender, x = languages
1141+
| KEEP salary, x, gender, min_sl, emp_no
1142+
| SORT salary
1143+
| LIMIT 5
1144+
;
1145+
warning:Line 2:39: Field 'x' shadowed by field at line 2:51
1146+
1147+
salary:integer |x:integer |gender:keyword |min_sl:integer |emp_no:integer
1148+
25324 |5 |null |25324 |10015
1149+
25945 |5 |M |25324 |10035
1150+
25976 |1 |F |25976 |10092
1151+
26436 |3 |M |26436 |10048
1152+
27215 |4 |F |27215 |10057
1153+
;
1154+
1155+
overridingExpressionGroupings
1156+
required_capability: inlinestats_v8
1157+
1158+
FROM employees
1159+
| INLINESTATS min_sl = MIN(salary) BY x = TO_LOWER(gender), x = CONCAT(gender, gender)
1160+
| SORT salary DESC
1161+
| KEEP salary, x, gender, min_sl, emp_no
1162+
| LIMIT 5
1163+
;
1164+
warning:Line 2:39: Field 'x' shadowed by field at line 2:61
1165+
1166+
salary:integer |x:keyword |gender:keyword |min_sl:integer |emp_no:integer
1167+
74999 |MM |M |25945 |10029
1168+
74970 |MM |M |25945 |10045
1169+
74572 |FF |F |25976 |10007
1170+
73851 |FF |F |25976 |10027
1171+
73717 |null |null |25324 |10019
1172+
;
1173+
1174+
reusingEvalExpressions_UsedInGroupings
1175+
required_capability: inlinestats_v8
1176+
1177+
FROM employees
1178+
| KEEP salary, gender, emp_no
1179+
| EVAL x = TO_LOWER(gender), x = CONCAT(x, " ", x)
1180+
| INLINESTATS min_sl = MIN(salary) BY x
1181+
| SORT salary DESC
1182+
| LIMIT 5
1183+
;
1184+
1185+
salary:integer |gender:keyword |emp_no:integer |min_sl:integer | x:keyword
1186+
74999 |M |10029 |25945 |m m
1187+
74970 |M |10045 |25945 |m m
1188+
74572 |F |10007 |25976 |f f
1189+
73851 |F |10027 |25976 |f f
1190+
73717 |null |10019 |25324 |null
1191+
;
1192+
1193+

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateInlineEvals.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323

2424
/**
2525
* Replace any evaluation from the inlined aggregation side (right side) to the left side (source) to perform the matching.
26-
* In INLINE m = MIN(x) BY a + b the right side contains STATS m = MIN(X) BY a + b.
26+
* In INLINESTATS m = MIN(x) BY a + b the right side contains STATS m = MIN(X) BY a + b.
2727
* As the grouping key is used to perform the join, the evaluation required for creating it has to be copied to the left side
2828
* as well.
2929
*/

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/InlineStats.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.List;
2929
import java.util.Objects;
3030

31+
import static java.util.Collections.emptyList;
3132
import static org.elasticsearch.xpack.esql.expression.NamedExpressions.mergeOutputAttributes;
3233

3334
/**
@@ -101,6 +102,9 @@ private JoinConfig joinConfig() {
101102
for (Expression g : groupings) {
102103
namedGroupings.add(Expressions.attribute(g));
103104
}
105+
// last named grouping wins, just like it happens for regular STATS
106+
// ie BY x = field_1, x = field_2, the grouping is actually performed on second x (field_2)
107+
namedGroupings = mergeOutputAttributes(namedGroupings, emptyList());
104108

105109
List<Attribute> leftFields = new ArrayList<>(groupings.size());
106110
List<Attribute> rightFields = new ArrayList<>(groupings.size());

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/InlineJoin.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,16 @@ public static LogicalPlan stubSource(UnaryPlan sourcePlan, LogicalPlan target) {
5959
* Replaces the stubbed source with the actual source.
6060
*/
6161
public static LogicalPlan replaceStub(LogicalPlan source, LogicalPlan stubbed) {
62-
return stubbed.transformUp(StubRelation.class, stubRelation -> source);
62+
// here we could have used stubbed.transformUp(StubRelation.class, stubRelation -> source)
63+
// but transformUp skips changing a node if its tranformed variant it's equal to its original variant.
64+
// A StubRelation can contain in its output ReferenceAttributes which do not use NameIds for equality, but only names and
65+
// two ReferenceAttributes with the same name are equal and the transformation will not be applied.
66+
return stubbed.transformUp(UnaryPlan.class, up -> {
67+
if (up.child() instanceof StubRelation) {
68+
return up.replaceChild(source);
69+
}
70+
return up;
71+
});
6372
}
6473

6574
/**

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4089,6 +4089,21 @@ public void testImplicitCastingForDateAndDateNanosFields() {
40894089
assertEquals("test*", esRelation.indexPattern());
40904090
}
40914091

4092+
public void testGroupingOverridesInStats() {
4093+
verifyUnsupported("""
4094+
from test
4095+
| stats MIN(salary) BY x = languages, x = x + 1
4096+
""", "Found 1 problem\n" + "line 2:43: Unknown column [x]", "mapping-default.json");
4097+
}
4098+
4099+
public void testGroupingOverridesInInlinestats() {
4100+
assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V8.isEnabled());
4101+
verifyUnsupported("""
4102+
from test
4103+
| inlinestats MIN(salary) BY x = languages, x = x + 1
4104+
""", "Found 1 problem\n" + "line 2:49: Unknown column [x]", "mapping-default.json");
4105+
}
4106+
40924107
private void verifyNameAndType(String actualName, DataType actualType, String expectedName, DataType expectedType) {
40934108
assertEquals(expectedName, actualName);
40944109
assertEquals(expectedType, actualType);

0 commit comments

Comments
 (0)