Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix!: EXPOSED-569 groupConcat uses wrong SQLite syntax & ignores DISTINCT in Oracle & SQL Server #2257

Merged
merged 2 commits into from
Sep 28, 2024

Conversation

bog-walk
Copy link
Member

Description

Summary of the change:
groupConcat() no longer generates wrong SQL syntax for SQLite and no longer ignores unsupported parameters for Oracle and SQL Server.

Detailed description:

  • Why:

It was observed by a user that SQLite uses the wrong syntax for a separator in its GROUP_CONCAT funtion. They were also attempting to use the ORDER BY clause, which Exposed prevents because the database used to not support it. ORDER BY with aggregate functions has been supported since version 3.44.0.

While adjusting the tests to no longer exclude, it was confirmed that SQL Server doesn't actually support DISTINCT in the function. Exposed was ignoring this value if set to true and not including it in SQL, whereas the usual behavior is to fail early.

Oracle was doing the same thing for DISTINCT. But it was also, for some reason, preventing groupConcat() from being used unless an argument for orderBy was provided. ORDER BY is supposed to be optional with LISTAGG function.

  • How:
    • [SQLite] Uses correct (expr, '?') syntax instead of (expr, SEPARATOR '?'). Allows arguments to be passed to orderBy for syntax (expr, '?' ORDER BY col).
    • [SQL Server] Throws UnsupportedByDialectException if dialect is set to true.
    • [Oracle] Allows function to be called without passing argument to orderBy. Throws UnsupportedByDialectException if dialect is set to true.

Type of Change

Please mark the relevant options with an "X":

  • Bug fix

Updates/remove existing public API methods:

  • Is breaking change

Affected databases:

  • SQLite
  • SQL Server
  • Oracle

Checklist

  • Unit tests are in place
  • The build is green (including the Detekt check)

Related Issues

EXPOSED-569

…INCT in

 Oracle & SQL Server

- SQLite syntax now uses correct separator syntax and allows ORDER BY clause
- Oracle & SQL Server now fail early with unsupported by dialect exception if
distinct = true (before it would just be ignored by SQL builder).
…INCT in

Oracle & SQL Server

- Fixed wrong function name in exception message
- Change to use appendTo() with prefix
@bog-walk bog-walk force-pushed the bog-walk/fix-group-concat branch from 704093e to d08b38f Compare September 28, 2024 14:42
@bog-walk bog-walk merged commit 7ee085b into main Sep 28, 2024
5 checks passed
@bog-walk bog-walk deleted the bog-walk/fix-group-concat branch September 28, 2024 16:02
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