You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/05/15 01:30:46 UTC

[GitHub] [druid] clintropolis opened a new pull request #11259: fix sql planner bug with inner offset causing loop

clintropolis opened a new pull request #11259:
URL: https://github.com/apache/druid/pull/11259


   ### Description
   This PR fixes a bug in the Druid SQL planner where a query with an offset would incorrectly try to plan to a timeseries query, (which does not support offset), effectively ignoring it which could lead to an infinite loop in the planner rewriting the Sort of a query such as this example, which has been added to the tests:
   
   ```sql
   SELECT r0.c, r1.c
   FROM (
     SELECT COUNT(*) AS c
     FROM "foo"
     GROUP BY ()
     OFFSET 1
   ) AS r0
   LEFT JOIN (
     SELECT COUNT(*) AS c
     FROM "foo"
     GROUP BY ()
   ) AS r1 ON TRUE
   LIMIT 10
   ```
   
   This is because the timeseries query construction during planning was only checking that offset was not present if there was at least one grouping dimension (when it should've always been checking this), so simply moving this check outside of an 'if' statement resolves the issue.
   
   Credit to @vogievetsky for finding this :+1:
   
   This PR has:
   - [x] been self-reviewed.
   - [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

[GitHub] [druid] clintropolis commented on pull request #11259: fix sql planner bug with inner offset causing loop

Posted by GitBox <gi...@apache.org>.
clintropolis commented on pull request #11259:
URL: https://github.com/apache/druid/pull/11259#issuecomment-841727896


   thanks for review @jihoonson and @vogievetsky 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

[GitHub] [druid] clintropolis merged pull request #11259: fix sql planner bug with inner offset causing loop

Posted by GitBox <gi...@apache.org>.
clintropolis merged pull request #11259:
URL: https://github.com/apache/druid/pull/11259


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

[GitHub] [druid] clintropolis commented on pull request #11259: fix sql planner bug with inner offset causing loop

Posted by GitBox <gi...@apache.org>.
clintropolis commented on pull request #11259:
URL: https://github.com/apache/druid/pull/11259#issuecomment-841727896


   thanks for review @jihoonson and @vogievetsky 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

[GitHub] [druid] clintropolis merged pull request #11259: fix sql planner bug with inner offset causing loop

Posted by GitBox <gi...@apache.org>.
clintropolis merged pull request #11259:
URL: https://github.com/apache/druid/pull/11259


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org