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. Duplicate table: 7 ERROR: relation" PRIMARY "already exists" #12629

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
4 participants
@jumper423
Copy link
Contributor

jumper423 commented Feb 16, 2017

CONSTRAINT must be unique

Hello!

  • Type: bug fix
  • Link to issue:

In raising this pull request, I confirm the following:

  • I have read and understood the Contributing Guidelines?
  • I have checked that another pull request for this purpose does not exist.
  • I wrote some tests for this PR.

Small description of change:

Postgresql in CONSTRAINT must specify a unique name for this CONSTRAINT, otherwise it leads to mistakes.

"ERROR: SQLSTATE [42P07]: Duplicate table: 7 ERROR: relation" PRIMARY "already exists"

Thanks

jumper423 added some commits Feb 16, 2017

Update postgresql.zep
CONSTRAINT must be unique
Merge pull request #1 from jumper423/patch-2
Update PostgresqlDialect.php

@jumper423 jumper423 changed the title Update postgresql.zep PostgreSQL. Duplicate table: 7 ERROR: relation" PRIMARY "already exists" Feb 16, 2017

@jumper423

This comment has been minimized.

Copy link
Contributor Author

jumper423 commented Feb 16, 2017

It is possible to completely remove the use CONSTRAINT, I do not see it necessary.

@sergeyklay

This comment has been minimized.

Copy link
Member

sergeyklay commented Feb 16, 2017

CONSTRAINT must be unique

So now do you provided this opportunity?

@jumper423

This comment has been minimized.

Copy link
Contributor Author

jumper423 commented Feb 16, 2017

Yes. Now CONSTRAINT name part of the table name. The need to add more and field names, I doubt.

@sergeyklay

This comment has been minimized.

Copy link
Member

sergeyklay commented Feb 16, 2017

In general looks weird to me.

For example, when I call dropPrimaryKey I want to drop real PR, not table_PK, isn't it? Or for example, I want to call addPrimaryKey twice and the problem with the duplicates still present. So I don't understand what problem you are trying to solve.

Could you please elaborate on that?

@jumper423

This comment has been minimized.

Copy link
Contributor Author

jumper423 commented Feb 16, 2017

Here is an example.
There are two migration to create tables. Each table must create a primary key.
But this can not be done. Because the error occurs.
"ERROR: SQLSTATE [42P07]: Duplicate table 7 ERROR: relation" core "already exists"
https://gist.github.com/jumper423/087d0abce5659aec22f1037ad08a6651

@sergeyklay

This comment has been minimized.

Copy link
Member

sergeyklay commented Feb 26, 2017

@jumper423
We don't add changes directly to stable branches or master. Could you please send this PR to the 3.1.x branch?

Also please update Phalcon\Db\Dialect\Postgresql::createTable and Phalcon\Db\Dialect\Postgresql::addIndex by implement new logic.

And update CHANGELOG.md.

@sergeyklay sergeyklay added this to the 3.2.0 milestone May 2, 2017

@nsossonko

This comment has been minimized.

Copy link
Contributor

nsossonko commented May 3, 2017

This change is backwards incompatible it would seem and can cause issues for users currently using postgres since it will assume a changed primary key name that has not been that way in the past...I guess maybe there should be some sort of migration system (optional) to the new naming?

@sergeyklay sergeyklay modified the milestones: 4.0.0, 3.2.0 May 3, 2017

@jumper423

This comment has been minimized.

Copy link
Contributor Author

jumper423 commented May 10, 2017

You can make imports. Ask for the name of the primary keys. Change them to a new standard. Do you think a good idea?

@jumper423

This comment has been minimized.

Copy link
Contributor Author

jumper423 commented May 10, 2017

And if so, then where to describe the migration itself? In fact, you can try to make one SQL request.

@sergeyklay

This comment has been minimized.

Copy link
Member

sergeyklay commented May 10, 2017

We'll try to sort out with this in 4.x

@niden

This comment has been minimized.

Copy link
Member

niden commented Dec 11, 2018

This has been addressed with #13643

Thank you @jumper423 and @CameronHall

@niden niden closed this Dec 11, 2018

@niden niden moved this from In progress to Done in 4.0 Release Dec 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment