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
986 stars 60 forks source link

Incorrect params for patch/update for clustered index with no sort key #412

Open santiagomera opened 1 month ago

santiagomera commented 1 month ago

Describe the bug When executing a patch/update, the GSI sort key is changed from the value generated at create, causing that the entity is not returned when queried using that GSI.

At create: image

At update: image

When querying: image

Because the update removed the #myentity_1 suffix from gsic1sk, the query conditions won't match and thus the entity is excluded from the query results.

ElectroDB Version 2.14.2

ElectroDB Playground Link Playground Link

Entity/Service Definitions

const MyEntity = new Entity({
  model: {
    version: '1',
    service: 'app',
    entity: 'myEntity',
  },
  attributes: {
    id: { type: 'string', required: true },
    name: { type: 'string', required: true },
    date: { type: 'string', required: true },
  },
  indexes: {
    primary: {
      pk: { field: 'pk', composite: ['id'] },
      sk: { field: 'sk', composite: [] }
    },
    byName: {
      collection: 'names',
      type: 'clustered',
      index: 'gsic1',
      pk: { field: 'gsic1pk', composite: ['name'] },
      sk: { field: 'gsic1sk', composite: [] }
    },
    byDate: {
      collection: "dates",
      index: 'gsic2',
      pk: { field: 'gsic2pk', composite: ['date'] },
      sk: { field: 'gsic2sk', composite: [] }
    }
  }
}, { table });

Expected behavior That the format of the GSI attributes is not altered during patch/updates.

Errors None

Additional context

santiagomera commented 1 month ago

Related discussion I could find: https://github.com/tywalch/electrodb/discussions/361

daniel7byte commented 1 month ago

Thanks @santiagomera for the reference https://github.com/tywalch/electrodb/discussions/361 and @tywalch for your help in this topic!

tywalch commented 1 month ago

Firstly, thank you for raising this; this is not a good bug in the slightest. I looked into this and it should be a small tweak, I really wish I had not somehow missed #361. You can expect a PR for this tomorrow.

Secondly, I suggest you avoid using a clustered index for your gsic1 index. Because it does not have an SK attribute, it is functionally the same as an isolated index. Also, because this is an edge case for clustered indexes with no sk composite attributes, you wouldn't have the issue you are experiencing.

santiagomera commented 1 month ago

Thanks Tyler! Your suggestion is in fact what we did to resolve the issue, which got me thinking maybe we weren't using the index in the best way.

The reasoning behind using a clustered index was that it is used in a collection, and the docs mention a clustered index works better for querying across entities (the playground example was simplified for focusing on the issue).

Thank you very much for the quick response, and for the amazing work you are doing with this project!