Skip to content

Conversation

@rw-access
Copy link
Contributor

Relates to #51756 (closed with API support)

Add support for ? params to EQL. This hasn't been integrated/tested with for the API, but that can be added here easily if this is the right PR for that.

@rw-access rw-access added the :Analytics/EQL EQL querying label Feb 13, 2020
@rw-access rw-access requested review from astefan and costin February 13, 2020 00:46
@elasticmachine
Copy link
Collaborator

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

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.

I've left few comments.

Also, since ? has other meaning in the grammar, as well, I would like to see a test that contains both uses of ?. For example:

assertEquals(expr("a == ?\"String with literal 'slash' \\ characters included\" and b == ? or c == ?", Arrays.asList("test", false)),
                expr("a == ?\"String with literal 'slash' \\ characters included\" and b == 'test' or c == false"));

Copy link
Member

@costin costin 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 - more tests are needed to check that not only the param get replaced but also that tricky strings are properly added in the query.


public class ExpressionBuilder extends IdentifierBuilder {

protected final ParserParams params;
Copy link
Member

Choose a reason for hiding this comment

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

Why protected and not private?

@matriv
Copy link
Contributor

matriv commented Mar 30, 2020

@elastic/es-ql

@rjernst rjernst added the Team:QL (Deprecated) Meta label for query languages team label May 4, 2020
@matriv
Copy link
Contributor

matriv commented Sep 15, 2020

After searching around regarding query params in SQL DBs, I saw that ? is the one used for prepared statements, and then each DB uses different syntax for its procedural language (defining custom functions/stored procedures, etc) like: @param, $param or :param. In my opinion I would say let's proceed with the basic usage of ? as a place holder and a list (array) of value replacements that correspond to those params.

@rw-access rw-access closed this Mar 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/EQL EQL querying Team:QL (Deprecated) Meta label for query languages team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants