tywalch / electrodb

A DynamoDB library to ease the use of modeling complex hierarchical relationships and implementing a Single Table Design while keeping your query code readable.
MIT License
1.03k stars 66 forks source link

Removing attributes with upsert #339

Open mikeyaa opened 11 months ago

mikeyaa commented 11 months ago

Is it possible to remove attributes from a record while using upsert? I see update has set() and remove() but how would I go about upserting a record that removes some properties in one go? It seems it will only set attributes but not unset them.

tywalch commented 11 months ago

Definitely, I can't quite remember why I omitted that operation but I will take a look!

mikeyaa commented 11 months ago

Thank you! Your library has been very helpful to far!

rangoc commented 7 months ago

@tywalch

We've encountered the same issue, just today, as the OP has posted about.

Upon reading the documentation, we first thought that the update() was what we were looking for, as it had remove():

The DynamoDB method update() will create an item if one does not exist. Because updates have reduced attribute validations when compared to put, the practical ramifications of this is that an update can create a record without all the attributes you’d expect from a newly created item.

And also, based on this, we thought that if there is no record in the db, new one will be created. That is true, BUT... it gets created without _created_at field, even though on the entity we have this:

 _created_at: {
            type: 'string',
            required: true,
            readOnly: true,
            set: () => new Date().toISOString(),
            default: () => new Date().toISOString(),
},

So that's where we hit a roadblock.

Then we came across upsert() which does the job, but it lacks the ability to remove fields, as the OP says. And we need to have that ability.

Definitely, I can't quite remember why I omitted that operation but I will take a look!

This brings some hope, are there any updates on this?

tywalch commented 7 months ago

This brings some hope, are there any updates on this?

Hi @rangoc

I definitely owe this ticket an update; thank you for weighing in as well, it always helps with prioritization to know when multiple experience the same scenario. I apologize @mikeyaa that I have kept you waiting.

I did start to look at this and found there was some hidden complexity with remove, namely when that property is a composite attribute. I see a path to adding it, though I think it's best to wrap this update into another request to be able to drop and reindex indexes because there is a lot of overlapping parts. I also found that currently the remove method needs a little bit of attention in this area when the attribute is a composite attribute to a key, so that will also need to be addressed.

I can't really commit right now to how long this might take, we our first baby in February and that has definitely slowed down my updates in the short term. That said, I fold this need into the drop/reindex addition I think I can make faster progress.

In the meantime, you might be able to get around this by avoiding the readOnly: true constraint at least temporarily and enforcing that within the app code that accesses the database. If that doesn't work maybe we could brainstorm some other way to work around this until a fix is in?

rangoc commented 7 months ago

@tywalch Thanks for such a rapid feedback 🥇 And also congratulations on the baby! :)

Our _created_at is not a part of the composite, so that's one less constraint to worry about for now I guess :)

I'll try with the workaround, and see where it gets me! :) Thanks!

rangoc commented 7 months ago

@tywalch

Hmm... just now tested the workaround, doesn't seem to have any effect.

How we decided to solve this for the time being, is by doing an additional query, querying for an item, and checking whether it already exists.

If it does, we do an update(), if not we do create(). It does cost us that one additional query, but oh well, it provides some additional data safety and more peaceful mind :)

tywalch commented 7 months ago

If you share your model I could put something more specific together for you, though here's an example of what I meant:

https://electrodb.fun/?ssl=59&ssc=1&pln=58&pc=8#code/JYWwDg9gTgLgBAbzgUQHY2DAngGjgZQFMoA3YAY0LgF84AzKCEOAIkIBtDyZGATAIxYBuAFAjyEVAGd4MAIb9OcALyssEAK5QA+vMWFtqOSELCxE6fDCMAVl3irUhAO4p0mLAAoRcRD99wIBC8HABcfgEBhO7Y4SzWEHbcLDj+kSTEUsCScQCMKWkBUsRklHHyUgDWcmBgLIXUqZFyMDzA-BowhFLhCIW+Xca9-QHYYIRxMlDAqADmBZGRUIQAjhrAy7zhPBqEI40jRibDi6NY45NtcwuncMtrG4RbcDuETYsHpyByRrPEJ7cxhNWFMZvN9u9IuRli0ngBBGAA05AuKoDQgfjEG6nEJ0OQadiIuCeACUKgAfHAACKwgB0qAgzlJkMWxSJpIp1LpDKZJJZvk+iw0YF4sN4CKRixRrDRGKx-ICAHpFXBnC1yAALejQOCan5-KQvCBwH5YE2taYdLojXxqmCauIAKmxi2VcDZuq0y3QL1A3Xk4FVGuicGFoq6vBt7sI7LJykpNK69MZzKjMN4AHlUOwsNsoLt9g0WTMQgAPbqS3wJJIwHoRW5gSqVxZ0YAcZ7xSou04ScAQLJdcIAbRYgxALAAulHBacqs3Iq325MuwrIm7oYRYbqmJAB1RKoQsIa6DqwHJYMA5Ox3dB4AezWtiG2pFHfL3d5hgSOjqYp7caIWkTUP4gpIHoSjASSogiNW9i0mGsKeEgP7hAA5PwcjkAeqC8AAtMseLcNAqF4GOaESMsqE0CS-i0sAdAAHIQDAyClsAMhSEhuowhGEpckmPIctQNG+LSbJId8vz-HAqFQPqEDkKhwm0bMECkiIQA

The use of ifNotExists is a good fit for createdAt, but requires the attribute to not be readOnly: true.

Depending on your model, this approach may/may not work well, but there could be other options depending on your specifics

tywalch commented 7 months ago

I'll also add there is also a patch method which only updates the record if it exists (via a ConditionExpression). If you do move forward with a two-step operation I'd recommend using that method over update.

I don't know your architecture obviously, but if you believe the item exists at the time of write, a patch will ensure those cases are single-step and then you can always "fail back" to a create. Both those options are "safer" because they will enforce your expectations.

rangoc commented 7 months ago

@tywalch Sorry, just now seeing that you've replied. Thanks for trying to help! 🙌🏻

Here's the model:

export const UserDataEntity = makeOnce(
  () =>
    new Entity(
      {
        model: {
          service: 'user',
          entity: 'UserData',
          version: '1',
        },
        attributes: {
          user_id: {
            type: 'string',
            required: true,
          },
          first_name: {
            type: 'string',
            required: true,
          },
          last_name: {
            type: 'string',
            required: true,
          },
          country_id: {
            type: 'number',
            required: true,
          },
          organisation_title: {
            type: 'string',
          },
          job_level_id: {
            type: 'number',
            required: true,
          },
          job_function_id: {
            type: 'number',
            required: true,
          },
          job_industry_id: {
            type: 'number',
          },
          job_title: {
            type: 'string',
          },
          _updated_at: {
            type: 'string',
            readOnly: true,
            watch: '*',
            set: () => new Date().toISOString(),
          },
          _created_at: {
            type: 'string',
            required: true,
            readOnly: true,
            set: () => new Date().toISOString(),
            default: () => new Date().toISOString(),
          },
        },
        indexes: {
          byUserId: {
            pk: {
              field: 'pk',
              composite: ['user_id'],
            },
            // pk: $user#user_id_
            sk: {
              field: 'sk',
              composite: [],
            },
          },
        },
      },
      {
        identifiers: {
          entity: '_type',
          version: '_version',
        },
        // logger: console.log,
        client: ElectroDbClient().client,
        table: ElectroDbClient().tableName,
      } satisfies EntityConfiguration,
    ),
);

Here's the model.