-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Closed
Labels
:Analytics/ES|QLAKA ESQLAKA ESQL>non-issueTeam:AnalyticsMeta label for analytical engine team (ESQL/Aggs/Geo)Meta label for analytical engine team (ESQL/Aggs/Geo)
Description
Factored out of #138888
Figure out if #139417 already covers all scenarios required for load and add tests:
- run the existing spec tests but with various fields unmapped and have a look at what happens; many will fail, but it's interesting which exactly and how.
- unmap nothing at first
- unmap keyword field
first_nameinemployees./gradlew ":x-pack:plugin:esql:qa:server:single-node:javaRestTest" --tests "org.elasticsearch.xpack.esql.qa.single_node.EsqlSpecIT" -Dtests.method="test {csv-spec:inlinestats.InlineStatsAfterPruningAggregate8_NoLimit}"failing withCannot use field [id] due to ambiguities being mapped as [2] incompatible types: [keyword] enforced by INSIST command, and [long] in index mappingskstack_trace�y��org.elasticsearch.xpack.esql, already covered in ESQL: existing spec tests should still run with unmapped_field="load" #141874
- unmap text field
jobinemployees-> worked fine, no new discoveries - unmap keyword field
nameinapps-> this got us- ESQL: unmapped_fields="load" can lead to wrong sort order #141925
- ESQL: unmapped_fields="load" can lead to 0 pages emitted #141920
./gradlew ":x-pack:plugin:esql:qa:server:single-node:javaRestTest" --tests "org.elasticsearch.xpack.esql.qa.single_node.EsqlSpecIT" -Dtests.method="test {csv-spec:union_types.ShortIntegerWideningFork}"and others failing withoptimized incorrectly due to missing references- already covered in ESQL: existing spec tests should still run with unmapped_field="load" #141874- ESQL: unmapped_fields="load" and "nullify" do not keep column ordering stable #141923
- unmap version field
versionin apps- ESQL: disallow unmapped_fields="load" with full-text search/
MATCH#141927 ./gradlew ":x-pack:plugin:esql:qa:server:single-node:javaRestTest" --tests "org.elasticsearch.xpack.esql.qa.single_node.EsqlSpecIT" -Dtests.method="test {csv-spec:union_types.ShortIntegerWideningFork}"results inFound 2 problems\nline 6:3: Plan [Eval[[x{r}#103889 AS $$x$temp_name$103900#103901,\ \ x{r}#103897 + 1[INTEGER] AS x#103879]]] optimized incorrectly due to missing\ \ references [x{r}#103889]\nline 5:3: Plan [TopN[[Order[$$x$temp_name$103900{r$}#103901,ASC,LAST],\ \ Order[name{r}#103887,ASC,LAST], Order[version{r}#103899,ASC,\nLAST]],10[INTEGER],false]]\ \ optimized incorrectly due to missing references [name{r}#103887], already covered in ESQL: existing spec tests should still run with unmapped_field="load" #141874
- ESQL: disallow unmapped_fields="load" with full-text search/
- unmap geo_point field
locationfromairports:- no unexpected failures
- unmap date field
@timestampfromsample_data(this requires adding a new TestDataSet to avoid also unmapping this in derived data sets, likesample_data_ts_longandsample_data_ts_nanos):
- The tests in AnalyzerUnmappedTests.java mostly cover
nullify. Make similar tests forload(ESQL: Increases unmapped fields test coverage (golden and spec) #143522) - Ensure that including indices with unmapped fields doesn't accidentally change the column ordering in the query result, see ESQL: introduce support for mapping-unavailable fields (Fork from #139417) #140463 (comment) (PropgateUnmappedFields uses
mergeOutputAttributes(), which does not preserve the ordering of the attributes in the EsRelation corresponding to theFROM ...command).
|--> Added a comment about this.
|--> (@alex-spies ) we've encountered ESQL: unmapped_fields="load" and "nullify" do not keep column ordering stable #141923 in case of unmapped fields. Partially mapped fields are covered in large quantities in the regular spec tests because we randomly nullify our test queries. - Address review comments on
x-pack/plugin/esql/qa/testFixtures/src/main/resources/unmapped-load.csv-spec, see this review
|--> (@alex-spies ) Test suggestions were moved into their own "support this" issues. - spec tests for
-
fork-> ESQL: unmapped_fields="load" needs consistent behavior for branching commands (FORK, subqueries, views) #142033 -
fuse-> ESQL: unmapped_fields="load" needs consistent behavior for branching commands (FORK, subqueries, views) #142033 -
lookup join-> ESQL: unmapped_fields="load" with LOOKUP JOIN #142026 - TS (ESQL: Increases unmapped fields test coverage (golden and spec) #143522)
- partially mapped field, missing in one index, present in the other with a non-KEYWORD type, but being cast explicitly in the query (ESQL: Type conflict resolution in unmapped-fields load #143693)
- partially mapped field, missing in one index, present with type conflict in two other indices but being cast explicitly in the query (ESQL: Type conflict resolution in unmapped-fields load #143693)
- partially mapped field, missing in one index, present with type conflict in two other indices, not cast but also not used outside of KEEP/DROP -> should be consistent with non-cast union types, i.e. unsupported, filled with nulls. (https://github.com/elastic/elasticsearch/pull/143693/changes#r2980545924)
- partially mapped field, missing in one index, present with type conflict in two other indices, cast explicitly and used in expressions (ESQL: Type conflict resolution in unmapped-fields load #143693)
-
- on field aliases (required by o11y) (ESQL: unmapped_fields="nullify" and "load" with field aliases #144844)
- CCS (/ CPS?)
|--> @alex-spies : spec tests are being run in multi-cluster, too, no need to do anything particular for CCS. CPS only affects how index patterns are resolved, but not their mappings, so I'm going to say that this is most likely fine. - general: @timestamp (TimestampAware functions), coalesce()
|--> @alex-spies : should be covered by ESQL: Forbid SET nullify and load for implicit @timestamp field #144239. - ~~we fail gracefully (no 500) in case of type errors caused by
load. This is important for types that will cause type errors when they go unmapped, likeTEXTbecause we cannot cast toTEXT. ~~
|--> @alex-spies : not really actionable; we'll track 5xx errors as separate bugs when we encounter them. - esp.: full text search functions (require
TEXT) and stuff that uses e.g. COUNTER_LONG (also cannot be cast to)
|--> @alex-spies : ESQL: unmapped_fields="load" should work with full-text search/MATCH #144121
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
:Analytics/ES|QLAKA ESQLAKA ESQL>non-issueTeam:AnalyticsMeta label for analytical engine team (ESQL/Aggs/Geo)Meta label for analytical engine team (ESQL/Aggs/Geo)
Type
Fields
Give feedbackNo fields configured for issues without a type.