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

Be able to update or retrieve a single record including non-unique fields in the "where" conditions. #7290

Closed
ksmithut opened this issue May 26, 2021 · 49 comments
Assignees
Labels
kind/feature A request for a new feature. team/client Issue for team Client. topic: client api
Milestone

Comments

@ksmithut
Copy link

ksmithut commented May 26, 2021

Problem

I am unable to utilize non-unique fields in single update queries. To be clear, I want to still use the unique fields, I just want to filter it down even more.

Suggested solution

Here are my models

model Person {
  id     String   @id @db.Uuid
  client Client[]
}

model Client {
  id       String @id @db.Uuid
  name     String
  personId String @db.Uuid
  person   Person @relation(fields: [personId], references: [id])
}

I have "people" who have "clients" attached to them. Only the person who owns the client can update the name. So when an API call comes in, I know who the current person is and what client they're trying to update. So the update query I would like to use is this:

const client = await prisma.client.update({
  data: { name: input.name },
  where: {
    id: input.clientId,
    personId: input.personId
  }
})

but it only allows fields which are marked as @unique or @id

Alternatives

One alternative is to do a .findUnique({ where: { id: input.clientId } }) and then check if the personId of the client is the same as the one passed in. This however creates two database calls where only one is needed.

Another alternative is to do a .updateMany({ where: { id: input.clientId, personId: input.personId } }) but I don't get any of the clients back. If I got the clients back in the query and if there was a limit I could pass in to limit it to one, I would feel better about this so it wouldn't have to do any unneeded scans of the rows, but it still feels less idiomatic than updating the .update() command to allow for non-unique fields.

@ksmithut
Copy link
Author

The same goes for .delete({ where: {} }). I can only add unique fields to the where conditions.

@janpio
Copy link
Member

janpio commented May 26, 2021

You should use updateMany and deleteMany in this case - as these queries can potentially affect multiple entries.

@ksmithut
Copy link
Author

But if I'm using a unique field along with the other non-unique fields, it won't potentially affect multiple entries, right? Like I mentioned, I would use those, except they don't return the updated records, just an updated count.

@ksmithut
Copy link
Author

To be clear, my intention is to only update the singular record with the primary key field, I just want to add an additional condition to the field to make sure someone who shouldn't have permission to update the "client" won't be able to.

@janpio
Copy link
Member

janpio commented May 26, 2021

Couldn't you then not just add a unique index across these two fields and use that as the identifier?

@ksmithut
Copy link
Author

I guess that would work, it just seems superfluous to add a unique index that includes a field that is already considered unique, especially if it's a workaround for a missing feature. Adding indexes to a database isn't "free" in that adding an indexes increases disk usage. It's probably not significant enough for my current use case currently, but I could see a future use case where adding unique indexes to every combination of fields I need to conditionally update/delete would be prohibitive.

@pantharshit00 pantharshit00 added kind/feature A request for a new feature. team/client Issue for team Client. topic: client api labels May 28, 2021
@stefan-watt-mednet
Copy link

I've stumbled across this issue a lot as well.
I see no reason why you shouldn't be able to optionally include additional conditions in the where clause.
So long as you provide a unique identifier that is.
There's some workarounds but none of them really make me too happy tbh.

@janpio
Copy link
Member

janpio commented Jun 14, 2021

What is the use case for adding another field, if you already have a unique identifier that you know uniquely identifies 1 specific row?

@ksmithut
Copy link
Author

@janpio I use the additional filter as a way of further qualifying the operation.

For example, an API request comes in to update a Todo item. I have the unique id of the todo from the parsed URL, and I have the current user from the auth token. I want to send a single update command to the database where the id is the given todo id and the owner of the todo is the current user’s id. This way, if someone tries to update a todo item they are not owner of, the update returns null.

@stefan-watt-mednet
Copy link

@janpio Because you might not want the query to return that row if it doesn't fit the additional criteria.

@janpio janpio changed the title Be able to update a single record including non-unique fields in the "where" conditions. Be able to update or retrieve a single record including non-unique fields in the "where" conditions. Jun 15, 2021
@s-ormanns
Copy link

I have the same problem. Two use cases that come to mind: maybe there is some kind of "Soft-Delete-Strategy" in place or an update is only "allowed" as long as a version field hasn't changed.

The problem with the many methods is, they do not return the affected "entities". So you would have to do additional queries.

@stefan-watt-mednet
Copy link

stefan-watt-mednet commented Jun 17, 2021

I have the same problem. Two use cases that come to mind: maybe there is some kind of "Soft-Delete-Strategy" in place or an update is only "allowed" as long as a version field hasn't changed.

Exactly! Especially since that is the recommended alternative to long running transactions.

The problem with the many methods is, they do not return the affected "entities". So you would have to do additional queries.

From a pragmatical standpoint that might be "the only" problem, but conceptually if you're trying to update a single record then you shouldn't use the updateMany method.

@vimutti77
Copy link

It would be useful to use this feature for Optimistic Concurrency Control #4988

const id = 'user_id'
const user = prisma.user.findUnique({ where: { id } })   // { id: 'user_id', version: 1, ... }

try {
  // Throws if not found
  prisma.user.update({
    where: {
      id: user.id,
      version: user.version,
    }
    data: { 
      version: {
        increment: 1
      },
      ...
    },
  )
} catch (e) {
  // Consider redoing, or error out.
}

@jgee67
Copy link

jgee67 commented Aug 26, 2021

@janpio another very compelling reason to support this change is to make the recommended soft-delete middleware far less hacky. Currently the recommendation is to overwrite update calls with updateMany in order to shove in the where: { deleted: false } clause, because update does not support this feature. This recommendation works, but has many drawbacks including but not limited to:

  1. As noted in the docs, the return type of update is affected
  2. update now secretly conforms to the updateMany api, which results in many unexpected errors. Deleting nested records, for example: Calls to update will accept the nested association deleteMany parameters, but since we are swapping it to updateMany under the hood, it will blow up with an unexpected error whenever update is called with these "valid"
    params.

Supporting non-unique where clauses on singular resource reads/updates/deletes (as is very typical and provided by default in many databases) should relieve the middleware of the responsibility of swapping out update for updateMany calls. This will provide a more simple, straightforward, and thought-out solution for soft-deletes.

@kvizcarra
Copy link

Another downside with this workaround: updateMany doesn't support nested upsert.

@leerobert
Copy link

Did we ever find a solution to this?

      const todo = await prisma.todo.update({
          where: {
            id: query.id,
            userId: session.userId
          },
          data
        })

Would be nice to leverage this to confirm user owns the TODO object.

@iqbalhasnan
Copy link

I've been looking for a way to upsert with multiple where conditions, I guess it is not supported yet ?

await db.user.upsert({
  where: { uuid: params_uuid, email: params_email },
  create: {
    email: email,
    uuid: uuid,
    products: {
      create: productsMap,
    },
  },
  update: {
    products: {
      create: productsMap,
    },
  },
})

@spvn
Copy link

spvn commented Dec 29, 2021

I too require this function. My use case is a user can request a relationship with another user (e.g. trying to "friend" another user). When the second user tries to accept the request, I want to make sure there's a pre-existing record in the pivot table with the status currently at "REQUESTED" instead of "BLOCKED" or some other possible status.

@oyatek

This comment was marked as spam.

@mrhammadasif
Copy link

mrhammadasif commented Mar 9, 2022

Definitely required. For the workaround we are first querying the record and see if that meet the conditions and then calling update function with the matched id. So, why not do that in single update query ?

Well, it could be a updateFirst as well

@hallvors
Copy link

I have a unique constraint on several columns
@@unique([userId, portfolioId, privilege])
I also tested
@@id([userId, portfolioId, privilege])

However, the Prisma client's types will not allow me to run a .delete() query using these three fields, so I have to resort to raw queries to actually delete specific rows from this table..

@MansurAliKoroglu

This comment was marked as spam.

@ro-savage
Copy link

Agree with everyone else here - is very odd that this behaviour isn't supported.

It also appears updateMany actually just does an update on ID, but first runs a select statement to get the ids.

    await prisma.assignment.updateMany({
      where: {
        project_id: project_id,
      },
      data: {
        minutes_per_day: minutes,
      },
    })

Generates

prisma:query BEGIN
prisma:query SELECT "public"."assignments"."id" FROM "public"."assignments" WHERE "public"."assignments"."project_id" = $1
prisma:query UPDATE "public"."assignments" SET "minutes_per_day" = $1 WHERE "public"."assignments"."id" IN ($2,$3,$4,$5,$6,$7)
prisma:query COMMIT

Where handwritten sql would be

UPDATE "public"."assignments" SET "minutes_per_day" = $1 WHERE "public"."assignments"."project_id" = ($2)

Is there a reason for this? Is it to normalise the queries across different database types? Or is it related to the fact Prisma will only update based on unique's?

@liesislukas
Copy link

it's limiting but also it's powerful. If updates are done by unique ids only, it opens doors to many automation around it. E.g. DynamoDB has limitation at database layer to be able to update/delete rows only if you give exact unique identifier to the row, no mass updates/removes and it's the reason why they are able to do something like seamless cache layer w/o any api modifications. You can also easily pipe updates/deletes to some queue/stream and have exact flow of events and ability to re-create state of data. etc. etc. update/delete should update delete only and only single item by it's unique identifier. in sql you have option to mass delete/update, that's why there are updateMany and deleteMany. If you don't like result of *Many functions, you can talk about having options for that call, but i would argue to keep update/delete limited to single item update/delete only no matter what.

@ksmithut
Copy link
Author

ksmithut commented Jun 25, 2022

@liesislukas You're right, it is super powerful, but the argument that I'm making isn't that the update call should be able to update many documents. I absolutely agree it should only be able to update single documents and that a unique/id field should be required to be passed in. The argument is that additional fields should be able to be passed in along-side a unique field.

Here's a real example that has severe security implications:

In the OAuth2.0 spec in the authorization flow where a user delegates permission to a 3rd party application, when the user grants access, the authorization server generates a one-time use auth code to give to the 3rd party application. The 3rd party application will exchange that auth code in addition to its credentials to get an access token. The prisma model for the auth codes might look something like this:

model AuthorizationCode {
  code                String            @id
  user                User              @relation(fields: [userId], references: [id])
  userId              String
  client              ClientApplication @relation(fields: [clientApplicationId], references: [id])
  clientApplicationId String
  createdAt           DateTime
  expiresAt           DateTime
  scope               String
  consumedAt          DateTime?
}

When the 3rd party application makes the call with the auth code, we want to in the same database call fetch the auth code and set the consumedAt field to the current date. That query ideally would look something like this:

const now = new Date()
const authCode = await prisma.authorizationCode.update({
  where: {
    authCode: req.body.code, // This is the unique field and should be required
    clientApplicationId: req.body.client_id, // not unique, but helps make sure clients don't consume other auth codes
    expiresAt: { gt: now }, // Make sure we don't touch an expired auth code
    consumedAt: null // Make sure we don't touch an auth code that has already been consumed
  },
  data: {
    consumedAt: now // Update consumedAt so it can't be consumed again
  }
})
if (!authCode) return res.status(400).json({ error: 'invalid_code' })

Because this kind of update query is not possible, it turns into something like this:

const now = new Date()
const authCode = await prisma.authorizationCode.findFirst({
  where: {
    code: req.body.code,
    clientApplicationId: client.id,
    expiresAt: { gt: now },
    consumedAt: null
  }
})
if (!authCode) return res.status(400).json({ error: 'invalid_code' })
await prisma.authorizationCode.update({
  where: { code: req.body.code },
  data: { consumedAt: now }
})

This change not only makes two database calls where one would be fine, but now there is a gap of time between when we fetch the auth code (and check whether or not it has been consumed or not) and when we update the consumedAt field. This would allow for a Reply attack. The security consideration for this is briefly described here: https://www.oauth.com/oauth2-servers/access-tokens/authorization-code-request/#security-considerations

@Philzen
Copy link

Philzen commented Jun 30, 2022

This is the workaround we currently use, based on

prisma.person.update({
  data: { id: input.personId },
  where: {
    clients: {
      update: { 
        where: { input.clientId }, data: { name: input.name } 
      }
    }
  },
  select: { 
    clients: { 
      where: { id: input.clientId } 
    }  
  }
}).then((data) => data.clients[0])

...and there you go. A little convoluted but does the job. The select/then part is actually really important, b/c otherwise you will get back a person object instead of a client.

Of course, there is no solution available for the delete-mutation (other than a transaction as mentioned in this discussion), the only thing we can do there is .then(() => { return { id:data.clientId } } ), which is better than nothing.

@oyatek
Copy link

oyatek commented Jul 8, 2022

@ksmithut Totally agree.

It should be allowed to provide an extra non-unique field along with an unique one in the .where clause. This simple change would make our life so much easier and the code would become much more stable.

Let me provide an example from my app.

I need to mark a transaction as "finished" and then apply the transaction amount to user's balance.

  const transactionUpdated = await db.transaction.update({
    where: {
      id,
    },
    data: {
      status: TransactionStatus.finished,
      user: {
        update: {
          balance: {
            increment: transaction.amountNet
          }
        }
      }
    }
  })

But when I do this, in order to avoid double balance change I must check first that transaction is not finished by another process. I do this as suggested on Prisma docs by checking lastUpdated field that is auto-incremented on every update. To make this kind of where possible, I'm changing .update to .updateMany:

  const transactionUpdated = await db.transaction.updateMany({
    where: {
      id,
      lastUpdated: transaction.lastUpdated
    },
    data: {
      status: TransactionStatus.finished,
      user: {
        update: {
          balance: {
            increment: transaction.amountNet
          }
        }
      }
    }
  })

But now the problem is that since I've changed .update to .updateMany - I can't do the nested update of user's balance!

I have all kinds of similar issues because of this limitation. I'm developing an app that works with money and I must pay close attention to race conditions.

Please change this.

@vsinghipcium

This comment was marked as spam.

1 similar comment
@AgusVelez5

This comment was marked as spam.

@sorenbs
Copy link
Member

sorenbs commented Jul 27, 2022

I think this thread has uncovered a very real use case that is currently not well supported by Prisma.

We want the where clause to be limited to unique fields for reasons pointed out already.

The where clause is used to identify the singular row to be updated. Additionally, it should be possible to submit a filter against any columns on the row to decide wether to actually perform the update. Multiple use cases for this are given already. Another use case I have heard is to only perform the update if the data has actually changed. This is useful to avoid an updatedAt column being updated when all other data stays the same.

In the Prisma API, it could look like this:

const client = await prisma.client.update({
  data: { name: input.name },
  where: {
    id: input.clientId,
  },
  if: {
    personId: input.personId
  }
})

Conceptually, the where clause selects the row, and the if clause determines if it should be updated.

Eventually, this could be expanded to support more complex use cases than just evaluating columns on the row to be updated. This idea is explored in some detail in a long conversation last year: #1844 (comment)

@olivierpichon
Copy link

olivierpichon commented Jul 29, 2022

We have encountered this problem with soft delete that we worked around using two queries. But we are wondering how do people handle authorization checks with Prisma in the end? If you have a multi-tenant application and the concept of organization and a Comment model for example. And you want to ensure regardless of the request you want to achieve that this one will be scoped to the organization you are part of. With Rails, you can add a .where(organization_id: id) to your query and chain that to further filtering. Having where with more than unique fields would allow this.

Should all our endpoints have this kind of code then?

    const comment =
      await dataSources.prisma.comment.findFirst({
        where: {
          id,
          organizationId,
        },
      });
    return await dataSources.prisma.comment.update({
      where: {
        id: comment?.id,
      },
      data,
    });

Any suggestions?

@Zac-Zajdel

This comment was marked as spam.

3 similar comments
@AgusVelez5

This comment was marked as spam.

@Arthur-Diesel

This comment was marked as spam.

@achinchen

This comment was marked as spam.

@oyatek
Copy link

oyatek commented Sep 6, 2022

Dear Prisma developers, let me know please if investing some money in your project will help to speed up adding this feature. We need it very much for the project that is already in production. Thank you.

@liesislukas
Copy link

liesislukas commented Sep 6, 2022

@ksmithut #7290 (comment)

dynamodb has same issue and resolve it with ConditionExpression in update call. Adding sample query. Maybe something to think about. Edited query here inline, hope no typos, but idea should be clear, check docs for more details.

related dynamo docs:
https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_UpdateItem.html

  await db
      .update({
        TableName: "some_table",
        Key: {
          authCode: req.body.code,
        },
        ConditionExpression:
          "attribute_not_exists(consumedAt) and #clientApplicationId = :clientApplicationId and #expiresAt > :now ",
        UpdateExpression:
          "set #consumedAt = :now",
        ExpressionAttributeNames: {
          "#clientApplicationId": "clientApplicationId",
          "#expiresAt": "expiresAt",
        },
        ExpressionAttributeValues: {
          ":now": Date.now(),
          ":clientApplicationId": req.body.client_id,
        },
      })
      .promise();

@oyatek
Copy link

oyatek commented Sep 12, 2022

I just realized what if we give prisma what it needs for the .update method - a unique field?

What if we use a uniqueuuid() value in the lastUpdated field, instead of a non-unique auto-incremented value, and update it every time the object updates? Then we will be able to use prisma .update and get what we need?

await db.user.update({
  where: {
    lastUpdated: uuidValueFromThePreviousQuery
  },
  data: {
    something: "foo",
    lastUpdated: generateNewUuid()
  }
})

@ksmithut
Copy link
Author

@oyatek That's a good way to handle conflicts, similar to a version field like you mentioned. But there are a lot of other cases, such as making sure only an owner can update a post. The owner field cannot be unique because the owner can own multiple posts.

@Zac-Zajdel
Copy link

@oyatek That's a good way to handle conflicts, similar to a version field like you mentioned. But there are a lot of other cases, such as making sure only an owner can update a post. The owner field cannot be unique because the owner can own multiple posts.

My exact problem here. It just seems like such a base level feature although I am sure it probably is not to implement without breaking other things in their codebase. While I respect the heck out of all the work the team is doing, I feel like the community is being ignored here. This was opened on May of last year and since I posted, I keep getting notifications showing so many others pointing out this flaw. And that doesn't even account for the several comment threads elsewhere.

@oyatek
Copy link

oyatek commented Sep 13, 2022

@oyatek That's a good way to handle conflicts, similar to a version field like you mentioned. But there are a lot of other cases, such as making sure only an owner can update a post. The owner field cannot be unique because the owner can own multiple posts.

Then you can make a unique index based on 2 fields:

@@unique([userId, lastUpdated])

UPDATE
sorry, just realized it wouldn't work in your case as you need to query DB without knowing lastUpdated

@LinusU
Copy link

LinusU commented Sep 13, 2022

Currently it isn't possible to do something like:

prisma.project.delete({
  where: {
    id: 'foo',
    userId: 'bar'
  }
} 

To make sure that the user can only see the project that itself owns.

The only workaround I can find involves adding an additional unique index, and query on that:

prisma.project.delete({
  where: {
    id_userId: {
      id: 'foo',
      userId: 'bar'
    }
  }
} 

Surely we shouldn't be required to add additional unique constraints on combinations with id? 🤔

And it also doesn't work at all for ensuring things only get deleted once, since @@unique exclude null values:

prisma.project.update({
  where: {
    id_userId_deletedAt: {
      id: 'foo',
      userId: 'bar',
      deletedAt: null
    }
  },
  data: {
    deletedAt: new Date()
  }
} 

(property) deletedAt: string | Date
Type 'null' is not assignable to type 'string | Date'.ts(2322)

@alitoshmatov

This comment was marked as off-topic.

@nickfreemandesign
Copy link

nickfreemandesign commented Sep 23, 2022

@janpio another use case here is is for data security and compliance - we need to ensure that all queries where relevant are bound by the parent organization.

   prisma.someDao.findUnique({
      where: {
          id: 'someUniqueId', // <--- yes this will find one
          parentOrg: 'parentOrgThatOwnsSomeUniqueId' //<--- but for compliance we need to ensure that the id belongs to the requesting organization
      }
   })

UPDATE: adding @@unique([id, parentOrg]) to the relevant Prisma entity will generate a way to query by parentOrg but this is problematic because technically parentOrg isn't unique.

The first error if you attempt to provide both arguments without inspecting the types carefully is:

Invalid this.prisma.someDao.findUnique() invocation: Argument where of type SomeDaoWhereUniqueInput needs exactly one argument, but you provided id and id_parentOrg. Please choose one. Available args:

type SomeDaoWhereUniqueInput {
  id?: String
  id_parentOrg?: SomeDaoIdParentOrgCompoundUniqueInput
}

And upon further inspection, SomeDaoIdParentOrgCompoundUniqueInput is an object allowing you to provide the compound unique. The successful query looks like:

   prisma.someDao.findUnique({
      where: {
          id_parentOrg:  {
              id: 'someUniqueId',
              parentOrg: 'parentOrgThatOwnsSomeUniqueId', 
          }      
     }
   })

So in conclusion (for our needs) this is good enough, but likely worth some docs to clarify further.

@viktornord
Copy link

this obvious easy thing should be allowed. How come prisma does not support this? Please bring this in...

@tatianajiselle
Copy link

+1 to supporting a updateOne() similar to findOne() based on id and attribute/where clause

@janpio janpio added this to the 4.5.0 milestone Oct 11, 2022
@Weakky
Copy link
Member

Weakky commented Oct 18, 2022

Hey folks,

A solution was implemented by prisma/prisma-engines#3281. It will be available in the next release under a new preview flag called extendedWhereUnique.

If you have any feedback you'd like to give about this feature, please post it here #15837 (comment)

Thanks for your patience, closing now 🙏.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature A request for a new feature. team/client Issue for team Client. topic: client api
Projects
None yet
Development

No branches or pull requests