If a module includes a $spec['length'] in its field schema then that length is simply added on to the end field type in _db_create_field_sql, which is a problem in postgres when that field type isn't varchar or character (there are other types where this syntax is allowed but drupal does not use them: see http://www.postgresql.org/docs/7.4/interactive/datatype.html).
A simple fix would be something like
if (!empty($spec['length'])) {
- $sql .= '('. $spec['length'] .')';
+ if(in_array($spec['pgsql_type'], array('varchar', 'character'))){
+ $sql .= '('. $spec['length'] .')';
+ }
}
but obviously it would be better to look at the length supplied and process it according to the type, eg an int of size 8 becomes bigint. However, the above fix at least allows modules like the computed field module to work with postgres, even if they length parameter will sometimes be ignored.
Thanks,
Steven H. Noble
Comment | File | Size | Author |
---|---|---|---|
#12 | 346450-database-D6.patch | 1.74 KB | neilnz |
#9 | 346450-database-D7-2.patch | 3.04 KB | Josh Waihi |
#5 | 346450-database-D7.patch | 2.14 KB | Dave Reid |
Comments
Comment #1
Gábor HojtsyWould be great to have another Postgres fluent reviewer. Setting patch needs review since we have suggested code modifications.
Comment #2
Damien Tournoud CreditAttribution: Damien Tournoud commentedOk, bumping to D7.
"length" is only meaningful for varchar, char and text, as documented in http://drupal.org/node/146939, but the documentation also says that this parameter is *ignored* for other datatypes.
This affect all drivers.
Comment #3
Damien Tournoud CreditAttribution: Damien Tournoud commentedComment #4
neilnz CreditAttribution: neilnz commentedsubscribe
Comment #5
Dave ReidInitial patch for review.
Comment #6
Dave ReidAdding tag
Comment #7
Josh Waihi CreditAttribution: Josh Waihi commented@Dave Reid, I had a patch!, you got in before me, I'm happy, so lets get it commuted and backported to 6
Comment #8
Dries CreditAttribution: Dries commentedLet's add some code comments, please. Ideally, we'd update the tests too.
Comment #9
Josh Waihi CreditAttribution: Josh Waihi commentedThis is already documented in includes/database/schema.inc, I merely updated it a bit.
Comment #10
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #11
Josh Waihi CreditAttribution: Josh Waihi commented@neilnz has a backport I believe.
Comment #12
neilnz CreditAttribution: neilnz commentedIndeed I do. I've redone it slightly to mimic the D7 patch. Attached.
Comment #13
Josh Waihi CreditAttribution: Josh Waihi commentedawesome
Comment #14
Darren OhPatch works.
Comment #15
Gábor HojtsyCommitted to Drupal 6 too, thanks!