tywalch/electrodb

Removing attributes with upsert

mikeyaa opened this issue ยท 9 comments

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.

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

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

@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?

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?

@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!

@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 :)

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.

@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.