Shorten ActiveRecord::InternalMetadata.table_name to ar_internal_metadata

[yahonda]

This pull request addresses the following error when tested with Oracle enhanced adapter.

It changes ActiveRecord::InternalMetadata.table_name to ar_internal_metadatas to support Oracle database which only supports 30 byte identifier length.

ActiveRecord::StatementInvalid:
OCIError: ORA-00972: identifier is too long: CREATE TABLE "ACTIVE_RECORD_INTERNAL_METADATAS" ("KEY" VARCHAR2(255), "VALUE" VARCHAR2(255), "CREATED_AT" TIMESTAMP NOT NULL, "UPDATED_AT" TIMESTAMP NOT NULL)

I think "ar_internal_metadatas" can explain what it intends.

[rails-bot]

r? @arthurnn

(@rails-bot has picked a reviewer for you, use r? to override)

[matthewd]

I'm leaning towards renaming it schema_metadata, so it'll visually live with schema_migrations (and avoid the awkward "metadatas").

[yahonda]

Thanks for the suggestion. As long as the table name length is shorter than 26 byte considering prefix and suffix support which requires 4 bytes. I'm fine with schema_metadata, ar_internal_metadatas or whatever.

[yahonda]

I've been thinking about which schema_metadata or ar_internal_metadatas table name is a better choice.

Looks like ActiveRecord::InternalMetadata is aiming to manage Rails environments to avoid tables dropped unintentionally, schema_metadata is little bit narrow. I think. Then I prefer ar_internal_metadatas.

This pull request is important for Oracle since it's identifier length is limited to 30 bytes (Yes, it is way short, but it is what it is so far..), and there are some use cases who develops their applications using non Oracle database and migrate it to Oracle at production environment.

It would be appreciated this pull request reviewed.

[sgrif]

This will need to check for existing tables with the old name and rename them if they exist

[yahonda]

@sgrif Thanks for the review. I have a question.

As far as I understand ActiveRecord::InternalMetadata is introduced in Rails 5.0, which is in beta status. I'm wondering if it still needs to implement a feature to handle if the old table name exist or not.

[sgrif]

Given how easy it is to rename, I think it's worth it for people who are
currently on the beta.

On Tue, Jan 26, 2016, 3:07 PM Yasuo Honda notifications@github.com wrote:

@sgrif https://github.com/sgrif Thanks for the review. I have a
question.

As far as I understand ActiveRecord::InternalMetadata is introduced in
Rails 5.0, which is in beta status. I'm wondering if it still needs to
implement a feature to handle if the old table name exist or not.

—
Reply to this email directly or view it on GitHub
#23025 (comment).

[yahonda]

Let me try to implement renaming and update this pull request.

[yahonda]

Updated this pull request to include rename active_record_internal_metadatas to ar_internal_metadatas.

[sgrif]

Can we add a note to remove this method in 5.1?

[yahonda]

Sure. Let me add one line comment in this method.

[yahonda]

Comment added to mention that this method will be removed in Rails 5.1

[maclover7]

Can you change the table name to be ar_internal_metadata, (remove the "s" at the end)? I think the previous name had a grammar error. :)

[yahonda]

Thanks for the review and advice. Updated to use ar_internal_metadata, removing the trailing "s"