-
Notifications
You must be signed in to change notification settings - Fork 25.6k
ESQL: Fix some more usages of field attribute names in LOOKUP JOIN #129355
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
alex-spies
merged 22 commits into
elastic:main
from
alex-spies:fix-lu-join-with-union-types
Jun 17, 2025
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
08be01c
Fix some attr.name() usages that meant field names
alex-spies 44dca94
Allow union type fields to be match fields
alex-spies 35ed8ea
Improve some comments
alex-spies 646f378
Add csv test for LU JOIN with union types
alex-spies e3ef5c2
[CI] Auto commit changes from spotless
8dfd4dc
Fix test compilation
alex-spies 4858b1d
One more time
alex-spies 4db8c81
Simplify setup in LookupJoinTypesIT
alex-spies a3da244
Refactor some more
alex-spies 82d359b
Revert "Refactor some more"
alex-spies 0a87d8b
Expand comment
alex-spies 6634289
Refactor some more
alex-spies 3470281
Refactor some more
alex-spies 213f5df
Simplify TestConfigPasses
alex-spies 77c5835
Add union type test cases to LookupJoinTypesIT
alex-spies 7f98271
Merge remote-tracking branch 'upstream/main' into fix-lu-join-with-un…
alex-spies db3b5bb
Improve readability based on Fang's comments
alex-spies e86539b
Avoid HandleLimitFS in LookupJoinTypesIT
alex-spies 9d4102e
[CI] Auto commit changes from spotless
bf01b0d
Merge remote-tracking branch 'upstream/main' into fix-lu-join-with-un…
alex-spies f0f4b48
Make naming more consistent
alex-spies 3a8594e
Improve error message
alex-spies File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding tests for multi-typed numeric fields as join keys. We still have gaps in test coverage for multi-typed join keys involving string types (keyword, text, ip) and date types (millis, nanos). If these are not within the scope of this PR, they should be covered in a separate one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a more complete list of tests to
LookupJoinTypesIT. It's still not exhaustive but IMO covers enough ground for now.That required a bit of refactoring ^^