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

Empty dbgenerated() still breaking for Unsupported() types #15654

Closed
jkcorrea opened this issue Oct 4, 2022 · 9 comments · Fixed by prisma/prisma-engines#4841
Closed

Empty dbgenerated() still breaking for Unsupported() types #15654

jkcorrea opened this issue Oct 4, 2022 · 9 comments · Fixed by prisma/prisma-engines#4841
Assignees
Labels
bug/2-confirmed Bug has been reproduced and confirmed. kind/regression A reported bug in functionality that used to work before. team/schema Issue for team Schema. tech/engines/migration engine Issue in the Migration Engine topic: dbgenerated topic: generated columns topic: Unsupported Scalar pseudo type `Unsupported`
Milestone

Comments

@jkcorrea
Copy link

jkcorrea commented Oct 4, 2022

Bug description

Using a default(dbgenerated()) on a column with type Unsupported("...") where a Postgres generated column is manually added leads to Prisma migrate wanting to DROP DEFAULT on that column, which is an error in pg.

Empty dbgenerated() were broken for all column types in v4 and fixed recently here: #14799. Seems like the fix didn't apply to Unsupported() types.

How to reproduce

Clone & follow instructions here:
https://github.com/jkcorrea/prisma-unsupported-dbgenerated-bug

Expected behavior

