Fix identifier formatting for migrations when used with Oracledb #6058
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Presently, knex encounters issues in Oracle database when certain migration commands are executed when a custom
wrapIdentifier
option is specified in the knexfile - particularly when this option is used to normalize casing and formatting of database objects (ex. camelCase to UPPER_SNAKE_CASE). The cause of the issue is inconsistent usage of identifier formatting withformatter.wrap()
throughout the oracle schema query compilers, where some parts of the code are formatting identifiers, and others are not in places that alter the same resource. This is problematic as Oracle treats any identifier surrounded in quotes as case-sensitive and knex routinely quotes all identifiers in its queries.A prime example of this is the table and column renaming schema builder methods, which would not format identifiers that were passed-in to these methods. This contrasts with how table and column names are formatted on initial creation. This inconsistency sometimes results in failing queries when the exact same name used during creation and renaming of a table or column were used if the name passed to the query builder was later formatted by
wrapIdentifer
in a way that would alter the casing of the name. An example of such a scenario would look like this:Another instance of inconsistent formatting was causing issues with the
dropTableIfExists()
method as well. The table name would be successfully formatted and dropped, though the auto-generated sequence for a.increments()
id column would fail with an error. This is because knex always generates these auto-generated sequences in all lowercase regardless of what is specified inwrapIdentifier
, while thedropSequenceIfExists()
schema builder compiler for Oracle would always format the sequence name. Building off of the previous example, the following statement would also fail:This was fixed by altering the code path for related sequence deletion to not format the sequence name on deletion. I took this approach instead of removing formatting in
dropSequenceIfExists()
directly as previous changelogs indicated that this may have been previously, or is intended to be used if or when a public API for sequence creation in Oracle is added to knex. If this is not the case, I can refactor that to eliminate some dead code sincedropSequenceIfExists()
is not presently in use by any other part of the codebase (either by removing that function or prefixing it with _ to indicate that it is intended for private use). This approach was taken instead of formatting auto-created triggers going forward to avoid breaking migration commands that manipulate these triggers and sequences (as new implementations would always format these names where prior versions would have always lower_snake_case'd the trigger and sequence name).I had to make several adjustments to some of the existing unit tests for oracle since the code now calls the
wrapIdentifier
function an additional time during the affected tests. I also added several new unit tests to check thatwrapIdentifier
is called properly in the schema builder functions that were affected by the inconsistent formatting issues.Fixes #5801