Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Wrong grammar for negative number #488

Closed
dai-chen opened this issue May 26, 2020 · 4 comments
Closed

Wrong grammar for negative number #488

dai-chen opened this issue May 26, 2020 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@dai-chen
Copy link
Member

The negative integer and floating-point number is matched wrongly in current ANTLR grammar. This bug seems introduced since the very beginning when new ANTLR parser was added. Here is an example:

PUT float-test/_doc/1
{
  "balance": 1.23
}

PUT float-test/_doc/2
{
  "balance": 2.21
}

PUT float-test/_doc/3
{
  "balance": -746.0000000000075
}

POST _opendistro/_sql?pretty
{
  "query": """
    SELECT
      balance FROM null-sum-test WHERE balance > -746
  """
}
{
  "error": {
    "reason": "Invalid SQL query",
    "details": "*Field [-746] cannot be found or used here.*",
    "type": "SemanticAnalysisException"
  },
  "status": 400
}

POST _opendistro/_sql?pretty
{
  "query": """
    SELECT
      balance FROM null-sum-test WHERE balance > -1.0
  """
}
{
  "error": {
    "reason": "Invalid SQL query",
    "details": "Failed to parse query due to *offending symbol [.0]* at: 
    '\n    SELECT\n      balance FROM null-sum-test WHERE balance > -1.0' <--- HERE... 
    More details: extraneous input '.0' expecting <EOF>",
    "type": "SyntaxAnalysisException"
  },
  "status": 400
}
@bernony
Copy link

bernony commented Jun 16, 2020

Hi everyone,
In my company, we use Open Distro to access ES.
Yesterday, we realized negative float and integer values are not well handled by open distro. When we checked, we saw you already solved the issue(what is a good news) but not released yet.
As it is critical for us and have no workaround, we'd like to know when gonna be the next release including that fix.

Thank you I advance.

BR

Bernony

@dai-chen
Copy link
Member Author

Hi @bernony , the error was in our ANTLR parser grammar which is for semantic check. You can disable it for now and enable it to verify after new release. Thanks!

Reference for how to disable semantic analyzer: https://github.com/opendistro-for-elasticsearch/sql/blob/master/docs/user/admin/settings.rst#opendistro-sql-query-analysis-enabled

@bernony
Copy link

bernony commented Jun 25, 2020

Hi @dai-chen . We tried it and it works. Thank you

@dai-chen
Copy link
Member Author

Hi @dai-chen . We tried it and it works. Thank you

Cool. Just to let you know, the fix is already included in upcoming 1.9 release. Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants