Skip to content

Conversation

@artembilan
Copy link
Member

@artembilan artembilan commented Dec 9, 2021

SO: https://stackoverflow.com/questions/70286480/how-can-spring-integration-5-5-x-use-mysql-as-message-store

Turns out the condition word is reserved in many SQL DB vendors, e.g.:
https://dev.mysql.com/doc/refman/8.0/en/keywords.html

  • Fix SQL scripts for quoting CONDITION column name for
    those vendors which have it as a reserved word
  • Fix JdbcMessageStore to have a CONDITION quoted to "" by default and
    replaced to "`" for MySQL
  • Add MySqlContainerTest to test against MySQL Docker container
  • Looks like quoted identifiers work well even if they are not reserved words in the SQL vendor

@artembilan
Copy link
Member Author

Copy link
Contributor

@garyrussell garyrussell left a comment

Choose a reason for hiding this comment

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

I fixed a few typos in the migration guide.

+ " values (?, ?, 0, 0, ?, ?)"),

UPDATE_MESSAGE_GROUP("UPDATE %PREFIX%MESSAGE_GROUP set UPDATED_DATE=?, CONDITION=? " +
UPDATE_MESSAGE_GROUP("UPDATE %PREFIX%MESSAGE_GROUP set UPDATED_DATE=?, GROUP_CONDITION=? " +
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a big breaking change in a point release; I think it's ok to change the scripts but I think this code needs to be made conditional on a property (defaulting to CONDITION in the point release and GROUP_CONDITION in 6.0).

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. How about to make it "smart" conditional: based on vendor derived from the provided DataSource?
All those which have this condition as a reserved word are not going to work anyway.
So, we change the word exactly in their scripts and check for them in the JdbcMessageStore.
This way we would leave the rest of supported vendors working as before this change.

@artembilan
Copy link
Member Author

I have reproduced the issue with MySQL under Testcontainers:

Caused by: java.sql.SQLSyntaxErrorException: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'CONDITION VARCHAR(255), COMPLETE BIGINT, LAST_RELEASED_SEQUENCE BIGINT, CREATED_' at line 1

This fixes the problem:

 `CONDITION` VARCHAR(255),

Now looking for the possible fix in the rest of scripts and JdbcMessageStore...

@artembilan artembilan force-pushed the Fix_condition_for_SQL branch from 7dd07c3 to 7cc9ff4 Compare December 10, 2021 19:22
SO: https://stackoverflow.com/questions/70286480/how-can-spring-integration-5-5-x-use-mysql-as-message-store

Turns out the condition word is reserved in many SQL DB vendors, e.g.:
https://dev.mysql.com/doc/refman/8.0/en/keywords.html

* Fix SQL scripts for quoting `CONDITION` column name for
those vendors which have it as a reserved word
* Fix `JdbcMessageStore` to have a `CONDITION` quoted to `""` by default and
replaced to "`" for MySQL
* Add `MySqlContainerTest` to test against MySQL Docker container
* Looks like quoted identifiers work well even if they are not reserved words in the SQL vendor
@artembilan artembilan force-pushed the Fix_condition_for_SQL branch from 7cc9ff4 to f712be5 Compare December 10, 2021 19:24
@artembilan artembilan changed the title Change CONDITION to GROUP_CONDITION Quote CONDITION whenever necessary for JDBC Dec 10, 2021
@artembilan
Copy link
Member Author

Pushed the fix which is more flexible according the current state of things:

  1. If vendor doesn't fail for CONDITION identifier, it won't fail with this fix.
  2. If it fails, then script is updated.
  3. MySQL has a special quotation rules.

@garyrussell garyrussell merged commit b424cbe into spring-projects:main Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants