reposilite-playground / exposed-upsert

Upsert DSL extension for Exposed, Kotlin SQL framework
The Unlicense
34 stars 3 forks source link

fix(sql): Sort columns alphabetically so that it works properly #7

Closed tecc closed 1 year ago

tecc commented 1 year ago

There's a very specific bug with this library - the DO UPDATE SET part of the generated query has the fields ordered as the user specified them, whereas the replacement is set based on the key (i.e. they're alphabetically ordered). This PR just fixes that bug, plus makes that subset of the code slightly more maintainable.

I am not entirely sure if this has introduced another bug or not, but it seems to work so it's probably fine.

tecc commented 1 year ago

As a side effect, the order of fields that the user specifies in the insert/update statements should now be irrelevant. Yay for usability!

dzikoysk commented 1 year ago

Thanks, could you also add some kind of test-case scenario, so we can track changes related to this part of impl? :)

I guess this is related to #6

tecc commented 1 year ago

It most likely is related, at a guess. I added a test case for it (although I'm unsure if it's correct or not), which makes all records in statisticsRepository have a count equal to one and have any xyz in their URI substituted with abc.

dzikoysk commented 1 year ago

Looks like the test failed on CI for SQLiteUpsertTest 🤔

java.lang.ArrayIndexOutOfBoundsException: Index 4 out of bounds for length 4
tecc commented 1 year ago

I debugged the code I wrote and I realised the problem: update statements cannot update any key in indexColumns, which caused a problem when I tried to update uri.

dzikoysk commented 1 year ago

Thanks! Sorry for late response, I'm slightly busy lately 😅 I'll release 1.2.0, so it should be available within an hour on Maven Central :)