Skip to content

[8.18] ESQL: Fix FieldAttribute name usage in InferNonNullAggConstraint (#128910) #129272

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 2 commits into from
Jun 11, 2025

Conversation

alex-spies
Copy link
Contributor

@alex-spies alex-spies commented Jun 11, 2025

This will backport the following commits from main to 8.18:

…stic#128910)

* Fix InferNonNullAggConstraint with union types
* Begin fixing LucenePushdownPredicates with union types
* Introduce a dedicated wrapper record FieldName to be used where field names are really required.

The fixes consist of using FieldAttribute.fieldName() instead of .name() or .field().name(). .name() can be some temporary string unrelated to the actual name of the Lucene index field, whereas .field().name() doesn't know about parent fields; .fieldName() gives the full field name (from the root of the document).

The biggest offender of such misuse is SearchStats; make this always require a FieldName, not a String - and make FieldAttribute#fieldName handily return an instance of FieldName so users of SearchStats don't accidentally use the return value of FieldAttribute#name.

(cherry picked from commit 0850bd7)

# Conflicts:
#	x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/LucenePushdownPredicates.java
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/EsPhysicalOperationProviders.java
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/stats/SearchContextStats.java
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/stats/SearchStats.java
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/TestPhysicalOperationProviders.java
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/stats/DisabledSearchStats.java
@alex-spies alex-spies added >bug auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Analytics/ES|QL AKA ESQL labels Jun 11, 2025
@elasticsearchmachine elasticsearchmachine merged commit 255dbb6 into elastic:8.18 Jun 11, 2025
15 checks passed
@alex-spies alex-spies deleted the backport/8.18/pr-128910 branch June 11, 2025 17:35
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-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport >bug v8.18.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants