Possible bug in Gdn_DatabaseStructure->PrimaryKey()

businessdad

I stumbled upon a possible bug when I tried to define the schema for a new table for my plugin. Specifically, it's not possible to specify a Primary Key field which is not an Integer, although the method signature indicates that it should be possible. This is due to the fact that method _Gdn_DatabaseStructure->PrimaryKey() _attempts to set the Primary Key as AutoIncrement regardless of the type passed as an argument (see code below):

public function PrimaryKey($Name, $Type = 'int') {
  $Column = $this->_CreateColumn($Name, $Type, FALSE, NULL, 'primary');  
  $Column->AutoIncrement = TRUE; // <-- Here's the issue.
  $this->_Columns[$Name] = $Column;  

  return $this;  
}

In my case, the Primary Key has to be a String, therefore the AutoIncrement doesn't apply. For this reason, Gdn_DatabaseStructure can't create the table and returns an "Incorrect column specifier" error message. To fix it, I'd see two options:
- Check if $Type is any of the available Integers and, in case, apply the AutoIncrement.
- Add the AutoIncrement as an optional parameter and let the Developer choose what type to use and when to use the AutoIncrement (even if the PK is an Integer, it doesn't necessarily mean I'd like to have it AutoIncrement).

businessdad

Correction: the above issue can be circumvented by using method Column() instead of PrimaryKey(). To be more precise, Column('MyCustomID', 'int', FALSE, 'primary') would behave as expected, creating an Integer field set as Primary Key, but not as AutoIncrement. Of course, the field could also be created as a varchar, if needed.

I'd therefore say that the improvement, in this case would involve expanding the documentation to explain the behaviour of PrimaryKey() method.

The optimal thing to do, in my opinion, would be renaming the method as AutoPrimaryKey(), or any name that better reflects the method's behaviour, and rewrite PrimaryKey() so that it uses Column() without forcing the AutoIncrement attribute.