Skip to content

SQL: Refactor named expression#46954

Closed
costin wants to merge 2 commits intoelastic:masterfrom
costin:refactor_named_expression_1
Closed

SQL: Refactor named expression#46954
costin wants to merge 2 commits intoelastic:masterfrom
costin:refactor_named_expression_1

Conversation

@costin
Copy link
Copy Markdown
Member

@costin costin commented Sep 21, 2019

Preface

The first commit milestone from refactoring of NamedExpressions.
Essentially there are only 3 types of NamedExpressions:

  1. Alias - user define (implicit or explicit) name
  2. FieldAttribute - field from Elasticsearch
  3. ReferenceAttribute - a reference to another source acting as an Attribute.

To recap, Attributes form the properties of a derived table an each LogicalPlan has Attributes 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 against Attribute is 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, UnresolvedAttribute and ReferenceAttribute.
Updated Analyzer, Verifier and VerifierErrorMessageTests are 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.

Restrict named expression hierarchy
Still plenty of compilation errors inside the optimizer regarding
functionId
Remove different types of Attributes and allow only FieldAttribute,
UnresolvedAttribute and ReferenceAttribute.

Updated Verifier and VerifierErrorMessageTests are passing
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-search

@costin
Copy link
Copy Markdown
Member Author

costin commented Nov 29, 2019

Closed by #49693

@costin costin closed this Nov 29, 2019
@costin costin deleted the refactor_named_expression_1 branch December 10, 2019 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants