Skip to content

fix: add SQLExpressions.stringAgg and ensure proper spacing for WITHI…#1258

Closed
renechoi wants to merge 1 commit intoOpenFeign:masterfrom
renechoi:Fix/correct-SQL-for-string_agg-WITHIN-GROUP-ORDER-BY-via-SQLExpressions.stringAgg
Closed

fix: add SQLExpressions.stringAgg and ensure proper spacing for WITHI…#1258
renechoi wants to merge 1 commit intoOpenFeign:masterfrom
renechoi:Fix/correct-SQL-for-string_agg-WITHIN-GROUP-ORDER-BY-via-SQLExpressions.stringAgg

Conversation

@renechoi
Copy link
Contributor

@renechoi renechoi commented Jun 28, 2025

📝 Pull-Request Description

What & Why

PostgreSQL’s string_agg( … ) WITHIN GROUP (ORDER BY …) was rendered without the required space before WITHIN GROUP, producing invalid SQL such as

string_agg(name,';')WITHIN GROUP(ORDER BY id)

The root cause is documented in the upstream bug report
➡️ [querydsl/querydsl #3851](querydsl/querydsl#3851).

This PR ports the fix to the openfeign/querydsl fork so downstream consumers get the corrected SQL generation.


Changes in this PR

Type Module / File Summary
Feature querydsl-sql/src/main/java/com/querydsl/sql/dsl/SQLExpressions.java Added stringAgg(Expression<String>, Expression<?> separator) convenience method, mirroring other aggregate helpers and enabling fluent withinGroup().orderBy(… ) chaining.
🛠 Bug-fix querydsl-sql/src/main/java/com/querydsl/sql/PostgreSQLTemplates.java Ensures a single whitespace is emitted between string_agg and WITHIN GROUP, producing valid SQL.
Test querydsl-sql/src/test/java/.../StringAggWithinGroupOrderByTest.java Verifies generated SQL matches string_agg(foo.name,';') WITHIN GROUP (ORDER BY foo.id asc); fails on old code, passes after fix.

Before vs. After

Generated SQL
❌ Before string_agg(foo.name,';')WITHIN GROUP(ORDER BY foo.id asc)
✅ After string_agg(foo.name,';') WITHIN GROUP (ORDER BY foo.id asc)

Compatibility

  • Non-breaking – only an additive API (stringAgg) and template fix.
  • Change is PostgreSQL-specific; other dialects unchanged.

Tests & CI

  • New unit test added; all existing SQL-module tests pass locally.
  • Current CI failure relates to easy-jacoco-maven-plugin resolution and is unrelated to this patch. I can follow up with a workflow or version pin if desired.

Related


🤝 Thanks for reviewing!

…N GROUP ORDER BY

* Added SQLExpressions.stringAgg(...) convenience method that wraps PostgreSQL
  string_agg, enabling fluent use with withinGroup().orderBy().
* Updated PostgreSQLTemplates to emit the mandatory space before WITHIN GROUP
  so generated SQL is syntactically correct.
* Added unit test StringAggWithinGroupOrderByTest to prevent regressions and
  document expected query output.
@velo
Copy link
Member

velo commented Jul 8, 2025

Hi @renechoi , thanks for your contributions, can you please take a look and fix the issues on CI. I'm happy to review the changes after we get a green build

@velo
Copy link
Member

velo commented Jul 24, 2025

Build is red, feel free to reopen when you get it working.

thanks for the support

@velo velo closed this Jul 24, 2025
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