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

binary for postgres with limit option throws exception #1736

Closed
MasterOdin opened this issue Apr 11, 2020 · 3 comments · Fixed by #1768
Closed

binary for postgres with limit option throws exception #1736

MasterOdin opened this issue Apr 11, 2020 · 3 comments · Fixed by #1768
Labels

Comments

@MasterOdin
Copy link
Member

MasterOdin commented Apr 11, 2020

Using the following migration:

        $this->table('users')
            ->addColumn('col', 'binary', ['limit' => 32])
            ->create();

generates the following SQL:

CREATE TABLE "public"."users" ("id" SERIAL NOT NULL, "col" BYTEA (32) NOT NULL, CONSTRAINT "users_pkey" PRIMARY KEY ("id"));

which throws the following exception / error:

PDOException: SQLSTATE[42601]: Syntax error: 7 ERROR:  type modifier is not allowed for type "bytea"
LINE 1: ...BLE "public"."users" ("id" SERIAL NOT NULL, "col" BYTEA (32)...
                                                             ^ in /Users/mpeveler/Work/Github/phinx/src/Phinx/Db/Adapter/PdoAdapter.php:184

It would feel like the appropriate thing would be to just drop the limit and generate the following SQL:

CREATE TABLE "public"."users" ("id" SERIAL NOT NULL, "col" BYTEA NOT NULL, CONSTRAINT "users_pkey" PRIMARY KEY ("id"));

Should this be documented in the docs that limit cannot be used with binary for postgresql, or should the limit just be dropped implicitly for postgresql adapter?

@dereuromark
Copy link
Member

dereuromark commented Apr 11, 2020

I think you should be using #1734 :)
Please check and we can continue with that.

But as to your specific issue (and for all other cases not related to binaryuuid):
I think it should be dropped than it that case if not needed.

@dereuromark dereuromark added this to the 0.12 (PHP 7.2+) milestone Apr 11, 2020
@MasterOdin
Copy link
Member Author

Changed the limit to be something not 16 to make the issue not tie into the binaryuuid stuff at all.

@dereuromark
Copy link
Member

dereuromark commented Apr 11, 2020

Jep, I adjusted my comment. Sry for that ;)
Just funny that today I had the same issue related to this length of 16 to work with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants