Upper case result columns only for case-insensitive databases

[fbiville]

Impact

• Bug fix (non-breaking change which fixes expected existing functionality)
• Enhancement/New feature (adds functionality without impacting existing logic)
• Breaking change (fix or feature that would cause existing functionality to change)

Description

Updates the "ResultSet into a Map" logic to preserve the column casing for case sensitive databases instead of always uppercasing them.

Marked as a breaking change, because if any code is using queries that return non-uppercased column names but then is reading from the map as upper case it will no longer get the column's value. That old behavior was wrong and this fixes it, but there may be 3rd party code that relies on that buggy behavior.

Fixes #3071

[nvoxland]

Code review and test results:

Things to be aware of:

• Only changes behavior if the database says it supports mixed case identifiers
• Current behavior is wrong in that case and likely not working, and this fixes that
• Relies on the driver to be correct about whether it supports mixed case identifiers
• The test harness and functional tests failed because the pro code that gets function definitions in mysql were getting the value from uppercase column names, but mysql says it's case sensitive and the actual column names are mixed case.
  • I create a fix in pro but the logic for combining the pro branch with the liquibase branch isn't working correctly with a fork PR like this, so while the automated tests run correctly for me locally I don't think I can get a build showing it does.
  • I think the best we can do is make sure right away things are running correctly right away after merging the PRs and react quickly if it does not

Things to worry about:

• Are there any other database configurations where the database is case sensitive and the code is currently incorrectly looking for upper-cased column names?
  • I think our existing tests should catch those well enough, so given that all the tests pass (at least locally with a correct build) I'm not too worried about that anymore

[github-actions]

Unit Test Results

  4 620 files  ±0    4 620 suites  ±0   34m 11s ⏱️ - 2m 59s
  4 617 tests ±0    4 402 ✔️ +4     215 💤  - 4  0 ❌ ±0 
54 576 runs  ±0  49 556 ✔️ +4  5 020 💤  - 4  0 ❌ ±0 

Results for commit ec1f70b. ± Comparison against base commit e742a88.

♻️ This comment has been updated with latest results.

[nvoxland]

Thanks for doing the update @fbiville . That may be the cause of the failing tests from before, I'll keep an eye on the new build

[nvoxland]

Our build process isn't correctly connecting some of the needed changes in our pro code to get a fully clean build with this.

We have plans to improve that build process to be able to work with a setup like this where the liquibase-side PR is from a fork, but we don't want to hold this PR up for that work.

So since the changes should be good due to me running most of the automated tests locally, I'll merge the PRs to master at a non-busy time in order to get a complete build that gets correctly automatically tested and if there is any surprising failures we can react then by quicky fixing whatever problems are found then or by reverting the changes until we can address them.

[fbiville]

@nvoxland would it help if the branch is pushed to the main repository instead of a fork?

[nvoxland]

@fbiville I don't think it's work making a separate branch/PR for this in the main repo. We're working on improving the build process currently so hopefully it shouldn't be an issue going forward.

[XDelphiGrl]

This fix prevents Liquibase from snapping column names TO UPPER when the database platform is case sensitive.

APPROVED

Note: We had some build issues that prevented us from running all functional tests in a branch. The plan is to merge this to liquibase/liquibase master & re-run the tests. @nvoxland and I will be watching to see if there are adverse effects due to the merge. If so, we will fix or revert (depending on severity).