Skip to content
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.

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

SQL: datetime functions not filtering correctly: ISO_WEEK_OF_YEAR, MINUTE_OF_DAY #67872

Closed
bpintea opened this issue Jan 24, 2021 · 4 comments
Closed
Assignees
Labels
:Analytics/SQL SQL querying >bug Team:QL (Deprecated) Meta label for query languages team

Comments

@bpintea
Copy link
Contributor

bpintea commented Jan 24, 2021

With the following index data:

POST test/_doc
{
  "a": "2021-01-22T14:26:06.039Z"
}

the following functions fail to filter by the same value they return:

  1. SELECT ISO_WEEK_OF_YEAR(a) AS x FROM test WHERE x=4
    returns:
       x       
---------------
3
  1. SELECT MINUTE_OF_DAY(a) AS x FROM test WHERE x=866
    fails with:
    "type" : "unsupported_operation_exception",
    "reason" : "is there a format for it?"
@elasticmachine elasticmachine added the Team:QL (Deprecated) Meta label for query languages team label Jan 24, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-ql (Team:QL)

@blookot
Copy link

blookot commented Jan 28, 2021

@bpintea weirdly this used to work (a few weeks ago), and today it works again!
This is my request:

GET /_sql?format=txt
{
  "query": """
  SELECT startTimestamp, CURRENT_TIMESTAMP, ISO_WEEK_OF_YEAR(startTimestamp), ISO_WEEK_OF_YEAR(CURRENT_TIMESTAMP) FROM "user-call-logs" WHERE ISO_WEEK_OF_YEAR(startTimestamp) = ISO_WEEK_OF_YEAR(CURRENT_TIMESTAMP) AND YEAR(startTimestamp) = YEAR(CURRENT_DATE)
  """
}

and the response is correct:

     startTimestamp     |   CURRENT_TIMESTAMP    |ISO_WEEK_OF_YEAR(startTimestamp)|ISO_WEEK_OF_YEAR(CURRENT_TIMESTAMP)
------------------------+------------------------+--------------------------------+-----------------------------------
2021-01-28T18:37:36.769Z|2021-01-28T18:43:51.644Z|4                               |4     

@matriv matriv assigned matriv and palesz and unassigned matriv Feb 8, 2021
@palesz
Copy link
Contributor

palesz commented Feb 8, 2021

Also worth mentioning that the ISO_WEEK_OF_YEAR function returns different results in group by vs outside of group by and the order by on this function is also incorrect:

With group by:

SELECT IW(birth_date) iso_week, WEEK(birth_date) week FROM test_emp WHERE IW(birth_date) < 5 GROUP BY iso_week, week ORDER BY iso_week

   iso_week    |     week      
---------------+---------------
1              |2              
3              |4              
4              |4              
4              |5              

Same dataset without group by, but with order by:

SELECT birth_date, IW(birth_date) iso_week, WEEK(birth_date) week FROM test_emp WHERE IW(birth_date) < 5 ORDER BY iso_week

       birth_date       |   iso_week    |     week      
------------------------+---------------+---------------
1953-01-07T00:00:00.000Z|2              |2              
1965-01-03T00:00:00.000Z|53             |2              
1955-01-21T00:00:00.000Z|3              |4              
1958-01-21T00:00:00.000Z|4              |4              
1953-01-23T00:00:00.000Z|4              |4              
1959-01-27T00:00:00.000Z|5              |5              

palesz pushed a commit to palesz/elasticsearch that referenced this issue Feb 9, 2021
The `SELECT ISO_WEEK_OF_YEAR(a) AS x FROM test WHERE x=4` query returned
with `x=3` results because the `ISO_WEEK_YEAR(a)` in the WHERE clause
that turns into a script query and the `ISO_WEEK_YEAR(a)` in the projections
that turns into a post-processing on top of the Query DSL results execute
different code to calculate the result.

This change unifies the different code paths and results in a single method
being responsible for the actual calculation.

Note: this change impacts the way how all the `DateTimeFunction`s that
do the field extraction from a date get translated into a script query.

Fixes part of elastic#67872
palesz pushed a commit to palesz/elasticsearch that referenced this issue Feb 9, 2021
… in comparisons

The `MINUTE_OF_DAY()` extraction function does not have an equivalent
expressable using a datetime format pattern.

The `MinuteOfDay.dateTimeFormat()` is called during the query
translation and throws an exception, but the return value actually
does not impact the translated query (binary comparisons with
`DateTimeFunction` on one side always turn into a script query).

