From 43c320f75fd4803fdf9c0902edc2ce609491723e Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Tue, 5 Dec 2023 08:53:52 +0100 Subject: [PATCH] Fix layout for MV_EXPAND (#102916) --- docs/changelog/102916.yaml | 6 ++++++ .../src/main/resources/mv_expand.csv-spec | 10 ++++++++++ .../elasticsearch/xpack/esql/planner/Layout.java | 9 +++++++++ .../xpack/esql/planner/LocalExecutionPlanner.java | 15 ++------------- 4 files changed, 27 insertions(+), 13 deletions(-) create mode 100644 docs/changelog/102916.yaml diff --git a/docs/changelog/102916.yaml b/docs/changelog/102916.yaml new file mode 100644 index 0000000000000..3943f34d91221 --- /dev/null +++ b/docs/changelog/102916.yaml @@ -0,0 +1,6 @@ +pr: 102916 +summary: Fix layout for MV_EXPAND +area: ES|QL +type: bug +issues: + - 102912 diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/mv_expand.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/mv_expand.csv-spec index c681a1a7e977c..a3bc9c6c6dcf6 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/mv_expand.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/mv_expand.csv-spec @@ -306,3 +306,13 @@ a:long | b:long | c:long | gender:keyword | str:keyword | x:key 57 |57 |57 |M |"57,M" |M 0 |10 |10 |null |null |null ; + + +//see https://github.com/elastic/elasticsearch/issues/102912 +statsDissectThatOverwritesAndMvExpand#[skip:-8.11.99] +row a = "a", b = 1 | stats e = min(b) by a | dissect a "%{e}" | mv_expand e; + +a:keyword | e:keyword +a | a +; + diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/Layout.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/Layout.java index 871d3751b225d..97885a060d639 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/Layout.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/Layout.java @@ -119,5 +119,14 @@ public Layout build() { } return new DefaultLayout(Collections.unmodifiableMap(layout), numberOfChannels); } + + public void replace(NameId id, NameId id1) { + for (ChannelSet channel : this.channels) { + if (channel != null && channel.nameIds.contains(id)) { + channel.nameIds.remove(id); + channel.nameIds.add(id1); + } + } + } } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java index 3d377497e17af..c531fd01c2a40 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java @@ -581,20 +581,9 @@ private PhysicalOperation planLimit(LimitExec limit, LocalExecutionPlannerContex private PhysicalOperation planMvExpand(MvExpandExec mvExpandExec, LocalExecutionPlannerContext context) { PhysicalOperation source = plan(mvExpandExec.child(), context); - List childOutput = mvExpandExec.child().output(); int blockSize = 5000;// TODO estimate row size and use context.pageSize() - - Layout.Builder layout = new Layout.Builder(); - List inverse = source.layout.inverse(); - var expandedName = mvExpandExec.expanded().name(); - for (int index = 0; index < inverse.size(); index++) { - if (childOutput.get(index).name().equals(expandedName)) { - layout.append(mvExpandExec.expanded()); - } else { - layout.append(inverse.get(index)); - } - } - + Layout.Builder layout = source.layout.builder(); + layout.replace(mvExpandExec.target().id(), mvExpandExec.expanded().id()); return source.with( new MvExpandOperator.Factory(source.layout.get(mvExpandExec.target().id()).channel(), blockSize), layout.build()