Skip to content

Conversation

@krisds
Copy link
Contributor

@krisds krisds commented Feb 1, 2019

As requested, I will split #38033 into separate PRs. Here is the second part.

This one resolves:

@jimczi jimczi added the :Analytics/SQL SQL querying label Feb 1, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@costin
Copy link
Member

costin commented Feb 2, 2019

Looks like I've commented incorrectly at #38151 (comment)

I'll post it here FTR:

Thanks for raising the PR.
However I don't think the proposed solution is correct; I can see how an analysis tool will pick it up (Literal and NamedExpression used to be different hierarchies which got unified at some point though they might be split again).
As the literal already inherits the NamedExpression behavior , the correct solution would be to remove the literal check all together - let me know if you want to pursue this or not.
Thanks,

@javanna
Copy link
Member

javanna commented Feb 25, 2019

hi @krisds have you seen the comment above? Are you for addressing your PR? Thanks!

@krisds
Copy link
Contributor Author

krisds commented Feb 25, 2019

@javanna Yes, I can fix that. Will do.

@javanna
Copy link
Member

javanna commented Mar 18, 2019

@costin can you have a look and merge this when ready please?

@javanna
Copy link
Member

javanna commented Mar 18, 2019

test this please

@matriv
Copy link
Contributor

matriv commented Feb 4, 2020

Closing this PR as the code has been changed and the relevant block doesn't exist anymore.

@matriv matriv closed this Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/SQL SQL querying

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants