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

Add option to skip unnecessary SELECT after create() #4246

Open
ziimakc opened this issue Nov 12, 2020 · 41 comments
Open

Add option to skip unnecessary SELECT after create() #4246

ziimakc opened this issue Nov 12, 2020 · 41 comments

Comments

@ziimakc
Copy link

ziimakc commented Nov 12, 2020

Problem

Every time you do operation with prisma-client it will make unnecessary select and return value

Query:

db.log
   .create({
      data: {
         smth: 1
      },
      select: { uuid: true }, // unnecessary select, even if you delete this line
   })

Logs:

prisma:query COMMIT
prisma:query BEGIN
prisma:query INSERT INTO "public"."log" ("smth") VALUES ($1) RETURNING "public"."log"."uuid"
prisma:query SELECT "public"."log"."uuid" FROM "public"."log" WHERE "public"."log"."uuid" = $1 LIMIT $2 OFFSET $3
prisma:query COMMIT

Suggested solution

Add option to skip select after insert or update (in case of operation fail error is thrown by default)

db.log
   .create({
      data: {
         smth: 1
      },
      select: false
   })

Logs:

prisma:query COMMIT
prisma:query BEGIN
prisma:query INSERT INTO "public"."log" ("smth") VALUES ($1)"
prisma:query COMMIT

Alternative solution (breaking change)

Do not return anything after insert or update if select was not explicitly set.

@michael-land
Copy link

michael-land commented Nov 18, 2020

I experience the same for upsert. Business logic removed

Code



      await this.prisma.product.upsert({
        select: { id: true },
        where: { id: product.id },
        update: { id: product.id },
        create: { id: product.id },
      });

Logs


prisma:query BEGIN
prisma:query SELECT "public"."product"."id" FROM "public"."product" WHERE "public"."product"."id" = $1 OFFSET $2
prisma:query SELECT "public"."product"."id" FROM "public"."product" WHERE "public"."product"."id" = $1
prisma:query UPDATE "public"."product" SET "gtin" = $1, "unit_price" = $2, ...and more WHERE "public"."product"."id" IN ($21)
prisma:query SELECT "public"."product"."id" FROM "public"."product" WHERE "public"."product"."id" = $1 LIMIT $2 OFFSET $3
prisma:query COMMIT

@coryvirok
Copy link

I am also seeing 2 SELECTs, 1 UPDATE, followed by another SELECT the same way that @xiaoyu-tamu reported above.
Are these 4 queries the intended behavior for upsert()?

This should be doable with at most 2 queries, (assuming you want to return the rows values after the UPDATE):

const id = 1
const col1 = 'foo'
const col2 = 'bar'

// First query (note: using MySQL syntax)
await client.product.$queryRaw`
INSERT INTO product (id, col1, col2) VALUES (${id}, ${col1}, ${col2})
ON DUPLICATE KEY UPDATE 
  col1=VALUES(col1)
  col2=VALUES(col2)
`

// Second query
const product1 = await client.product.findUnique({where: {id: 1}})

And if you don't want to return the whole row, just omit the second query.

@PranavSathy
Copy link

Is there any movement on this? Not sure if this is as relevant, but those select's (after insert even though I haven't specified a select argument) do eat into the transaction buffer limit.

@janpio janpio added the team/client Issue for team Client. label May 31, 2021
@rinogo
Copy link

rinogo commented Feb 8, 2022

Very much in favor of this. I was going to create the issue if it didn't exist.

For what it's worth, I like the solution proposed by @ziimakc, but I also don't mind if the default behavior of create() is to return the newly-created row. We should be able to disable the follow-up SELECT, though.

Until this feature is available, I'll have to resort to (ab)using $executeRaw.

@leoantony72
Copy link

Is there any solution to this , except specifying one field 😐

@rinogo
Copy link

rinogo commented Apr 21, 2022

@leoantony72 I just use $executeRaw instead to avoid the extra queries.

@yousufiqbal
Copy link

Make it optional plz. Not required in my use case at all.

@ziimakc ziimakc changed the title Add option to skip unnecessary select after create and return operation status instead Add option to skip unnecessary select after create Jul 20, 2022
@rohanrajpal
Copy link

Would appreciate an option to remove the unnecessary select

@drevisios
Copy link

@matthewmueller
I would like to add that it is not only an issue of performance and optimisations as in some cases it breaks functionality.

In my case where we use Postgres and Row Level Security (RLS), these "unnecessary" selects cause many problems in the flows and constantly require workarounds to bypass RLS.

As an example:

-- db admin
create table rls_test( id BIGSERIAL PRIMARY KEY);
create policy rls_test_sel_policy on rls_test for select to app using (false);
create policy rls_test_ins_policy on rls_test for insert to app with check (true);

alter table rls_test ENABLE ROW LEVEL SECURITY;
alter table rls_test FORCE ROW LEVEL SECURITY;

-- as user app
insert into rls_test default values; -- works
insert into rls_test default values returning id; -- fails

This is a dummy example (can be solved just with DB Roles) just to demonstrate the issue, as the "returning id" clause violates the select policy.

Also in the case of FKs, the { connect:{ id: 1 } } select also violates security policies that may have been applied. In the connect method I would strongly suggest allowing an "unsafe" or "unchecked" connection.

I can guess removing these selects may break things in the generic way that Prisma works but in our case this is a functionality problem and not a performance one.

@joshkaplan
Copy link

Additional note: this also applies to DELETE (it runs the select before the delete).

Big +1 for this request -- it should be a no-brainer to let users avoid extra queries without having to use an escape-hatch like $executeRaw. Every query adds load and latency, and best practice is to minimize both unless it's needed / it's something the user explicitly wants.

@thilllon
Copy link

thilllon commented Nov 12, 2022

+1
Because of this, it shows a significant performance degradation compared to TypeORM. By using $executeRaw, I lost the benefits of using Prisma

@josethz00
Copy link

josethz00 commented Nov 25, 2022

+1, this is slowing down my queries, how can I just insert new data without having to select it? Would be nice to see this in the Prisma Roadmap

@MoSattler
Copy link

+1 for the problems it causes with RLS

@janpio
Copy link
Member

janpio commented Dec 2, 2022

Can you elaborate exactly the problem this causes for RLS? I am not sure I follow.

@MoSattler
Copy link

MoSattler commented Dec 2, 2022

@janpio
Same as elaborated here.

If I have a policy that only allows inserting like this:

ALTER TABLE "User" ENABLE ROW LEVEL SECURITY;
ALTER TABLE "User" FORCE ROW LEVEL SECURITY;

CREATE POLICY only_create_no_read ON "User"
  FOR INSERT
  WITH CHECK (true);

a prisma.user.create() call seems to always fail. I guess because create calls RETURNING with the INSERT.
But it looks like RETURNING is covered by SELECT policies, and is not allowed in the policy above.

@MoSattler
Copy link

MoSattler commented Dec 2, 2022

To give you a more "real world" scenario:
I have users, which must be part of an organization.

model Organization {
  id   String @id @default(cuid())
  name String @unique

  users     User[]
}

model User {
  id String @id @default(cuid())

  name String
  organization   Organization @relation(fields: [organizationId], references: [id])
  organizationId String
}

If a user signs up, I would have a logic like this:

  const user = await prisma.user.create({
    data: {
      name: "MyUser",
      organization: {
        create: {
          name: "teamlearns",
        },
      },
    },
  });

so far so good. Now I am adding RLS. I want that

  • everyone can create an organization
  • user can only see their own organization

So I might add something like this

CREATE POLICY organization_create_policy ON "Organization"
    FOR INSERT
    WITH CHECK (true);
    
CREATE POLICY organization_only_member_policy ON "Organization"
  USING (
  "Organization"."id" = (
    SELECT "User"."organizationId"
    FROM "User" 
    WHERE "User"."id" = current_setting('user.userId', true)::text
  )
);

which will break the create() call, due to the INSERT ... RETURNING "public"."Organization"."id".

Though, now that I think about it, it probably won't work either way, since

      organization: {
        create: {
          name: "teamlearns",
        },
      },

is probably relying on the INSERT ... RETURNING "public"."Organization"."id" for the User INSERT.

@janpio does this make things a bit clearer?

@janpio
Copy link
Member

janpio commented Dec 2, 2022

Yes it does.

@bhavesh-chaudhari
Copy link

Is there any update on this issue? I don't want to select anything after update.

@ajhollowayvrm
Copy link

+1 on this. Currently I've changed my update to only return one field but that's just a workaround. Often the select is the more costly than the create/update itself.

@jonatandorozco
Copy link

Any update on this?

@josethz00
Copy link

+1

Relational databases have consistency, its information can almost always be trusted, there's no need to query the database again only to return the same things that were just created

@azzadrood
Copy link

+1

@etcimon
Copy link

etcimon commented Mar 16, 2023

https://dev.to/josethz00/benchmark-prisma-vs-typeorm-3873

Prisma is over 1,000x slower than typeorm on create workloads because of this

https://res.cloudinary.com/practicaldev/image/fetch/s--zvI0qHD9--/c_limit%2Cf_auto%2Cfl_progressive%2Cq_auto%2Cw_880/https://dev-to-uploads.s3.amazonaws.com/uploads/articles/qfvpaw4xz434xtl2t34b.png

I would not recommend doing an insert through ORM, especially for logs

@PhelixTaken
Copy link

I wish I could skip the SELECT ... function when checking if a record exists, without having to retrieve all of its data. Currently, the function selects everything by default, even when I only need to verify its presence. Unfortunately, I can't pass an empty object to the select parameter; otherwise it throws an error that I must specify what I want to select.

@Jun4928
Copy link

Jun4928 commented Mar 17, 2023

+1 Not yet on the roadmap?

@stemaDev
Copy link

as a workaround we could use createmany, right?

@jamiter
Copy link

jamiter commented Mar 17, 2023

For a current scenario I've also switched to using updateMany instead of update to prevent the extra select. A wast of resourcing if you're not interested in the result IMO.

A global setting to disable this behaviour would also be nice, instead of a per query solution. IMO it's bad practice to always do an extra select.

@reorx
Copy link

reorx commented May 12, 2023

@jamiter I've tried updateMany, the select still went on, no even a single one was reduced.

The most ridiculous thing happens when the model has a self-relation, it will select 3 times before the UPDATE statement is executed, causing random Expected parent IDs to be set when ordering by parent ID error with no clue. I've read the code of the underlying Rust query engine and was sure it's a bug not related to the database but the query engine itself. But I won't bother submitting a new issue since there are so many unresolved ones (#9875, #15745, #17316, #17776). It's always the case that the developers ask the issue owner to provide a reproducible example and then respond that they cannot reproduce, leaving the issue untracked for months. Surely, the errors are uneasy to reproduce, but that doesn't mean the issues should be ignored, but how obscure and terrible the bug is.

I'm grateful that Prisma saved us a lot of time for rapid development, but I really wish its developers could have more attention on improving the SQL level logics. In my understanding, being an ORM means you have the best practices of all kinds of stuff related to SQL and database, that's the reason for the community to choose you in the long run.

@Songkeys
Copy link

@reorx Totally agree with you.

(Off-topic ranting below)

There are many unsolved issues with many votes and requests but no developer has been participating in. The only comment you may get is some product manager asking what the real world scenarios you are facing and no response anymore. Based on the current roadmap, they were spending most of time working on improving the performance for serverless and data proxy product but ignoring the basic SQL features and bugs. I understand that they have their own business plan and have to focus on it. And sometimes I really wanted to help but don't know writing rust. I appreciate the brilliant DX of Prisma but just keep getting blocked by issues like this for years. Don't know if there's an alternative ORM to try. :(

@coryvirok
Copy link

This is likely going to be an unsatisfying recommendation but all those who have felt the pain for long enough will agree - just use a query builder. Knex is great and while there's marginally more boilerplate, it's easy and extensible.

I use Knex + sql-ts + dmate and I've never looked back (to Prisma.)

@reorx
Copy link

reorx commented May 12, 2023

@Songkeys SQL query builder is the thing! As @coryvirok also mentioned. I had the experience where I migrate from gorm to sqlx when writting go applications, and I was more than happy about the result. Having control over what SQL is executed is vital in serious use cases such as trading or payment systems. I havn't try Knex or Kysely because I was not quite familar with TypeScript and the timeline was urgent, but I think it's fairly the same comparing with sqlx in Go. Both Go and TypeScript are strong-typed, so we already have the "O" on language level, what we actually need is building SQLs in a DSL-like way to make them more coherent and maintainable.

@yeliex
Copy link

yeliex commented May 18, 2023

@janpio could anyone in dev tema pay attention to this, it is a very basic sql query feature

@schulzf
Copy link

schulzf commented May 25, 2023

+1 to "fix" this...
a easy way to do it would be to have an updateOnly func that returns just the number of affected rows

@mhadi512
Copy link

2+ years later, and this problem is still not resolved, these extra queries might be needed in some cases.
But I don't see any need for them in my simple case, which is simple creating a new record in an isolated table and returning all columns.

I really started re-thinking the usage of these ORMs. specially those who support NoSQL DBs. because they implemented an extra layer of validation to maintain data validity and integrity.

@janpio janpio changed the title Add option to skip unnecessary select after create Add option to skip unnecessary SELECT after create Jun 19, 2023
@rohanrajpal
Copy link

rohanrajpal commented Aug 1, 2023

So prisma 5.1.0 finally does not use SELECT but uses RETURNING instead

https://github.com/prisma/prisma/releases/tag/5.1.0

I guess there should be an option to skip this unecessary returning as well

@luxaritas
Copy link
Contributor

Also - the use of RETURNING also does not help with mysql

@alonbilu
Copy link

alonbilu commented Jan 24, 2024

+1
This is causing a major performance issue. I need to write a large amounts of data (few hundred rows per second) and this slows down things drastically. In my use case, I have no need for either initial select nor the final 'return'. I just want to do as fast as possible create/update calls

@theodiablo
Copy link

Same than many above, we use MySQL so the RETURNING feature introduced in Prisma 5.1.0 isn't helpful for us. We'd like to only insert rows and we don't use the result, so we have this unnecessary select after a .create or .update which is definitely hurting our performance when we have a lot of traffic.

I hear that we can use raw SQL to do that, but the whole point we're using Prisma is to avoid writing plain SQL and don't have to update all our queries when doing DB changes.

I guess that we'll have to migrate to a query builder instead, unless there are any plans from the Prisma team to address this? Given that this issue has been opened since 2020, I'm not sure it is quite a high priority yet on the roadmap 🤔
CC @janpio

@raress96
Copy link

I still can't believe this bug was not fixed. And yes, it is a bug IMO. When you create something, you expect it to be created and that's it. If you want to return fields then you should use include and select.
This is a HUGE problem in Microservice type application that need to handle a lot of traffic to the database, every additional SELECT hurts performance. RETURNING helps, but it is still unnecessary in a lot of cases.

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

No branches or pull requests