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

Invalid params generated for patch/update where table/GSI specified with Composite Attribute Templates #343

Open
rogersillito opened this issue Jan 4, 2024 · 1 comment

Comments

@rogersillito
Copy link

rogersillito commented Jan 4, 2024

Describe the bug
We need to do an update to a field "status" (that is itself the primary key of a GSI) where that update includes a ConditionExpression to ensure the status has the expected value before applying the update. Our index definitions use attribute templates (rationale below!). When we try to patch (or update) in this way, the params that are generated result in an update expression where setting of the status field is duplicated twice - this results in a DynamoDB error when the patch/update is executed.

image

If I remove the GSI, then the patch/upate params are generated correctly, albeit we can't then benefit from that index (Playground demo of this).

ElectroDB Version
2.12.2

ElectroDB Playground Link
ElectroDB Playground Example "patch" mutation

Entity/Service Definitions

{
    model: {
      entity: 'activeApplication',
      version: '1',
      service: 'internalAppPortal',
    },
    attributes: {
      username: {
        type: 'string',
        field: 'un',
        required: true,
      },
      applicationKey: {
        type: 'string',
        field: 'ak',
        required: true,
      },
      status: {
        type: [
          'SPAWN_REQUESTED',
          'SPAWNING_CODEBUILD',
          'SPAWNING_READINESS_CHECK',
          'SPAWNED',
          'REMOVE_REQUESTED',
          'REMOVING_CODEBUILD',
          'FAILED',
        ] as const,
        field: 'st',
        required: true,
      },
      hostName: {
        type: 'string',
        field: 'hn',
        readOnly: true,
        required: true,
      },
      spawnId: {
        type: 'string',
        field: 'si',
      },
      removeId: {
        type: 'string',
        field: 'ri',
      },
    },
    indexes: {
      byUsername: {
        pk: {
          field: 'un',
          composite: ['username'],
          template: '${username}',
          casing: 'none',
        },
        sk: {
          field: 'ak',
          composite: ['applicationKey'],
          template: '${applicationKey}',
          casing: 'none',
        },
      },
      byStatus: {
        index: 'active-applications-by-status',
        pk: {
          field: 'st',
          composite: ['status'],
          template: '${status}',
          casing: 'none',
        },
      },
    },
  }

Expected behavior
The patch params should be generated without the duplicated setting of "status".

Errors

{
  "message": "Error thrown by DynamoDB client: \"Invalid UpdateExpression: Two document paths overlap with each other; must remove or rewrite one of these paths; path one: [st], path two: [__edb_e__]\" - For more detail on this error reference: https://electrodb.dev/en/reference/errors/#aws-error",
  "name": "ElectroError",
  "ref": {
    "code": 4001,
    "section": "aws-error",
    "name": "AWSError"
  },
  "code": 4001,
  "date": 1704387127434,
  "isElectroError": true
}

stack trace:

    at Entity.go (C:\DEV\internal-app-portal\admin-app\node_modules\electrodb\src\entity.js:497:20)
    at Object.action (C:\DEV\internal-app-portal\admin-app\node_modules\electrodb\src\clauses.js:1240:23)
    at current.<computed> [as go] (C:\DEV\internal-app-portal\admin-app\node_modules\electrodb\src\clauses.js:1343:41)
    at executeDbOps (webpack-internal:///(api)/./electrodbtest.ts:199:71)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

Additional context
We have a very simple use-case where our app has only a single entity and a single DynamoDB table needed to store it in. Our keys and access patterns naturally align with the primary key/sort key requirement of DynamoDB. For this reason we would prefer not to use the "default", Single Table Design orientated behaviour of electrodb around indexes which embeds the additional attribute metadata within the pk/sk values. This is why we are going against your recommendation and making use of index attribute templates. If there's an alternative way we could get to the same place and avoid the issue we seem to have hit, that would be good to know?

@tywalch
Copy link
Owner

tywalch commented Jan 4, 2024

Hi @rogersillito 👋

Oh darn, thank you for raising this, I will take a look and get back to you soon!

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

No branches or pull requests

2 participants