PostgreSQL incompatibility in content.install

This patch patches the following two files to fix the problem:

content_admin.inc
Changed all functions that were creating unsigned ints in tables in pgsql to create BIGINT columns with CHECK(column > 0) functions on them. This should fix the main problem.

secondary issue:
Line 873: The bigint type should not be downgraded to integer type, it should stay bigint type.
This doesn't seem to have an effect on CCK just yet.

Why? Because the code in this module only seems to create ints regardless of min and max values for the integer data type. Someone correct me if I'm wrong, but if I give it a bigint value like over 4 billion for a max value, it blithely creates an integer type in the database.

Modified with a new function:
function content_db_construct_column_type
Centralizes the logic for deciding what a data type from the front end becomes in the database back end. This code was repeated in two places. Updated the code to handle unsigned ints by turning them into a BIGINT with a CHECK(column > 0) for postgresql.
This code doesn't appear ever to be called or used from what I can tell. Again, integer support in the code doesn't seem to support anything but regular integers, so this code doesn't seem to be used at all.

content.install
Changed all functions that were creating unsigned ints in tables in pgsql to create BIGINT columns with CHECK(column > 0) functions on them. This should fix the main problem.

I've added a description of my complaint about database integer types not being used properly when creating integers here: http://drupal.org/node/76432

This is an updated patch that corrects a bug in the previous patch related to text fields, and works off of the 8/4 cvs version of the CCK module.

Thanks for putting this patch through Paul! 'Twas very annoying for me when I installed CCK (such a highly talked about module) and it didn't work! Talk about poor form ;)

I disagree with the use of bigint's where the unsigned integer fields are throughout all your code. Please amend this to be integer with the CHECK attribute. Also please leave them in lowercase so they match what is already in the code written by the maintainers et al.

After that patch is submitted i'll review it for you.

Cheers,

Sammy Spets
Synerger
http://www.synerger.com

Sammy,

I'll have to rebuild the patch to fit the latest cvs version. It's bigger than just the BIGINT's, since it consolidates repeated code in field addition and modification as well.

As for BIGINT vs integer, it's a matter of capacity. Unsigned integers in MySQL can hold up to 4 billion while regular integers in MySQL can hold values of up to 2 billion (http://mysql.com/doc/refman/5.0/en/numeric-types.html). PostgreSQL integers are also limited to 2 billion, so tacking on the "check(col > 0)" rule will not support a 3 billion value that it would on MySQL. This is why I chose BIGINTs with the check function.

If you still think I should demote them to be regular integers, I'll do it to get the patch accepted for now and I guess raise a separate issue about it.

Here is an updated patch for the 2006-08-10 09:30 cvs version. This one uses bigint's but puts them and the check() constraint in lower case. I ran this through field creation on my own site to see if I'd broken anything.

Hi,

My thoughts on the 2/3 billion are that if you reach that level (most often you won't) then the tables can be manually converted over to bigint (or done so at installation time if the need is forseen) for the extra four bytes. By all means post an issue, but I seriously doubt many people will support it. The similarities between MySQL and PostgreSQL type names is far more important than range. If you do gain support, i'll happily concede.

Please adjust the patch back to integers and i'll get it reviewed.

FYI: Drupal 4.8 will have int_unsigned, smallint_unsigned and bigint_unsigned types. This is another reason why using bigints in place of ints is unwanted.

Here's a patch for the latest 8/14 patch using integers instead of bigints.

There was one unsigned early on and a bigint translation left in content_db_add_column(). Fixed. Patch tested and ready for commit.

Updated patch for August 21 code changes to 4.7+.

--
Sammy Spets
Synerger
http://www.synerger.com

Switching it over to be 4.7 version so it gets some more attention from the maintainers!

Committed to 4.7 patch. Can someone provide a HEAD version of the patch, please?

Thanks for patching it through. Have you tried applying the 4.7 patch to HEAD? It may work. If not, I'll provide one once i'm done with the other two bugs i'm fixing.

patching file content_admin.inc
Hunk #2 succeeded at 721 (offset 6 lines).
Hunk #3 succeeded at 742 (offset 6 lines).
Hunk #4 FAILED at 860.
Hunk #5 succeeded at 925 (offset 6 lines).
Hunk #6 FAILED at 1003.
Hunk #7 succeeded at 1025 (offset 6 lines).
2 out of 7 hunks FAILED -- saving rejects to file content_admin.inc.rej
patching file content.install

I can probably figure it out, but it will take some time.

When you're talking about "HEAD" are you talking about the 4.7.0 release of CCK on this page here?

As a patching newbie, I'm looking for someone to pick this back up and run with it over the finish line. Can someone do this (and let me learn from this last piece of the process at the same time)?

I'm thrilled to see my patches were included in the 2006-09-19 CVS version of CCK. Thank you for taking care of this! I'm setting this to closed, if I can.