Skip to content

Conversation

anshal21
Copy link
Contributor

Description

This PR fixes a bug in the SQL parser where the DISTINCT keyword was being lost during parsing and reconstruction when it appears right after the SELECT keyword. For example:

-- Before: This would lose the DISTINCT keyword
SELECT DISTINCT record_id FROM records
-- Would become:
SELECT record_id FROM records

-- After: The DISTINCT keyword is preserved
SELECT DISTINCT record_id FROM records
-- Remains as:
SELECT DISTINCT record_id FROM records

Note that this fix only affects the case where DISTINCT appears right after SELECT. The parser already correctly handles DISTINCT in other contexts, such as in aggregate functions:

SELECT count(DISTINCT(RECORD_ID)) FROM RECORD_TABLE

Changes

  • Added HasDistinct field to SelectQuery struct
  • Modified parseSelectStmt to store the DISTINCT information
  • Updated String() method to include DISTINCT in the output
  • Added test cases to verify the fix

Testing

Added test cases to verify that:

  1. SELECT DISTINCT record_id FROM records preserves the DISTINCT keyword
  2. SELECT count(DISTINCT(RECORD_ID)) FROM RECORD_TABLE continues to work as before

@coveralls
Copy link

Pull Request Test Coverage Report for Build 14959292031

Details

  • 5 of 5 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 62.818%

Totals Coverage Status
Change from base Build 14919903301: 0.01%
Covered Lines: 7057
Relevant Lines: 11234

💛 - Coveralls

Copy link
Member

@git-hulk git-hulk left a comment

Choose a reason for hiding this comment

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

LGTM, good catch!

@git-hulk git-hulk merged commit 0254798 into AfterShip:master May 12, 2025
1 check passed
@git-hulk
Copy link
Member

@anshal21 Thanks for your contribution.

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.

3 participants