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

Remove overriden primary_keys method #44

Merged
merged 8 commits into from
Apr 1, 2020

Conversation

alimi
Copy link
Contributor

@alimi alimi commented Mar 27, 2020

We can now remove the overridden primary_keys and use the PostgreSQL Adapter's definition.

This PR also includes a couple of other primary key changes.

  • Redfine PostgreSQLColumn#serial? so it checks for unique_rowid() instead of a sequence. 986dc9d
  • Ignore CockroachDB's use of rowid as a default primary key because it breaks a lot of ActiveRecord's assumptions about tables that don't have primary keys. 5dab156

The rest of the changes address tests that were failing in the ActiveRecord primary keys test. Failing tests have been excluded and replaced with appropriate tests for CockroachDB.

  * We can use the PostgreSQL Adapter's definition.
  * In the PostgreSQL Adapter, PostgreSQLColumn#serial? checks to see if the
    column's default function is sequence. In CockroachDB, serial columns will
    have a default function that uses unique_rowid().
  * ActiveRecord allows for tables to exist without primary keys. Databases like
    PostgreSQL support this behavior, but CockroachDB does not. If a table is
    created without a primary key, CockroachDB will add a rowid column to serve
    as its primary key. This breaks a lot of ActiveRecord's assumptions so we'll
    treat tables with rowid primary keys as if they didn't have primary keys at
    all.
  * In PostgreSQL, serial columns are backed by integer columns. They're also
    backed by integer columns in CockroachDB, but integer columns are the same
    size as PostgreSQL's bigints. Therefore, bigint? will be true for serial
    columns in the CockroachDB Adpater while it will be false in the PostgreSQL
    Adapter.
  * The ActiveRecord test creates a table with a serial primary key expecting
    the serial info to be included in the schema dump. That info will not be
    included in the schema dump because CockroachDB treats serial and bigserial
    columns as bigserial columns. ActiveRecord defaults to bigserial primary
    keys so it doesn't need to include that info in the schema dump.
integer test

  * The AR test fails because CockroachDB's INT type translates to PostgreSQL's
    bigint type. Although the test table is created with an integer primary key,
    it will come back as BIGINT from CockroachDB. So the schema dump will show
    it as a bigint.
Copy link
Contributor

@apantel apantel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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.

2 participants