Skip to content
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

[8.14] ESQL: Fix MV_DEDUPE when using data from an index (#107577) #107850

Merged
merged 2 commits into from Apr 24, 2024

Conversation

alex-spies
Copy link
Contributor

@alex-spies alex-spies commented Apr 24, 2024

Backport

This will backport the following commits from main to 8.14:

In contrast to the original pull request, label boolean/numerical blocks with MvOrdering.UNORDERED rather than SORTED_ASCENDING. This is still correct and avoids requiring to bump the transport version for this backport.

Correctly label numerical/boolean blocks loaded from indices, so that MV_DEDUPE works correctly.

(cherry picked from commit 70cfe6f)

# Conflicts:
#	server/src/main/java/org/elasticsearch/TransportVersions.java
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlFeatures.java
Comment on lines 178 to 185
public static final TransportVersion WATERMARK_THRESHOLDS_STATS = def(8_637_00_0);
public static final TransportVersion ENRICH_CACHE_ADDITIONAL_STATS = def(8_638_00_0);
public static final TransportVersion ML_INFERENCE_RATE_LIMIT_SETTINGS_ADDED = def(8_639_00_0);
public static final TransportVersion ML_TRAINED_MODEL_CACHE_METADATA_ADDED = def(8_640_00_0);
public static final TransportVersion TOP_LEVEL_KNN_SUPPORT_QUERY_NAME = def(8_641_00_0);
public static final TransportVersion INDEX_SEGMENTS_VECTOR_FORMATS = def(8_642_00_0);
public static final TransportVersion ADD_RESOURCE_ALREADY_UPLOADED_EXCEPTION = def(8_643_00_0);
public static final TransportVersion ESQL_MV_ORDERING_SORTED_ASCENDING = def(8_644_00_0);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh-oh, this backport would require to add capabilities to act on transport versions that are not yet handled in 8.14. I think this cannot work like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by not using SORTED_ASCENDING, instead labelling with UNORDERED.

Adapt this fix so that we do not have to introduce SORTED_ASCENDING, and
thus do not have to bump the transport version for this to work.

Boolean/numerical doc values are just labelled as UNORDERED, instead,
which is still correct.
@alex-spies alex-spies added the auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Apr 24, 2024
@elasticsearchmachine elasticsearchmachine merged commit 73a025e into elastic:8.14 Apr 24, 2024
14 checks passed
@alex-spies alex-spies deleted the backport/8.14/pr-107577 branch April 24, 2024 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport v8.14.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants