Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow unmanaged autogenerated sequence names #2345

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

matthewtusker
Copy link

To still support primary keys through db triggers, without adding back in support for generating and maintaining them with migrations, add an configuration boolen:

unmanaged_autogenerated_sequences = false # default

Which can be used in an initializer file. When set to true there will not be an exception raised when the sequence name is defined in the model with the previously supported value:

self.sequence_name = :autogenerated

This option allows for dbs managed outside of the application to use triggers for primary keys. Without the autogenerated option an error is raised when inserting the record:

ORA-02289: sequence does not exist

To still support primary keys through db triggers, without adding back in
support for generating and maintaining them with migrations, add an
configuration boolen:

    unmanaged_autogenerated_sequences = false # default

Which can be used in an initializer file. When set to `true` there will not be
an exception raised when the sequence name is defined in the model with the
previously supported value:

   self.sequence_name = :autogenerated

This option allows for dbs managed outside of the application to use triggers
for primary keys. Without the `autogenerated` option an error is raised when
inserting the record:

    ORA-02289: sequence does not exist
@matthewtusker
Copy link
Author

A bit of backstory here: oracle-enhanced was used at my current workplace until #1669 was implemented, which dropped support for trigger-based primary keys. This broke the app using it, and a fork was made with this change to keep things running. The database is managed outside of Rails so we don't care about schema dumps containing information about triggers (which was previously implemented), we just want to ignore the next_sequence_value as was being done before.

This isn't my code, I'm just trying to upstream it to prevent us having to maintain a fork with a single unique commit. We're also using the release61 branch because we're on Rails 6.1, but we want (well, need) to upgrade Rails to 7, which would be much easier if this change was included here (and backmerged as appropriate).

I'm also aware that this change is untested. I can look into adding these but may need some advice on the best place to add them.

@emm26
Copy link

emm26 commented Feb 2, 2024

My workplace could also benefit from the changes in this PR being merged in!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants