Skip to content

[9.0] ESQL: Refactor Greatest and Least functions to use evaluator map (#114036) #128429

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

shinsj4653
Copy link

@shinsj4653 shinsj4653 commented May 25, 2025

ESQL: Refactor Greatest and Least functions to use evaluator map

Description

This PR refactors the Greatest and Least functions to use an evaluator map for better code organization and maintainability. The changes include:

  1. Added EVALUATOR_MAP to map data types to their corresponding evaluator factories
  2. Updated toEvaluator method to use the map instead of if-else chains
  3. Added proper NULL type handling
  4. Updated test cases to match the new implementation

Changes

  • Greatest.java:

    • Added EVALUATOR_MAP for evaluator factory mapping
    • Updated toEvaluator to use map-based lookup
    • Added NULL type validation
    • Updated error messages to use getWriteableName()
  • Least.java:

    • Added EVALUATOR_MAP for evaluator factory mapping
    • Updated toEvaluator to use map-based lookup
    • Added NULL type validation
    • Updated error messages to use getWriteableName()

Testing

  • All existing tests pass (GreatestTests.java, LeastTests.java)

    • GreatestTests.java

    image

    • LeastTests.java

    image

  • Test coverage includes:

    • All supported data types (boolean, double, integer, long, keyword, text, version, ip, datetime, date_nanos)
    • NULL value handling
    • String comparisons
    • Numeric comparisons
    • Boolean operations
    • Date/time comparisons

Related Issues

Closes #114036

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Backport

  • Backport to 8.19
  • Backport to 8.18
  • Backport to 8.17

This commit refactors the Greatest function to use an evaluator map for better
code organization. The changes include:
- Added EVALUATOR_MAP for evaluator factory mapping
- Updated toEvaluator to use map-based lookup
- Added NULL type validation
- Updated error messages to use getWriteableName()

The refactoring improves code readability and maintainability by:
- Replacing if-else chains with a map-based lookup
- Centralizing evaluator factory creation
- Adding consistent NULL type validation
- Using getWriteableName() for error messages

Relates to elastic#114036
This commit refactors the Least function to use an evaluator map for better
code organization. The changes include:
- Added EVALUATOR_MAP for evaluator factory mapping
- Updated toEvaluator to use map-based lookup
- Added NULL type validation
- Updated error messages to use getWriteableName()

The refactoring improves code readability and maintainability by:
- Replacing if-else chains with a map-based lookup
- Centralizing evaluator factory creation
- Adding consistent NULL type validation
- Using getWriteableName() for error messages

Relates to elastic#114036
This commit adds test cases for both Greatest and Least functions to verify:
- All supported data types (boolean, double, integer, long, keyword, text, version, ip, datetime, date_nanos)
- NULL value handling
- String comparisons
- Numeric comparisons
- Boolean operations
- Date/time comparisons

The tests ensure that:
- Both functions handle NULL values correctly
- Type validation works as expected
- Evaluator selection is correct for each data type
- Results are computed correctly for all supported types

Relates to elastic#114036
@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label v9.1.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels May 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external-contributor Pull request authored by a developer outside the Elasticsearch team needs:triage Requires assignment of a team area label v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ESQL] Greatest and Least can fail with Illegal Data Type on types that resolved correctly
2 participants