To not touch the defaults for any columns with default(dbgenerated()

Prisma information

generator client {
  provider = "prisma-client-js"
}

datasource db {
  provider = "postgresql"
  url      = env("DATABASE_URL")
}

model table {
  id            String                   @id
  hereBeDragons Unsupported("tsvector")? @default(dbgenerated())
}

Environment & setup

  • OS: macOS
  • Database: Postgres
  • Node.js version: v16.15.1

Prisma Version

Happens on prisma v4+ (including v4.4 and v4.5)

@jkcorrea jkcorrea added the kind/bug A reported bug. label Oct 4, 2022
@tomhoule tomhoule added bug/1-unconfirmed Bug should have enough information for reproduction, but confirmation has not happened yet. kind/regression A reported bug in functionality that used to work before. process/candidate team/schema Issue for team Schema. topic: Unsupported Scalar pseudo type `Unsupported` topic: dbgenerated tech/engines/migration engine Issue in the Migration Engine labels Oct 4, 2022
@jkcorrea
Copy link
Author

jkcorrea commented Oct 4, 2022

In case this helps others, I was trying to have a "full text search" tsvector column be auto-generated upon every update using Postgres' GENERATED ALWAYS AS feature. Since that's not jiving with prisma, here's my current workaround using PG triggers:

Set a default empty tsvector & Gin index

model table {
  id      String                   @id
  stuff   String?
  fts     Unsupported("tsvector")? @default(dbgenerated("''::tsvector"))

  @@index([fts], type: Gin)
}

Manually add an update/insert trigger

CREATE EXTENSION IF NOT EXISTS pg_trgm;
CREATE EXTENSION IF NOT EXISTS btree_gin;

create or replace function update_table_fts() returns trigger as $$
begin
  NEW.fts := to_tsvector('english',
    NEW.id::text
    || ' ' || COALESCE(NEW."stuff", '')
  );

  RETURN NEW;
end; $$ language plpgsql security definer set search_path = public, pg_temp;

drop trigger if exists "update_fts" on "table";
create trigger "update_fts"
  before insert or update on "table"
  for each row
  execute function update_table_fts();

Pretty simple and honestly I'm fine with using this until the coming Prisma FTS improvements land.

@janpio janpio removed kind/bug A reported bug. process/candidate labels Oct 19, 2022
@zakariamofaddel
Copy link

zakariamofaddel commented Dec 13, 2022

I am facing this exact issue, is there any update on this?

@jkcorrea thank you for the workaround, although I'm not very well versed with SQL, but the for each row above does not seem ideal when there are millions of rows on the affected table, or am I wrong?

EDIT:
The for each row clause is not an issue as it will run only on "each row" of the INSERT transaction.

@pimeys
Copy link
Contributor

pimeys commented Dec 13, 2022

The easiest way to not get this behavior is to db pull the data model in the database. I get this:

datasource db {
  provider = "postgresql"
  url      = "postgres://postgres:prisma@localhost:5438/postgres"
}

model table {
  id            String                   @id
  hereBeDragons Unsupported("tsvector")? @default(dbgenerated("to_tsvector('english'::regconfig, id)"))
}

Now, the next migrate will not try to drop the default constraint.

Although an empty @default() should at least give a validation error, which would be less confusing.

@pimeys pimeys added bug/2-confirmed Bug has been reproduced and confirmed. and removed bug/1-unconfirmed Bug should have enough information for reproduction, but confirmation has not happened yet. labels Dec 13, 2022
@zakariamofaddel
Copy link

zakariamofaddel commented Dec 13, 2022

The easiest way to not get this behavior is to db pull the data model in the database. I get this:

datasource db {
  provider = "postgresql"
  url      = "postgres://postgres:prisma@localhost:5438/postgres"
}

model table {
  id            String                   @id
  hereBeDragons Unsupported("tsvector")? @default(dbgenerated("to_tsvector('english'::regconfig, id)"))
}

Now, the next migrate will not try to drop the default constraint.

Although an empty @default() should at least give a validation error, which would be less confusing.

@pimeys This is what I have been doing, the problem is that when running npx prisma migrate dev --name initial-migration --create-only and look into the generated migration.sql file I see "hereBeDragons" tsvector DEFAULT to_tsvector('english'::regconfig, id), and trying to apply this will result in an error that looks something like this:

Error:
db error: ERROR: cannot use column reference in DEFAULT expression    
   0: sql_migration_connector::validate_migrations
             at migration-engine\connectors\sql-migration-connector\src\lib.rs:289
   1: migration_core::state::DevDiagnostic
             at migration-engine\core\src\state.rs:251

@janpio
Copy link
Member

janpio commented Dec 13, 2022

The workaround from @pimeys will not work here unfortunately, as the underlying SQL does not actually represent a "default" that we could create via dbgenerated(...):

"hereBeDragons" tsvector GENERATED ALWAYS AS (to_tsvector('english', id::text)) STORED,

or

prompt_ts tsvector GENERATED ALWAYS AS (to_tsvector('english'::regconfig, prompt)) STORED,

Using dbgenerated leads to this SQL instead:

"prompt_ts" tsvector DEFAULT to_tsvector('english'::regconfig, prompt),

(via Slack discussion with @zakariamofaddel: https://prisma.slack.com/archives/CA491RJH0/p1670931902950529)

@janpio
Copy link
Member

janpio commented Dec 13, 2022

I think you have 2 options how to deal with this for now. Both are not super great. Both are also untested, so let me know how it goes:

  1. Option 1 is to remove the @default from the schema. When you create a migration for this table with prisma migrate dev --create-only this will not include the to_tsvector but you can edit that to include the correct SQL. Any future db pull might unfortunately then add the @default again so you need to get rid of that. Future migrations should be clena though, so you can iterate on all the other things in your database.

  2. Option 2 is the opposite of that, you keep around whatever db pull put in the schema file, and then you only edit the migration you got via migrate dev --create-only. This will lead to any future migrate dev run also to create a new migration that tries to write the column with DEFAULT - which would fail. So you need to ignore that.

Please let me know if one of these options work - if not we need to dig a bit deeper.

I think this is the feature request to track support for generated columns properly: #6336

@zakariamofaddel
Copy link

zakariamofaddel commented Dec 13, 2022

Thank you @janpio for the answer, I found this to be the cleanest solution at the moment:

  • From the prod database I will remove the GENERATED ALWAYS AS (to_tsvector('english'::regconfig, <column-name>)) STORED from the columnName and leave it as a simple columnName tsvector.
  • Always in prod I will then set a trigger, as @jkcorrea suggested, that will take care of setting the columnName to the correct type before every INSERT.
  • Now running prisma db pull should generate columnName Unsupported("tsvector")? in my schema.prisma when running prisma db pull and always match the production database.

This should allow me to follow the prisma guide and cleanly introduce prisma migrate to the existing database.

I will update this comment if I'm successful 😄

EDIT:
This was indeed the best solution I could find and it worked flawlessly. We were able to introduce prisma migrate to an existing database with lots of production data.

Thank you @jkcorrea!!

@janpio
Copy link
Member

janpio commented Dec 13, 2022

Indeed, removing the generated column and using a trigger instead might be the actually best solution right now. Be aware that the trigger is not reflected in Prisma schema, so you need to put it into a migration file manually once or it will be missing on new deployments.

@pimeys pimeys assigned pimeys and unassigned pimeys Dec 15, 2022
@rohanrajpal
Copy link

Thanks a lot for sharing this! This method works perfectly, shall stick to it until we have support for GENERATED.

@apolanc apolanc added bug/0-unknown Bug is new, does not have information for reproduction or reproduction could not be confirmed. bug/1-unconfirmed Bug should have enough information for reproduction, but confirmation has not happened yet. and removed bug/0-unknown Bug is new, does not have information for reproduction or reproduction could not be confirmed. labels Jan 31, 2024
@apolanc apolanc removed the bug/1-unconfirmed Bug should have enough information for reproduction, but confirmation has not happened yet. label Jan 31, 2024
@janpio janpio added this to the 5.14.0 milestone Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/2-confirmed Bug has been reproduced and confirmed. kind/regression A reported bug in functionality that used to work before. team/schema Issue for team Schema. tech/engines/migration engine Issue in the Migration Engine topic: dbgenerated topic: generated columns topic: Unsupported Scalar pseudo type `Unsupported`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants