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

Update sequel #176

Closed
wants to merge 1 commit into from
Closed

Update sequel #176

wants to merge 1 commit into from

Conversation

phyrog
Copy link
Contributor

@phyrog phyrog commented Apr 4, 2018

From Postgres 10 onwards, IDENTITY columns seem to be favored (at least by Sequel) over serial columns. After updating Sequel, I now get an error for the bigserial columns, because it tries to make it an identity column with type bigserial. I believe this is a bug in Sequel, but I haven't been able to make a self-contained example for this.

The easiest solution would be to use bigint columns, but I'm not sure about the impact on the Hets/Haskell side here, especially when Postgres < 10 is used. @eugenk maybe you know if this is a critical change or if looking for another solution would be better.

This is the generated SQL query:

CREATE TABLE "actions" ("id" bigserial GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, "evaluation_state" text COLLATE "C" DEFAULT 'not_yet_enqueued' NOT NULL, "message" text NULL, "created_at" timestamp NOT NULL, "updated_at" timestamp NOT NULL)

and the error:

PG::SyntaxError: ERROR:  both default and identity specified for column "id" of table "actions": CREATE TABLE "actions" ("id" bigserial GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, "evaluation_state" text COLLATE "C" DEFAULT 'not_yet_enqueued' NOT NULL, "message" text NULL, "created_at" timestamp NOT NULL, "updated_at" timestamp NOT NULL)

What's strange is that this code works: it only worked because I accidently used a different Ruby version with a different Gem version.

require 'sequel'
require 'logger'

DB = Sequel.connect(adapter: :postgres, user: 'postgres', password: 'postgres',
  database: 'sequel_test', logger: Logger.new(STDOUT))

DB.create_table(:as) do
  primary_key :id, :type=>:bigserial
end

as it generates this query:

CREATE TABLE "as" ("id" bigserial PRIMARY KEY)

@phyrog
Copy link
Contributor Author

phyrog commented Apr 4, 2018

Ok, I already see that this won't work with Postgres < 10.

@phyrog
Copy link
Contributor Author

phyrog commented Apr 4, 2018

I opened jeremyevans/sequel#1490 for this

@phyrog
Copy link
Contributor Author

phyrog commented Apr 4, 2018

For now I created #177 to revert the sequel update. This PR now updates sequel to 5.7.1, which includes a fix for the bigserial situation.

@phyrog phyrog force-pushed the change_bigserial_to_bigint branch from 2b9cb04 to 37c8140 Compare April 5, 2018 09:33
@phyrog phyrog changed the title Change bigserial to bigint Update sequel Apr 5, 2018
@codecov-io
Copy link

codecov-io commented Apr 5, 2018

Codecov Report

Merging #176 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #176   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files         178    178           
  Lines        2648   2648           
=====================================
  Hits         2648   2648

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4bd2f96...37c8140. Read the comment docs.

@phyrog
Copy link
Contributor Author

phyrog commented Apr 5, 2018

Closed in favor of #178

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.

2 participants