Antonio-RiveroMartnez opened a new pull request, #27934:
URL: https://github.com/apache/superset/pull/27934

   ### SUMMARY
   When applying a time_offset in Advanced Analytics, if the row_limit is too 
small, data will not be present when the join occurs, so this PR prevent such 
behavior by not using limit nor offset in the inner query (the shifted one).
   
   For example this query shown in the following video has all the records for 
a period of 10+ years:
   
   
https://github.com/apache/superset/assets/38889534/8dca8b16-5c05-4b2d-8fd6-67387c8708e7
   
   We can see that the result of shifted queries not always include the same 
periods than the original query, so when the join gets applied, it won't match 
some records, thus giving you incorrect output.
   
   Original query with limit and sorting:
   ![Captura de pantalla 08 04 2024 a 11 05 13 
AM](https://github.com/apache/superset/assets/38889534/bcf9b955-742b-4443-b502-5f0d807cd25f)
   
   1 year ago query with limit and sorting:
   ![Captura de pantalla 08 04 2024 a 11 05 50 
AM](https://github.com/apache/superset/assets/38889534/549b8e45-c90b-4d36-bfc7-b96d8a3baf93)
   
   2 years ago query with limit and sorting:
   ![Captura de pantalla 08 04 2024 a 11 06 08 
AM](https://github.com/apache/superset/assets/38889534/7d3ab934-6129-4719-a9f7-a7772621aa87)
   
   
   
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   Error:
   In the following image some datapoints are missing in the shifted columns 
because they are not part of the result dataframe after sorting and limiting it 
like the original query.
   
![error](https://github.com/apache/superset/assets/38889534/7fe6bdb0-b527-4d40-b067-f79f56f68668)
   
   Fix:
   Now we can see that the query is able to find the records that belong to 
those offsets regardless, because it's joining using the complete data.
   
![test](https://github.com/apache/superset/assets/38889534/4d018be1-ad8c-42b6-a4a0-85123d8f480e)
   
   
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to