Add TransactWriteItems

[jpeterschmidt]

An expansion on this PR from mid-2020, this PR implements TransactWriteItems.

related to #98

[exoego]

Addresses #98

[ashkanjj]

any update from this?

[derolf]

Great PR, would love to see it merged.

[jpeterschmidt]

I've been using a local build of this branch for testing my own code for several months and haven't found any issues. I'd say this is ready for review/to be merged!

@mhart

[dimaqq]

Out of curiosity, why was it necessary to move this function and the one below?

[jpeterschmidt]

Good question! I originally wanted to reuse the updateItem action completely, but an update within a transaction has several fundamental difference from a single update:

• We have to check if there's a key conflict between an update in a transaction and any other query in the transaction. Single item updates by definition can't have conflicts.
• A transactional item can't be unlocked in the local database until all the operations are completed. Single item updates release their lock as soon as the one row is changed.
• Similarly, I'm using batch to perform the puts and deletes for transactions, I can't actually perform the update until every query in the transaction checks out as valid.

I did need these two functions, but I needed them separately from the rest of the updateItem action. Since they were being reused in multiple places and no longer "owned" by updateItem, I moved them up to index.

[bbbates]

Any update on this? Would love to see it merged in….

[ajoslin]

Likewise!

[florianbepunkt]

Is there a reason that prevents this PR from being merged? Something we could help with?

@jpeterschmidt There is a bug in the transaction implementation: Condition checks apparently don't work

  await client
    .put({
      TableName: "someTable",
      Item: {
        streamId: "someAggregateId",
        version: 1,
      },
    })
    .promise();

// this works as expected: condition is not met, op fails
  try {
    await client
      .put({
        TableName: "someTable",
        ConditionExpression: "attribute_not_exists(version)",
        Item: {
          streamId: "someAggregateId",
          version: 1,
        },
      })
      .promise();
  } 

// this does NOT works as expected: condition is not met, but op succeeds
  try {
    await client
      .transactWrite({
        TransactItems: [
          {
            Put: {
              TableName: tableName,
              ConditionExpression: "attribute_not_exists(version)",
              Item: {
                streamId: "someAggregateId",
                version: 1,
              },
            },
          },
        ],
      })
      .promise();
  }

[scorsi]

Any news ? I would like to see that feature :(

[jpeterschmidt]

@florianbepunkt Just pushed a commit that should fix your issue; I mistakenly omitted checking ConditionExpressions from Puts and Deletes. Pretty big miss on my part - thanks for catching!

@scorsi @bbbates @ajoslin no updates from me; maybe @mhart can weigh in on what else needs to happen!

[lpsinger]

Any news on this?

[dimaqq]

cc @mhart

[diogo-alexandre]

any news?

[c-py]

@mhart Would there be any chance of merging and releasing TransactWrite support?

I have been using a fork of the repo with only this PR merged:
https://github.com/c-py/dynalite

It's working really well, would love to see this merged and on NPM.

[ryanblock]

@jpeterschmidt thank you so much for all the legwork you did on this PR! It's chonky and will take me a while to review, but reviewed it shall be. Are you amenable to picking it back up together? Thanks in advance!

[jpeterschmidt]

hey @ryanblock, absolutely! I'll spend some time this week getting familiar again with what I did.

[ryanblock]

@jpeterschmidt great news! I'll dig into it as well and provide any additional specific feedback (although tbh right now you probably know this codebase better than I do!). Also, if you'd like to get more real-time, please find us in our Discord's dynalite channel.

[ryanblock]

Earlier today I updated the linting on this project, and pulled those changes (and related fixes the linter caught) over here: #174 – @jpeterschmidt I don't have write access to this PR, so would you like to merge that branch into this one?