Skip to content

fix(sqlserver): detect top-level pagination ordering - #2694

Merged
openai0229 merged 2 commits into
OtterMind:mainfrom
HandSonic:fix/sqlserver-top-level-order-by
Sep 7, 2026
Merged

openai0229 merged 2 commits into
OtterMind:mainfrom
HandSonic:fix/sqlserver-top-level-order-by

Conversation

@HandSonic

@HandSonic HandSonic commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Detect ORDER BY at the top SQL level with the T-SQL lexer before adding SQL Server OFFSET/FETCH pagination.
  • Add the fallback ORDER BY (SELECT NULL) when ordering appears only inside a window expression, nested query, string literal, or comment.
  • Preserve a real outer ORDER BY, including one split by a comment.

Validation

  • SqlServerSqlBuilderTest regression coverage for window, nested query, string/comment, and outer-order cases.
  • Fork CI: Backend tests and package, Frontend lint/test/build, repository checks, JavaScript CodeQL, Java CodeQL, SBOM, and license summary passed.

Latest-main verification (2026-09-04)

  • Rebased onto 144a04ee2; full SQL Server plugin suite passed (83 tests).n- git diff --check` and pairwise integration analysis passed.

@HandSonic
HandSonic requested a review from openai0229 as a code owner August 12, 2026 05:06
@openai0229 openai0229 moved this to In Review in Chat2DB Community Aug 12, 2026
@HandSonic
HandSonic force-pushed the fix/sqlserver-top-level-order-by branch from 80f90a6 to 993f8e0 Compare August 19, 2026 20:01
openai0229
openai0229 previously approved these changes Aug 20, 2026

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lexer-based top-level detection correctly ignores nested queries, window clauses, strings, and comments while preserving a real outer clause. Focused tests cover the relevant regressions. Approved.

@openai0229
openai0229 dismissed their stale review August 20, 2026 03:45

Superseded by a corrected review because shell quoting corrupted the submitted text.

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lexer-based top-level ORDER BY detection correctly ignores nested queries, window clauses, strings, and comments while preserving a real outer clause. Focused tests cover the relevant regressions. Approved.

@HandSonic
HandSonic force-pushed the fix/sqlserver-top-level-order-by branch from 993f8e0 to 3e1ce93 Compare September 3, 2026 22:55
@HandSonic
HandSonic requested a review from Aias00 as a code owner September 3, 2026 22:55
@openai0229
openai0229 merged commit 75e8f0c into OtterMind:main Sep 7, 2026
15 of 16 checks passed
@openai0229 openai0229 moved this from In Review to Done in Chat2DB Community Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants