[dev] [commits] Horde branch master updated. cbf642fcd04568a42a6f3bf86508fe6b3b6c2a53

[Chuck Hagenbuch]

Quoting Jan Schneider <jan at horde.org>:

> Zitat von Michael M Slusarz <slusarz at horde.org>:
>
>> Quoting Jan Schneider <jan at horde.org>:
>>
>>>> -----------------------------------------------------------------------
>>>>
>>>> commit 07c40864adc4f8f8d05a95ba75321d09daef7e4c
>>>> Author: Michael M Slusarz <slusarz at curecanti.org>
>>>> Date:   Thu Mar 24 17:07:13 2011 -0600
>>>>
>>>>  Revert "Bug #9711: Fix creation of auto-increment columns in Postgresql"
>>>>
>>>>  This reverts commit 395a6e1f5eaf894889892e2d1c58f5bb71a22188.
>>>>
>>>>  Problem occurred in VFS library.  Added more complete unit tests.
>>>>
>>>> framework/Db/lib/Horde/Db/Adapter/Postgresql/Schema.php |    4 +-
>>>> framework/Db/package.xml                                |   16 +++----
>>>> framework/Db/test/Horde/Db/Adapter/Pdo/PgsqlTest.php    |   33  
>>>> ++++++++++++++-
>>>> 3 files changed, 40 insertions(+), 13 deletions(-)
>>>>
>>>> http://git.horde.org/horde-git/-/commit/07c40864adc4f8f8d05a95ba75321d09daef7e4c
>>>
>>> I have a question about the testAlterTableWithSeparatePk test.  
>>> Shouldn't the 'foo' column be of type integer when creating the  
>>> intial table?
>>>
>>> Or is the column not autoincrementing when created as a primaryKey  
>>> column (as opposed to being *updated* to a primaryKey column)?  
>>> That would be a bug then.
>>
>> This was the previous behavior I was seeing - but it had to do with  
>> the VFS bug (creating a folder in VFS was using an undefined  
>> variable for the vfs_id field which was breaking the not-null  
>> constraint).  I never converted back - we should be creating as an  
>> integer.  Although in alter table, no matter the current value of  
>> the column were still dropping/altering the column so the test is  
>> properly covering the method.
>>
>>> And I actually agree that we should use a different name for the  
>>> virtual type primaryKey. It's a bit confusing, and even though we  
>>> only use autoincrementing primary keys in Horde, other who are  
>>> going to use Horde_Db might not.
>>
>> I would vote for autoincrementKey then.  Or better still some  
>> constant value (Horde_Db::AUTOINCREMENT) with non-allowed SQL  
>> column characters so that the key will never collide with a  
>> possible SQL column name.
>
> This only must not collide with SQL types, not names. And I doubt  
> any DB vendor will introduce some autoincrementKey one day. :) The  
> name sounds fine.

I'm +1 for renaming primaryKey to autoincrementKey.

-chuck