This change fixes the immediate issue raised as part of elastic#67872,
add integration tests covering the problem, but leaves the removal
of the unnecessary `dateTimeFormat()` function a separate PR.
palesz pushed a commit that referenced this issue Feb 10, 2021
… in comparisons (#68783)

The `MINUTE_OF_DAY()` extraction function does not have an equivalent
expressable using a datetime format pattern.

The `MinuteOfDay.dateTimeFormat()` is called during the query
translation and throws an exception, but the return value actually
does not impact the translated query (binary comparisons with
`DateTimeFunction` on one side always turn into a script query).

This change fixes the immediate issue raised as part of #67872,
add integration tests covering the problem, but leaves the removal
of the unnecessary `dateTimeFormat()` function a separate PR.
palesz pushed a commit to palesz/elasticsearch that referenced this issue Feb 10, 2021
… in comparisons (elastic#68783)

The `MINUTE_OF_DAY()` extraction function does not have an equivalent
expressable using a datetime format pattern.

The `MinuteOfDay.dateTimeFormat()` is called during the query
translation and throws an exception, but the return value actually
does not impact the translated query (binary comparisons with
`DateTimeFunction` on one side always turn into a script query).

This change fixes the immediate issue raised as part of elastic#67872,
add integration tests covering the problem, but leaves the removal
of the unnecessary `dateTimeFormat()` function a separate PR.
palesz pushed a commit to palesz/elasticsearch that referenced this issue Feb 10, 2021
… in comparisons (elastic#68783)

The `MINUTE_OF_DAY()` extraction function does not have an equivalent
expressible using a datetime format pattern.

The `MinuteOfDay.dateTimeFormat()` is called during the query
translation and throws an exception, but the return value actually
does not impact the translated query (binary comparisons with
`DateTimeFunction` on one side always turn into a script query).

This change fixes the immediate issue raised as part of elastic#67872,
add integration tests covering the problem, but leaves the removal
of the unnecessary `dateTimeFormat()` function a separate PR.
palesz pushed a commit that referenced this issue Feb 10, 2021
… in comparisons (#68783) (#68846)

The `MINUTE_OF_DAY()` extraction function does not have an equivalent
expressible using a datetime format pattern.

The `MinuteOfDay.dateTimeFormat()` is called during the query
translation and throws an exception, but the return value actually
does not impact the translated query (binary comparisons with
`DateTimeFunction` on one side always turn into a script query).

This change fixes the immediate issue raised as part of #67872,
add integration tests covering the problem, but leaves the removal
of the unnecessary `dateTimeFormat()` function a separate PR.
palesz pushed a commit that referenced this issue Feb 10, 2021
… in comparisons (#68783) (#68854)

The `MINUTE_OF_DAY()` extraction function does not have an equivalent
expressible using a datetime format pattern.

The `MinuteOfDay.dateTimeFormat()` is called during the query
translation and throws an exception, but the return value actually
does not impact the translated query (binary comparisons with
`DateTimeFunction` on one side always turn into a script query).

This change fixes the immediate issue raised as part of #67872,
add integration tests covering the problem, but leaves the removal
of the unnecessary `dateTimeFormat()` function a separate PR.
palesz pushed a commit that referenced this issue Feb 11, 2021
The `SELECT ISO_WEEK_OF_YEAR(a) AS x FROM test WHERE x=4` query returned
with `x=3` results because the `ISO_WEEK_YEAR(a)` in the WHERE clause
that turns into a script query and the `ISO_WEEK_YEAR(a)` in the projections
that turns into a post-processing on top of the Query DSL results execute
different code to calculate the result.

This change unifies the different code paths and results in a single method
being responsible for the actual calculation.

Note: this change impacts the way how all the `DateTimeFunction`s that
do the field extraction from a date get translated into a script query.

Fixes part of #67872
palesz pushed a commit to palesz/elasticsearch that referenced this issue Feb 11, 2021
The `SELECT ISO_WEEK_OF_YEAR(a) AS x FROM test WHERE x=4` query returned
with `x=3` results because the `ISO_WEEK_YEAR(a)` in the WHERE clause
that turns into a script query and the `ISO_WEEK_YEAR(a)` in the projections
that turns into a post-processing on top of the Query DSL results execute
different code to calculate the result.

This change unifies the different code paths and results in a single method
being responsible for the actual calculation.

Note: this change impacts the way how all the `DateTimeFunction`s that
do the field extraction from a date get translated into a script query.

Fixes part of elastic#67872
palesz pushed a commit to palesz/elasticsearch that referenced this issue Feb 11, 2021
The `SELECT ISO_WEEK_OF_YEAR(a) AS x FROM test WHERE x=4` query returned
with `x=3` results because the `ISO_WEEK_YEAR(a)` in the WHERE clause
that turns into a script query and the `ISO_WEEK_YEAR(a)` in the projections
that turns into a post-processing on top of the Query DSL results execute
different code to calculate the result.

This change unifies the different code paths and results in a single method
being responsible for the actual calculation.

Note: this change impacts the way how all the `DateTimeFunction`s that
do the field extraction from a date get translated into a script query.

Fixes part of elastic#67872
palesz pushed a commit that referenced this issue Feb 11, 2021
The `SELECT ISO_WEEK_OF_YEAR(a) AS x FROM test WHERE x=4` query returned
with `x=3` results because the `ISO_WEEK_YEAR(a)` in the WHERE clause
that turns into a script query and the `ISO_WEEK_YEAR(a)` in the projections
that turns into a post-processing on top of the Query DSL results execute
different code to calculate the result.

This change unifies the different code paths and results in a single method
being responsible for the actual calculation.

Note: this change impacts the way how all the `DateTimeFunction`s that
do the field extraction from a date get translated into a script query.

Fixes part of #67872
palesz pushed a commit that referenced this issue Feb 11, 2021
The `SELECT ISO_WEEK_OF_YEAR(a) AS x FROM test WHERE x=4` query returned
with `x=3` results because the `ISO_WEEK_YEAR(a)` in the WHERE clause
that turns into a script query and the `ISO_WEEK_YEAR(a)` in the projections
that turns into a post-processing on top of the Query DSL results execute
different code to calculate the result.

This change unifies the different code paths and results in a single method
being responsible for the actual calculation.

Note: this change impacts the way how all the `DateTimeFunction`s that
do the field extraction from a date get translated into a script query.

Fixes part of #67872
@palesz
Copy link
Contributor

palesz commented Feb 17, 2021

Fixed the issues of both functions, closing.

@palesz palesz closed this as completed Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/SQL SQL querying >bug Team:QL (Deprecated) Meta label for query languages team
Projects
None yet
Development

No branches or pull requests

5 participants