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-509 Upsert with escaped multiline string fails in prepared statement #2224

Merged
merged 3 commits into from
Sep 11, 2024

Conversation

bog-walk
Copy link
Member

@bog-walk bog-walk commented Sep 1, 2024

Description

Summary of the change:
Statement prepared by upsert() now properly registers parameter arguments in the UPDATE clause.

Detailed description:

  • Why:

Expression arguments in the UPDATE clause were being treated as statement literals. So multiline strings, for example, were being escaped and losing any indentation/formatting when sent to and from the database.

  • How:

Expression arguments are now properly registered during SQL string building. Databases that rely on the MERGE statement for upsert require special treatment because all expressions must be aliased, which is problematic because the expression should be registered but only with its column identifiers replaced by a dynamic alias.


Type of Change

Please mark the relevant options with an "X":

  • Bug fix

Affected databases:

  • All

Checklist

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

Related Issues

EXPOSED-509

@bog-walk bog-walk requested review from joc-a and e5l September 1, 2024 22:43
Copy link
Member

@e5l e5l left a comment

Choose a reason for hiding this comment

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

looks good!

…ed stateme>

The update clause of an upsert statement is being prepared without appropriate
parameterisation, leading to any multiline strings being treated as string lite>
and escaped, thereby losing any indentation.

This occurs because the appropriate registerArgument() invokations are not being used,
particularly for databases that use merge syntax.
…ed statement

- Remove accidentally left in println()
…ed statement

- Add test that covers onUpdate with set array
- Switch new test (passes even with fix) to actual db integration test that
would fail without StatementContext.expandArgs() fix
@bog-walk bog-walk force-pushed the bog-walk/fix-multiline-string-upsert branch from efc3218 to 015c0d1 Compare September 11, 2024 14:33
@bog-walk bog-walk merged commit ffb21de into main Sep 11, 2024
5 checks passed
@bog-walk bog-walk deleted the bog-walk/fix-multiline-string-upsert branch September 11, 2024 15:25
@bog-walk bog-walk mentioned this pull request Sep 11, 2024
14 tasks
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