Postgresql integer migrations are inconsistent #6272

Closed
bschaeffer opened this Issue May 11, 2012 · 8 comments

Comments

Projects
None yet
4 participants
@bschaeffer
Contributor

bschaeffer commented May 11, 2012

Migrations with integer columns using postgresql are inconsistent.

Take the following migration:

create_table :users do |t|
  t.integer :phone, :limit => 10
  t.integer :zip, :limit => 5
end

After migrating, examining the output of rake db:structure:dump, we get the following sql:

CREATE TABLE users (
    id integer NOT NULL,
    phone integer,
    zip bigint
);

Noticing that, in the migration above, the column phone has a larger limit than the column zip I would expect to see something like the following, where phone is also created as a bigint column:

CREATE TABLE users (
    id integer NOT NULL,
    phone bigint,
    zip bigint
);

I don't think I am expecting the wrong behavior, but maybe I am missing something with postgres...

@erichmenge

This comment has been minimized.

Show comment Hide comment
@erichmenge

erichmenge May 11, 2012

That is a really really big number you're requesting space for there.

You shouldn't be using integers for stuff like phone numbers and zip codes, use a string. You're specifying integer bytes not length of string.

As a general rule, only use integers or decimals for things you need to perform mathematical operations with, or numerical comparisons. Since you'll likely never be comparing whether one zip code is greater than or less than another, or multiplying zip codes, use a string.

P.S. http://www.postgresql.org/docs/8.1/static/datatype.html shows Postgres' support for integers. Max integer size is 8 bytes.

That is a really really big number you're requesting space for there.

You shouldn't be using integers for stuff like phone numbers and zip codes, use a string. You're specifying integer bytes not length of string.

As a general rule, only use integers or decimals for things you need to perform mathematical operations with, or numerical comparisons. Since you'll likely never be comparing whether one zip code is greater than or less than another, or multiplying zip codes, use a string.

P.S. http://www.postgresql.org/docs/8.1/static/datatype.html shows Postgres' support for integers. Max integer size is 8 bytes.

@erichmenge

This comment has been minimized.

Show comment Hide comment
@erichmenge

erichmenge May 11, 2012

To give you an example, a limit of 8 is 2 to the power of 8*8 (8 bits in a byte). Divide that in half to get your max value for signed integers: 9,223,372,036,854,775,807

A limit of 10 is a max value of 604,462,909,807,314,587,353,088

To give you an example, a limit of 8 is 2 to the power of 8*8 (8 bits in a byte). Divide that in half to get your max value for signed integers: 9,223,372,036,854,775,807

A limit of 10 is a max value of 604,462,909,807,314,587,353,088

@bschaeffer

This comment has been minimized.

Show comment Hide comment
@bschaeffer

bschaeffer May 11, 2012

Contributor

Thats makes sense... apparently I don't understand anything about databases!

Still, I think the behavior is inconsistent.

Contributor

bschaeffer commented May 11, 2012

Thats makes sense... apparently I don't understand anything about databases!

Still, I think the behavior is inconsistent.

@Sheeo

This comment has been minimized.

Show comment Hide comment
@Sheeo

Sheeo May 11, 2012

If postgres integer size is 8 bytes, then the migration produced a wrong result, not only inconsistent..

The docs say that http://editor.datatables.net/release/DataTables/extras/Editor/examples/inlineControls.html limit indeed specifies the number of maximum bytes, so phone should've been bigint and zip just int.

EDIT: Looking at postgres documentation (http://www.postgresql.org/docs/8.1/static/datatype.html#DATATYPE-INT), phone should've actually been of type numeric and zip of type integer. Also I do agree with @erichmenge, but choice of datatype is another issue.

Sheeo commented May 11, 2012

If postgres integer size is 8 bytes, then the migration produced a wrong result, not only inconsistent..

The docs say that http://editor.datatables.net/release/DataTables/extras/Editor/examples/inlineControls.html limit indeed specifies the number of maximum bytes, so phone should've been bigint and zip just int.

EDIT: Looking at postgres documentation (http://www.postgresql.org/docs/8.1/static/datatype.html#DATATYPE-INT), phone should've actually been of type numeric and zip of type integer. Also I do agree with @erichmenge, but choice of datatype is another issue.

@erichmenge

This comment has been minimized.

Show comment Hide comment
@erichmenge

erichmenge May 11, 2012

@Sheeo zip was specified as a limit of 5, postgres (and MySQL for that matter) have an integer size of 4 bytes, so 5 will go up to the next size, bigint, which is 8 bytes. zip is correctly bigint.

It would probably be best if the migration raised an exception if limit was out of range.

@Sheeo zip was specified as a limit of 5, postgres (and MySQL for that matter) have an integer size of 4 bytes, so 5 will go up to the next size, bigint, which is 8 bytes. zip is correctly bigint.

It would probably be best if the migration raised an exception if limit was out of range.

@erichmenge

This comment has been minimized.

Show comment Hide comment
@erichmenge

erichmenge May 11, 2012

# Maps logical Rails types to PostgreSQL-specific data types.
      def type_to_sql(type, limit = nil, precision = nil, scale = nil)
        return super unless type.to_s == 'integer'
        return 'integer' unless limit

        case limit
          when 1, 2; 'smallint'
          when 3, 4; 'integer'
          when 5..8; 'bigint'
          else raise(ActiveRecordError, "No integer type has byte size #{limit}. Use a numeric with precision 0 instead.")
        end
      end

It looks like it should have raised.

# Maps logical Rails types to PostgreSQL-specific data types.
      def type_to_sql(type, limit = nil, precision = nil, scale = nil)
        return super unless type.to_s == 'integer'
        return 'integer' unless limit

        case limit
          when 1, 2; 'smallint'
          when 3, 4; 'integer'
          when 5..8; 'bigint'
          else raise(ActiveRecordError, "No integer type has byte size #{limit}. Use a numeric with precision 0 instead.")
        end
      end

It looks like it should have raised.

@erichmenge

This comment has been minimized.

Show comment Hide comment
@erichmenge

erichmenge May 11, 2012

      def sql_type
        base.type_to_sql(type.to_sym, limit, precision, scale) rescue type
      end

Why is this being rescued?

      def sql_type
        base.type_to_sql(type.to_sym, limit, precision, scale) rescue type
      end

Why is this being rescued?

@erichmenge

This comment has been minimized.

Show comment Hide comment
@erichmenge

erichmenge May 11, 2012

cc/ @jeremy

lest pushed a commit to lest/rails that referenced this issue May 16, 2012

Merge pull request #6349 from erichmenge/patch-raise-type-errors
Integer limit out of range should be allowed to raise. Closes #6272

bf4 added a commit to bf4/acts-as-taggable-on that referenced this issue Dec 10, 2013

Update test schema.rb to match current migration
However, set force: true and didn't set indices

Fixes test failures for postgresql::

No integer type has byte size 11. Use a numeric with precision 0
instead.

https://travis-ci.org/mbleigh/acts-as-taggable-on/jobs/15196579

see rails/rails#6272

bf4 added a commit to mbleigh/acts-as-taggable-on that referenced this issue Dec 10, 2013

Update test schema.rb to match current migration
However, set force: true and didn't set indices

Fixes test failures for postgresql::

No integer type has byte size 11. Use a numeric with precision 0
instead.

https://travis-ci.org/mbleigh/acts-as-taggable-on/jobs/15196579

see rails/rails#6272
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment