-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added addPrimaryKey and dropPrimaryKey commands to CDbMigration class in... #851
Added addPrimaryKey and dropPrimaryKey commands to CDbMigration class in... #851
Conversation
* @param string $columns the column/s where the primary key will be effected. The name will be properly quoted by the method. | ||
* @return integer number of rows affected by the execution. | ||
* @since 1.1.6 | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@since
has to be 1.1.11 ;-)
same for dropPk...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1.1.13 already
Thanks. Please also check if the schema class for individual DBMS needs to override the implementation (it is possible different DBMS has different syntax of adding/dropping PKs). |
for sqlite it is not possible to add/drop primary key at all: http://stackoverflow.com/a/946119/1106908 |
Thanks, will go over this weekend. |
@ridget why did you close this pull request? |
@cebe because I was half asleep and at 100% stupid level didn't even notice. Sorry mate. |
|
||
/** | ||
* Builds a SQL statement for removing a primary key constraint to an existing table. | ||
* The method will properly quote the table and column names. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be tab indented.
@ridget are you still willing to work on this? If not, I'll take over. |
@cebe not currently as travelling. Unless it can wait a month and a bit happy for you to take over. Apologies |
no problem, will wait for you then :) |
@cebe back from the holiday. Will get back onto this. Do you have a preference for handling the sqlite inability to add/remove primary keys ? |
I think for sqlite we should throw an exception stating that it is not supported by sqlite. |
@cebe Just running some tests on this currently with Postgres and Oracle left to go. Am I better off just running the following for the sqlite check?
|
overide the method in sqlite schema and |
… in response to enh req yiisoft#848
…rt-to-migrations Conflicts: CHANGELOG framework/db/schema/CDbSchema.php
@cebe thanks for all the help on this one. I've tested this against oracle, mysql, mssql and postgresl as well as sqlite drivers. |
} | ||
public function getColumnType($type) | ||
{ | ||
if(isset($this->columnTypes[$type])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why diff shows chages here? To me it looks identical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was space indentation before.
Ah, ok. |
Looks OK to me overall. |
Added addPrimaryKey and dropPrimaryKey commands to CDbMigration class in...
Thanks for working on it. |
…t-relation-select * 'master' of https://github.com/yiisoft/yii: (144 commits) Fixes yiisoft#1567 Added filterSelector to CGridView Renamed the CONTRIBUTING file to CONTRIBUTING.md so that GitHub effectively uses the Markdown syntax already present in the file when presenting it. Fixes yiisoft#1694 - doc fix Added issue number fixed whitespace issues introduced with yiisoft#851 Updated CHANGELOG Fixes yiisoft#1584 same fix for CListView as already done for CGridView Fixes yiisoft#1344 same fix for CListView as already done in CGridView Fixes yiisoft#1104 same fix for CListView as already done for CGridView Fixes yiisoft#1676 - proper grouping when no group field is specified Added addPrimaryKey and dropPrimaryKey commands to CDbMigration class in response to enh req yiisoft#848 [messages/ja] yii.php updated csfix - thx @resurtm typo in changelog Reverted PR yiisoft#1467 (Issue yiisoft#1465, fixes related issue yiisoft#1661) Reverted PR yiisoft#1662 Better naming small update to documentation of filterAjaxOnly() Coding style fix Fixing bug yiisoft#1661 ... Conflicts: CHANGELOG
... response to enh req #848