redash icon indicating copy to clipboard operation
redash copied to clipboard

Add limit option for MSSQL query runner

Open dvandonkelaar opened this issue 1 year ago • 2 comments

What type of PR is this?

  • [ ] Refactor
  • [ ] Feature
  • [X] Bug Fix
  • [ ] New Query Runner (Data Source)
  • [ ] New Alert Destination
  • [ ] Other

Description

The limit query adds LIMIT 1000 to the end of the query, but this isn't working for MSSQL queries. This change adds the possibility to add the query runner specified limit text after the SELECT statement, based on the limit_after_select variable.

How is this tested?

  • [X] Unit tests (pytest, jest)
  • [ ] E2E Tests (Cypress)
  • [ ] Manually
  • [ ] N/A

Related Tickets & Documents

This issue is also discussed in #5773. No other issues

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

N/A

dvandonkelaar avatar Jan 14 '24 16:01 dvandonkelaar

Thanks for this @dvandonkelaar. It looks like this needs some formatting tweaks in order to pass the CI testing though.

Are you ok to get that fixed?

justinclift avatar Jan 15 '24 00:01 justinclift

Thanks for the quick reply! My apologies, apparently I ignored the non-installed linter errors. Commint 7e63a0fd872a2ac119677b0f8b73fa3693e355fc fixes the errors.

dvandonkelaar avatar Jan 15 '24 05:01 dvandonkelaar

Codecov Report

Attention: Patch coverage is 62.50000% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 63.41%. Comparing base (2fe0326) to head (9f26303).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6704      +/-   ##
==========================================
- Coverage   63.42%   63.41%   -0.01%     
==========================================
  Files         162      162              
  Lines       13173    13186      +13     
  Branches     1819     1822       +3     
==========================================
+ Hits         8355     8362       +7     
- Misses       4522     4527       +5     
- Partials      296      297       +1     
Files Coverage Δ
redash/query_runner/mssql.py 34.56% <100.00%> (+2.51%) :arrow_up:
redash/query_runner/mssql_odbc.py 36.14% <100.00%> (+2.39%) :arrow_up:
redash/query_runner/__init__.py 76.63% <40.00%> (-1.39%) :arrow_down:

codecov[bot] avatar Feb 26 '24 19:02 codecov[bot]

@dvandonkelaar Thanks for getting this done, it fixes an important bug with the MSSQL driver. :smile:

justinclift avatar Feb 26 '24 20:02 justinclift