Skip to content

SQL: Extract common parts into a separate QL project #50049

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 9 commits into from
Closed

SQL: Extract common parts into a separate QL project #50049

wants to merge 9 commits into from

Conversation

@costin
Copy link
Member

@costin costin commented Dec 10, 2019

First draft for extracting reusable components from SQL into their own project.
It is wip mainly to double check the tests

@costin costin added WIP :Analytics/SQL SQL querying :Analytics/EQL EQL querying labels Dec 10, 2019
@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 10, 2019

Pinging @elastic/es-search (:Search/SQL)

matriv
matriv reviewed Jan 9, 2020
View reviewed changes
Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments, but looks good!

matriv
matriv reviewed Jan 9, 2020
View reviewed changes
Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done with a few more comments.

@costin costin mentioned this pull request Jan 9, 2020
Merged
astefan
astefan approved these changes Jan 10, 2020
View reviewed changes
Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some minor comments. LGTM.

...plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/function/OptionalArgument.java
* Marker interface indicating that a function accepts one optional argument (typically the last one).
* This is used by the {@link FunctionRegistry} to perform validation of function declaration.
*/
public interface OptionalArgument {
Copy link
Contributor

@astefan astefan Jan 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change needs to go into SQL's function classes as well, at one point.

Copy link
Member Author

@costin costin Jan 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean? It's used by classes both in ql (Iif) and sql (TopHits).

@costin
Copy link
Member Author

costin commented Jan 10, 2020 •
edited
Loading

Superseded by #50815

@costin costin closed this Jan 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@astefan astefan astefan approved these changes

+1 more reviewer

@matriv matriv matriv left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

:Analytics/EQL EQL querying :Analytics/SQL SQL querying WIP

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@costin @elasticmachine @astefan @matriv