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

PostgreSQL - unique error #17263

Closed
cluwong opened this issue Apr 12, 2019 · 7 comments
Closed

PostgreSQL - unique error #17263

cluwong opened this issue Apr 12, 2019 · 7 comments

Comments

@cluwong
Copy link

cluwong commented Apr 12, 2019

What steps will reproduce the problem?

In migration script:
$this->alterColumn({{%table}}, ‘id’, $this->string(50)->unique());

What is the expected result?

The specified column in table changed to specified type and condition.

What do you get instead?

alter column id in table {{%table}} to string(50) UNIQUE ...Exception 'yii\db\Exception' with message 'SQLSTATE[42601]: Syntax error: 7 ERROR: syntax error at or near "UNIQUE"
LINE 1: ...TABLE "table" ALTER COLUMN "id" TYPE varchar(50) UNIQUE
^
The SQL being executed was: ALTER TABLE "table" ALTER COLUMN "id" TYPE varchar(50) UNIQUE'

Additional info

The syntax in psql should be in format:
ALTER TABLE the_table ADD CONSTRAINT constraint_name UNIQUE (thecolumn);

Running with PostgreSQL version 9.5.16

Q A
Yii version 2.0.17
PHP version 7.0
Operating system Ubuntu 16.04
@bizley
Copy link
Member

bizley commented Apr 12, 2019

Related to #12135

@samdark samdark added PostgreSQL status:ready for adoption Feel free to implement this issue. type:bug Bug labels Apr 12, 2019
@np25071984
Copy link
Contributor

  1. Change ColumnSchemaBuilder::unique() method:
    /**
     * Adds a `UNIQUE` constraint to the column.
     * @param string $constraintName
     * @return $this
     */
    public function unique($constraintName = null)
    {
        $this->isUnique = true;
        if ($constraintName) {
            $this->constraintName = $constraintName;
        }
        return $this;
    }
  1. For psql driver, ColumnSchemaBuilder::buildUniqueString() returns empty string

  2. Method QueryBuilder::alterColumn() now returns two subqueries for the driver:

    public function alterColumn($table, $column, $type)
    {
        $sql = "";
        if ($type->getIsUnique()) {
            $sql = 'ALTER TABLE ' . $this->db->quoteTableName($table) .
                ' ADD CONSTRAINT ' . $type->getUniqueConstraintName() .
                ' UNIQUE (' . $this->db->quoteColumnName($column) . '); ';
        }

        // https://github.com/yiisoft/yii2/issues/4492
        // http://www.postgresql.org/docs/9.1/static/sql-altertable.html
        if (!preg_match('/^(DROP|SET|RESET)\s+/i', $type)) {
            $type = 'TYPE ' . $this->getColumnType($type);
        }

        return $sql . 'ALTER TABLE ' . $this->db->quoteTableName($table) . ' ALTER COLUMN '
            . $this->db->quoteColumnName($column) . ' ' . $type;
    }

And eventually I stumbled upon PDOStatement::execute() method, which can't manage multiple queries an once:

Total 1 new migration to be applied:
        m190413_080901_test

Apply the above migration? (yes|no) [no]:yes
*** applying m190413_080901_test
    > alter column name in table {{%city1}} to string(50) ...Exception: SQLSTATE[42601]: Syntax error: 7 ERROR:  cannot insert multiple commands into a prepared statement
The SQL being executed was: ALTER TABLE "city1" ADD CONSTRAINT aaaa UNIQUE ("name"); ALTER TABLE "city1" ALTER COLUMN "name" TYPE varchar(50) (/var/www/yii2.test/apps/basic/vendor/yiisoft/yii2/db/Schema.php:664)

@bizley
Copy link
Member

bizley commented Apr 13, 2019

The proper way of doing this right now is

$this->alterColumn({{%table}}, 'id', 'ADD CONSTRAINT constraint_name UNIQUE (thecolumn)');

@np25071984
Copy link
Contributor

Yes, but I would like to know, what is the "proper" way to solve the problem. On one side we can't split the whole task to subqueries. On other side we can't do the task by single query.
Theoretically, we can split the sql-query by ';' symbols and run subqueries one by one, but what should we return as result? Now it is row count, but in case multiple query we can't just sum all row counts from the queries.

@bizley
Copy link
Member

bizley commented Apr 13, 2019

I'm not sure what is the best way of fixing this. PostgreSQL behaves here differently from the other DBMS, more atomically. Let's take classic example:

  • We've got simple table with one column (INT) and default value 1.
  • Now we want to change type to CHAR.

For MySQL if we would do migration like:

$this->alterColumn('table', 'column', $this->char(1));

it means that column lost its default value. But with

$this->alterColumn('table', 'column', $this->char(1)->defaultValue(1));

we've got CHAR column with default value according to the first idea.

As for PostgreSQL - with

$this->alterColumn('table', 'column', $this->char(1));

we switched column to CHAR but we still got the default. For the following to work

$this->alterColumn('table', 'column', $this->char(1)->defaultValue(1));

we would have to change the output of alterColumn() to produce ALTER TABLE followed by two ALTER COLUMN separated with comma but it would try to add default value.

And with the previous statement for it to work like in MySQL we would have to add hidden DROP DEFAULT alter statement. As I checked it's fine even if there is no default value to start with. But what about other things like NOT NULL, UNIQUE, etc.

@bizley
Copy link
Member

bizley commented Apr 17, 2019

I've tried to work on this but I'm stuck on how to handle some cases. Let's take this example

$this->alterColumn('table', 'column', $this->integer()->unique());

We could do it with 2 statements:

ALTER TABLE table ALTER COLUMN column TYPE int;
ALTER TABLE table ADD CONSTRAINT column-unique UNIQUE (column);

or... throw exception?

Please use addUnique() method to add this constraint.

On the other hand if we choose first option to make it compatible with other DMBS should we handle

$this->alterColumn('table', 'column', $this->integer()); // no unique in definition

like this:

ALTER TABLE table ALTER COLUMN column TYPE int;
ALTER TABLE table DROP CONSTRAINT column-unique;

The second statement almost for sure triggers error when there is no constraint with that name. And currently we are not checking previous column definition to detect if this DROP is needed.

What do you think @samdark @cebe @SilverFire ?

@samdark
Copy link
Member

samdark commented Apr 17, 2019

Throwing exception isn't a good idea since migrations become non-interoperable i.e. product can not use them for being installable on MySQL, PostgreSQL etc. at the same time. Multiple statements are OK. We can add another statement for checking if constraint exists for that field before dropping it.

@samdark samdark added status:under development Someone is working on a pull request. and removed status:ready for adoption Feel free to implement this issue. labels Apr 24, 2019
@samdark samdark added this to the 2.0.19 milestone Apr 30, 2019
@samdark samdark closed this as completed Apr 30, 2019
samdark pushed a commit that referenced this issue Apr 30, 2019
…n()` to accept properly `ColumnSchemaBuilder` definition of column
@samdark samdark removed the status:under development Someone is working on a pull request. label Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants