Skip to content

[Refactor] Query Formatter Test Case Fixes#102

Merged
colinthebomb1 merged 1 commit intomainfrom
colin/formatter-test-cases
Mar 31, 2026
Merged

[Refactor] Query Formatter Test Case Fixes#102
colinthebomb1 merged 1 commit intomainfrom
colin/formatter-test-cases

Conversation

@colinthebomb1
Copy link
Copy Markdown
Collaborator

Overview

This PR updates the query formatter logic to support more complex test cases. All existing test cases now pass.

TODOs

Add UNION test cases and support them in formatter

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR expands QueryFormatter’s AST→mo_sql_parsing JSON conversion to cover additional SQL constructs so previously-disabled formatter test cases can be asserted again.

Changes:

  • Re-enabled many tests/test_query_formatter.py assertions by comparing formatted SQL vs expected queries via mo_sql_parsing.parse().
  • Enhanced core/query_formatter.py to support SELECT DISTINCT, DISTINCT ON, subqueries in FROM/JOIN, NULL/IS NULL, CASE, IN lists, arithmetic operators, and INTERVAL.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
tests/test_query_formatter.py Uncomments/enables formatter assertions across many query fixtures.
core/query_formatter.py Adds/adjusts formatting logic for DISTINCT, DISTINCT ON, subquery sources, more operators/expressions, and interval handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@baiqiushi baiqiushi left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Copy Markdown
Collaborator

@HazelYuAhiru HazelYuAhiru left a comment

Choose a reason for hiding this comment

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

Overall LGTM

@colinthebomb1 colinthebomb1 merged commit a1f3337 into main Mar 31, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants