Skip to content
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

Merged
merged 2 commits into from
Nov 9, 2012

Conversation

ridget
Copy link
Contributor

@ridget ridget commented Jun 20, 2012

... response to enh req #848

* @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
*/
Copy link
Member

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...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.1.13 already

@qiangxue
Copy link
Member

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).

@cebe
Copy link
Member

cebe commented Jun 20, 2012

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

@ridget
Copy link
Contributor Author

ridget commented Jun 21, 2012

Thanks, will go over this weekend.

@ridget ridget closed this Jun 21, 2012
@cebe
Copy link
Member

cebe commented Jun 22, 2012

@ridget why did you close this pull request?

@ridget ridget reopened this Jun 25, 2012
@ridget
Copy link
Contributor Author

ridget commented Jun 25, 2012

@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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be tab indented.

@cebe
Copy link
Member

cebe commented Sep 3, 2012

@ridget are you still willing to work on this? If not, I'll take over.

@ghost ghost assigned cebe Sep 3, 2012
@ridget
Copy link
Contributor Author

ridget commented Sep 3, 2012

@cebe not currently as travelling. Unless it can wait a month and a bit happy for you to take over. Apologies

@cebe
Copy link
Member

cebe commented Sep 3, 2012

no problem, will wait for you then :)

@ridget
Copy link
Contributor Author

ridget commented Oct 23, 2012

@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 ?

@cebe
Copy link
Member

cebe commented Oct 23, 2012

I think for sqlite we should throw an exception stating that it is not supported by sqlite.

@ridget
Copy link
Contributor Author

ridget commented Nov 2, 2012

@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?

if(preg_match('/sqlite\d?/', $this->getDbConnection()->getDriverName()))
    throw new CDbException('Not supported by SQLite');

@resurtm
Copy link
Contributor

resurtm commented Nov 2, 2012

@cebe
Copy link
Member

cebe commented Nov 7, 2012

overide the method in sqlite schema and throw new CDbException('Adding a primary key after table has been created is not supported by SQLite.')

Tom Ridge and others added 2 commits November 9, 2012 13:46
…rt-to-migrations

Conflicts:
	CHANGELOG
	framework/db/schema/CDbSchema.php
@ridget
Copy link
Contributor Author

ridget commented Nov 9, 2012

@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]))
Copy link
Member

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.

Copy link
Contributor

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.

@samdark
Copy link
Member

samdark commented Nov 9, 2012

Ah, ok.

@samdark
Copy link
Member

samdark commented Nov 9, 2012

Looks OK to me overall.

samdark added a commit that referenced this pull request Nov 9, 2012
Added addPrimaryKey and dropPrimaryKey commands to CDbMigration class in...
@samdark samdark merged commit 1eed41c into yiisoft:master Nov 9, 2012
@samdark
Copy link
Member

samdark commented Nov 9, 2012

Thanks for working on it.

cebe added a commit to cebe/yii that referenced this pull request Nov 14, 2012
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants