Skip to content

Fix SQL emitted for compound queries - #5

Merged
jgaskins merged 2 commits into
jgaskins:masterfrom
concentric-health:fix-compond-queries
Aug 16, 2026
Merged

Fix SQL emitted for compound queries#5
jgaskins merged 2 commits into
jgaskins:masterfrom
concentric-health:fix-compond-queries

Conversation

@mloughran

Copy link
Copy Markdown
Contributor

Two small fixes, each with a regression spec:

  • first(n) on a compound query emitted no space before LIMIT.
  • The & combinator emitted INTERSECTION, which isn't a SQL keyword.

Both specs fail on master and pass with the fixes.

Previously SQL like "... = $2LIMIT $3", was emitted, resulting in an error like:

> trailing junk after parameter at or near "$2LIMIT" (PQ::PQError)
The & combinator used the wrong keyword, resulting in an error like:

> syntax error at or near "INTERSECTION" (PQ::PQError)
Comment thread src/query_builder.cr

def &(other : self) : CompoundQuery
CompoundQuery.new(self, "INTERSECTION", other, connection(CONFIG.read_db))
CompoundQuery.new(self, "INTERSECT", other, connection(CONFIG.read_db))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I make this exact mistake every time I need an INTERSECT query. Well, at least I'm consistent.

I'm surprised I've never needed to do it with Interro, though.

Comment thread src/query_builder.cr
str << rhs
if @limit
str << "LIMIT $" << (arg_count += 1)
str << " LIMIT $" << (arg_count += 1)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

💯

@jgaskins
jgaskins merged commit 8bfc2a2 into jgaskins:master Aug 16, 2026
30 checks passed
@jgaskins

Copy link
Copy Markdown
Owner

Thanks!

@mloughran
mloughran deleted the fix-compond-queries branch August 24, 2026 22:41
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.

2 participants