Closed
Conversation
Remove different types of Attributes and allow only FieldAttribute, UnresolvedAttribute and ReferenceAttribute. Updated Verifier and VerifierErrorMessageTests are passing
Collaborator
|
Pinging @elastic/es-search |
This was referenced Nov 25, 2019
Member
Author
|
Closed by #49693 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Preface
The first commit milestone from refactoring of
NamedExpressions.Essentially there are only 3 types of
NamedExpressions:Alias- user define (implicit or explicit) nameFieldAttribute- field fromElasticsearchReferenceAttribute- a reference to another source acting as anAttribute.To recap,
Attributes form the properties of a derived table an eachLogicalPlanhasAttributes as output since each one can be part of a query and its result sent to the user.This change essentially removes the name id comparison so any changes applied to existing functions should work as long as the functions are semantically equivalent.
By removing any property from an
Attribute, the various components need to look the original source for comparison which, while annoying, should prevent nodes from getting out of sync due to optimizations.For the most part, I expect by eliminating secondary aliases, this issue will be isolated and further more, by tweaking the rules of the Analyzer, we could opt for copying the source instead of a
ReferenceAttribute(though the latter makes things more robust as working againstAttributeis better since it means working against the official output/abstraction).Actual commit
Restrict named expression hierarchy
Remove different types of Attributes and allow only
FieldAttribute,UnresolvedAttributeandReferenceAttribute.Updated
Analyzer,VerifierandVerifierErrorMessageTestsare compiling and passing.There are still compilation errors down the stream however I'm taking a staged approach to introduce the concept in stages before ending up with final, big PR.