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

feat: inline primary key creation for postgres flavours #5233

Merged

Conversation

hairmare
Copy link
Contributor

@hairmare hairmare commented Jun 20, 2022

This is a continuation of my work in #5212. It's aimed at really fixing #5211

#5212 implemented uuid(<id>, { primaryKey: true }) for allowing to use PRIMARY KEY on the field type in the column declaration. This aims at adding support for a distinct PRIMARY KEY constraint during table creating.

Both of these solutions want to make life easier for folx using CockroachDB which autogenerates a rowid primary if none is specified during initial table creating.

I'm creating this as a draft PR, I'll finalize it once the accompanying docs pr is also done and CI is green.

Please let me know if this is a feasible way to go forward and if there is some special consideration needed for any non-postgresql postgres flavours (i don't know them well).

tasks

  • take a closer look at why some non-postgres flavours are failing
  • add guards for new tests and their non-postgres cases
  • figure out why call on table with columns and name is failing with a timeout after the tests expectations are met and it succeeded

@OlivierCavadenti OlivierCavadenti self-requested a review June 20, 2022 18:59
@coveralls
Copy link

coveralls commented Jun 20, 2022

Coverage Status

Coverage increased (+0.02%) to 92.263% when pulling 85a8c3d on hairmare:fix/postgresql-primary-in-table-not-in-field into fbb774b on knex:master.

@kibertoad
Copy link
Collaborator

@intech Could you please take a look?

@intech
Copy link
Contributor

intech commented Jun 20, 2022

@kibertoad ok, tomorrow

@hairmare

This comment was marked as off-topic.

@hairmare hairmare marked this pull request as ready for review June 24, 2022 11:03
@kibertoad
Copy link
Collaborator

@intech ping :)

@intech
Copy link
Contributor

intech commented Jun 30, 2022

I'm here. Sorry for the long wait

@intech
Copy link
Contributor

intech commented Jun 30, 2022

@kibertoad LGTM

@kibertoad kibertoad merged commit 8b0dd49 into knex:master Jun 30, 2022
@hairmare hairmare deleted the fix/postgresql-primary-in-table-not-in-field branch June 30, 2022 13:37
@christopher-kapic
Copy link

Any idea when there will be a new release which includes this fix?
(Sorry if this is the wrong place to ask)

@kibertoad
Copy link
Collaborator

I'll do my best to do it this week

@christopher-kapic
Copy link

Any word on this?

@kibertoad
Copy link
Collaborator

@OlivierCavadenti Can you try publishing? I'm out of town till sunday, no laptop :(

@kibertoad
Copy link
Collaborator

Released in 2.2.0

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.

None yet

5 participants