Override the max_index_name_size to keep local shortening algorithm. (8)

[andynu]

Rails now defaults to its own index name shortening (limited to 62 bytes). This change overrides the rails max to exceed this gems shortening method in order to preserve the historical behavior. names

See https://github.com/rails/rails/commit/3682aa1016e187398a7ae65e521f24f38b710d2b

So the shortening of index names that was added to rails itself was almost immediately changed to use an 'idx_' prefix instead of an 'ix_' prefix.

See https://github.com/rails/rails/commit/59f2b06493bed2c036f6b527e743198416ee1237

However, now that rails does shortening. It is an opportunity to consider simplifying this library and just using that.

Stacked on #2374

[davinlagerroos]

Something to consider: it appears oracle-enhanced has a practice of overriding these kinds of defaults. Following that pattern still gets spec/active_record/connection_adapters/oracle_enhanced/schema_statements_spec.rb:330 to pass and avoids the possibility of causing upgrade problems for someone who might have an existing schema that contains an index that is longer than 62 bytes.

[andynu]

I like this idea, but just overriding the max only works for newer oracle versions and isn't sufficient for oracle 11. https://github.com/rsim/oracle-enhanced/commit/7b36a5ce0b125523157dcf16c36e0d917cf42c98

[davinlagerroos]

Thanks for considering my suggestion! I think approach of aliasing the method still works with oracle 11. The method that would be aliased, max_identifier_length calls suports_long_identifier? which includes an oracle version check. For older versions of oracle suports_long_identifier? would return false and max_identifier_length would return 30.

It looks like in the test that breaks, the else branch of the test reflects this - when we are not testing with @oracle12cr2_or_higher the test's expectation is that the long index gets truncated to 30 characters.

[andynu]

@davinlagerroos I believe I tried what you suggest here https://github.com/rsim/oracle-enhanced/commit/7b36a5ce0b125523157dcf16c36e0d917cf42c98 But it didn't bear out in the tests: https://github.com/rsim/oracle-enhanced/actions/runs/8509850693/job/23306215927

I would welcome a review. Thanks!

[davinlagerroos]

@andynu My bad, I missed the test output! Thanks for making it more obvious. That is definitely not a good outcome.

hmm, when index_name calls super, activerecord uses the max_index_name_size to see if the name is too long and needs to be shortened. The change I suggested means that for oracle < 12.2, this is 30 and super returns a name shortened using activerecord's strategy.

However, oracle enhanced looks to be expecting to get the full activerecord index_name so it can shorten the name using its own strategy (where needed).

Would it work to define max_index_name_size as 128? Even though that is too long for oracle < 12.2, it should mean the call to super would return the full name and then the oracle enhanced could shorten it the way it wants to get to 30.

Or you can follow your original plan. That works too, it just might cause an issue if people have an index name that is longer than the new activerecord default. OTOH I do not know how many people would have an index name that long.

[andynu]

@davinlagerroos good idea! That has worked well and does a better job preserving existing behavior. I will rebase that into this PR tomorrow. https://github.com/andynu/oracle-enhanced/commit/696319271e666ff78eb6e46730bb8500509a80ff

[davinlagerroos]

@andynu Thanks for trying that out! I am glad it worked