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 foreignKey method for yii\db\ColumnSchemaBuilder #13018

Conversation

kfreiman
Copy link
Contributor

Q A
Is bugfix? no
New feature? yes
Breaks BC? no
Tests pass? yes
Fixed issues #13016, #8163

@arogachev
Copy link
Contributor

since tags are missing. What about unit tests?

@dynasource dynasource added the pr:request for unit tests Unit tests are needed. label Nov 16, 2016
@SilverFire SilverFire added this to the 2.0.12 milestone Nov 17, 2016
@SilverFire SilverFire added the status:under development Someone is working on a pull request. label Nov 17, 2016
@@ -265,6 +265,18 @@ public function createTable($table, $columns, $options = null)
if ($type instanceof ColumnSchemaBuilder && $type->comment !== null) {
$this->db->createCommand()->addCommentOnColumn($table, $column, $type->comment)->execute();
}

if ($type instanceof ColumnSchemaBuilder && $type->foreignKeyName !== null) {
$this->db->createCommand()->addForeignKey(
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't also create the index key?

/**
* @var string the name of the foreign key constraint.
*/
public $foreignKeyName;
Copy link
Contributor

@Faryshta Faryshta Apr 22, 2017

Choose a reason for hiding this comment

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

is it ok to make them public and not protected since only foreignKey() method can modify them?

Copy link
Member

Choose a reason for hiding this comment

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

Not really.

Copy link
Contributor

Choose a reason for hiding this comment

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

not ok or not bad that the properties are public?

Copy link
Member

Choose a reason for hiding this comment

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

Not OK.

* @param string $update the ON UPDATE option. Most DBMS support these options: RESTRICT, CASCADE, NO ACTION, SET DEFAULT, SET NULL
* @return $this
*/
public function foreignKey($foreignKeyName, $refTable, $refColumn, $delete=null, $update=null)
Copy link
Contributor

Choose a reason for hiding this comment

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

this name clashes with an existing functionality on the generator http://www.yiiframework.com/doc-2.0/guide-db-migrations.html#foreign-keys

@cebe cebe modified the milestones: 2.0.13, 2.0.12 Apr 25, 2017
@samdark samdark modified the milestones: 2.0.13, 2.0.14 Oct 7, 2017
@rob006
Copy link
Contributor

rob006 commented Nov 18, 2017

Related:
https://github.com/yiisoft/yii2/issues/9657
https://github.com/yiisoft/yii2/issues/14623

I think this should be rewritten to allow define FK for SQLite - FK definition should be injected into CREATE TABLE command.
See https://github.com/yiisoft/yii2/issues/14623#issuecomment-338854045 https://sqlite.org/foreignkeys.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature:db pr:request for unit tests Unit tests are needed. status:under development Someone is working on a pull request. type:enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants