Skip to content

[8.18] ESQL: Fix inconsistent column order in MV_EXPAND (#129745) #131448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/129745.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 129745
summary: "ESQL: Fix `mv_expand` inconsistent column order"
area: ES|QL
type: bug
issues:
- 129000
Original file line number Diff line number Diff line change
Expand Up @@ -418,3 +418,65 @@ emp_no:integer | job_positions:keyword
10001 | Accountant
10001 | Senior Python Developer
;

testMvExpandInconsistentColumnOrder1
required_capability: fix_mv_expand_inconsistent_column_order
from message_types
| eval foo_1 = 1, foo_2 = 2
| sort message
| mv_expand foo_1
;

message:keyword | type:keyword | foo_1:integer | foo_2:integer
Connected to 10.1.0.1 | Success | 1 | 2
Connected to 10.1.0.2 | Success | 1 | 2
Connected to 10.1.0.3 | Success | 1 | 2
Connection error | Error | 1 | 2
Development environment | Development | 1 | 2
Disconnected | Disconnected | 1 | 2
Production environment | Production | 1 | 2
;

testMvExpandInconsistentColumnOrder2
required_capability: fix_mv_expand_inconsistent_column_order
from message_types
| eval foo_1 = [1, 3], foo_2 = 2
| sort message
| mv_expand foo_1
;

message:keyword | type:keyword | foo_1:integer | foo_2:integer
Connected to 10.1.0.1 | Success | 1 | 2
Connected to 10.1.0.1 | Success | 3 | 2
Connected to 10.1.0.2 | Success | 1 | 2
Connected to 10.1.0.2 | Success | 3 | 2
Connected to 10.1.0.3 | Success | 1 | 2
Connected to 10.1.0.3 | Success | 3 | 2
Connection error | Error | 1 | 2
Connection error | Error | 3 | 2
Development environment | Development | 1 | 2
Development environment | Development | 3 | 2
Disconnected | Disconnected | 1 | 2
Disconnected | Disconnected | 3 | 2
Production environment | Production | 1 | 2
Production environment | Production | 3 | 2
;

testMvExpandInconsistentColumnOrder3
required_capability: fix_mv_expand_inconsistent_column_order
from message_types
| sort type
| eval language_code = 1, `language_name` = false, message = true, foo_3 = 1, foo_2 = null
| eval foo_3 = "1", `foo_3` = -1, foo_1 = 1, `language_code` = null, `foo_2` = "1"
| mv_expand foo_1
| limit 5
;

type:keyword | language_name:boolean | message:boolean | foo_3:integer | foo_1:integer | language_code:null | foo_2:keyword
Development | false | true | -1 | 1 | null | 1
Disconnected | false | true | -1 | 1 | null | 1
Error | false | true | -1 | 1 | null | 1
Production | false | true | -1 | 1 | null | 1
Success | false | true | -1 | 1 | null | 1
;

Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,13 @@ public enum Cap {
* see <a href="https://github.com/elastic/elasticsearch/issues/127468"> ES|QL: Grok only supports KEYWORD or TEXT values,
* found expression [type] type [INTEGER] #127468 </a>
*/
KEEP_REGEX_EXTRACT_ATTRIBUTES;
KEEP_REGEX_EXTRACT_ATTRIBUTES,

/**
* Support for the mv_expand target attribute should be retained in its original position.
* see <a href="https://github.com/elastic/elasticsearch/issues/129000"> ES|QL: inconsistent column order #129000 </a>
*/
FIX_MV_EXPAND_INCONSISTENT_COLUMN_ORDER;

private final boolean enabled;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,22 @@ public PhysicalPlan apply(PhysicalPlan plan) {

// no need for projection when dealing with aggs
if (logicalFragment instanceof Aggregate == false) {
List<Attribute> output = new ArrayList<>(requiredAttributes.get());
// we should respect the order of the attributes
List<Attribute> output = new ArrayList<>();
for (Attribute attribute : logicalFragment.output()) {
if (requiredAttributes.get().contains(attribute)) {
output.add(attribute);
}
}
// requiredAttributes should only have attributes that are also in the fragment's output.
// This assumption can be wrong in case of remote ENRICH, see https://github.com/elastic/elasticsearch/issues/118531
// TODO: stop adding the remaining required attributes once remote ENRICH is fixed.
if (output.size() != requiredAttributes.get().size()) {
AttributeSet alreadyAdded = new AttributeSet(output);
AttributeSet remaining = requiredAttributes.get().subtract(alreadyAdded);
output.addAll(remaining);
}

// if all the fields are filtered out, it's only the count that matters
// however until a proper fix (see https://github.com/elastic/elasticsearch/issues/98703)
// add a synthetic field (so it doesn't clash with the user defined one) to return a constant
Expand Down
Loading