Update on tables using OperationAttempt #1552

amorton · 2024-10-16T03:40:09Z

Waiting for the WhereCQLClauseAnalyzer to get strategy so it can check the where for a update

NOTE: builds on #1551

NOTE: no integration tests yet, and we need a where analysis strategy for updateOne - user must specify the full PK

What this PR does:

Which issue(s) this PR fixes:
Fixes #

Checklist

Changes manually tested
Automated Tests added/updated
Documentation added/updated
CLA Signed: DataStax CLA

Changes the warnings associated commands to use the new ApiExcpetions, and `warnings` in the return status now returns a list of error object V2. To do that needed to improve the way CommandResult was built, so it always had a status map so the CommandProcessor could append a warning. To do that expanded the CommandResultBuilder added for the OperationAttempts, removed the many overloads used for CommandResult, and updated all creation of the CommandResult to use the builder. See also #1518 to continue this

…cResponses Missed adding errors to the CommandResult builder for per document responses in InsertOperationPage

- change CommandErrorV2 property to match bean style - change CreateKeyspaceIntegrationTest to get new response warning

and made both create and drop tests check the names of the commands in the message

- our schema cache was not invalidating when a table was changed, so we would always give out missing index errors rather than errors when index was there but did not support the operation - updated integration tests for the full error in the warning, and made them check the id for the warning - big fix, we were not turning on allow filtering when doing a comparison query for some data types on indexed columns

Waiting for the WhereCQLClauseAnalyzer to get strategy so it can check the where for a delete

Waiting for the WhereCQLClauseAnalyzer to get strategy so it can check the where for a update

…eration-attempt # Conflicts: # src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/WhereCQLClauseAnalyzer.java

…empt' into ajm/update-command-operation-attempt

…r table updateOne

Yuqi-Du · 2024-10-21T16:07:26Z

Table UpdateOne (UpdateMany is not supported)
There are twos rules for update clause,

can not update on unknown column
can not update on primary key column

Currently, I added these rule checks in ColumnAssignment constructor. here
Thought about creating a update clause analyzer(similar to filterAnalyzer), but needs to change butch of visibility identifier, may break encapsulation. @amorton, what do you think.

Yuqi-Du · 2024-10-21T16:09:35Z

There is one thing weird, that is the status map for table updateOne.

We always construct this status map in UpdateAttemptPage.

    "status": {
        "matchedCount": 1,
        "modifiedCount": 1
    }

I know we can't know the update is successful or not, but is this response body needed?

…eration-attempt # Conflicts: # src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResultBuilder.java # src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/DeleteOperationPage.java # src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/CreateTableAttempt.java # src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/DeleteTableOperation.java # src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/UpdateTableOperation.java # src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/WhereCQLClauseAnalyzer.java # src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/TableFilterIntegrationTest.java # src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiResponseValidator.java # src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiTableCommandSender.java

…eration-attempt # Conflicts: # src/main/java/io/stargate/sgv2/jsonapi/service/operation/DeleteAttemptPage.java # src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/DeleteAttemptBuilder.java

pair up with unit tests and IT tests

…eration-attempt

…rgate/jsonapi into ajm/update-command-operation-attempt

amorton

perms broken, reviewed in person some changes

src/main/java/io/stargate/sgv2/jsonapi/exception/UpdateException.java

src/main/java/io/stargate/sgv2/jsonapi/service/resolver/update/TableUpdateAnalyzer.java

src/main/java/io/stargate/sgv2/jsonapi/service/resolver/update/TableUpdateResolver.java

src/main/java/io/stargate/sgv2/jsonapi/service/resolver/UpdateOneCommandResolver.java

src/main/java/io/stargate/sgv2/jsonapi/service/resolver/update/TableUpdateResolver.java

…eration-attempt # Conflicts: # src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/TestData.java

amorton and others added 21 commits October 9, 2024 15:56

Merge branch 'main' into ajm/warnings-full-error

7cc74b5

Merge branch 'main' into ajm/warnings-full-error

2aeb995

fixes from merge

b9cdb7f

fix failing unit tests

d3a46b8

fmt fix

ad15492

fix InsertIntegrationTest$InsertManyFails.orderedFailOnBadKeyReturnDo…

88c502c

…cResponses Missed adding errors to the CommandResult builder for per document responses in InsertOperationPage

integration test fixes

4db1c01

- change CommandErrorV2 property to match bean style - change CreateKeyspaceIntegrationTest to get new response warning

fixes for DropKeyspaceIntegrationTest

cd4969d

and made both create and drop tests check the names of the commands in the message

integration test fix

831921d

BIG FIX - created wrong type of result builder

d70ec4e

Merge branch 'main' into ajm/warnings-full-error

2fdcb0b

compile fix from merge

284ae2d

Merge branch 'main' into ajm/warnings-full-error

715b57c

Initial delete on tables using OperationAttempt

0d808e5

Waiting for the WhereCQLClauseAnalyzer to get strategy so it can check the where for a delete

Initial update on tables using OperationAttempt

3ad5b76

Waiting for the WhereCQLClauseAnalyzer to get strategy so it can check the where for a update

Merge remote-tracking branch 'origin/main' into ajm/delete-command-op…

e779ed4

…eration-attempt # Conflicts: # src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/WhereCQLClauseAnalyzer.java

add integrations tests for table DeleteOne, DeleteMany commands

e69e8a5

Merge remote-tracking branch 'origin/ajm/delete-command-operation-att…

5fa7bd3

…empt' into ajm/update-command-operation-attempt

add rules for update ColumnAssignment check, add integration tests fo…

a3c0ad2

…r table updateOne

Yuqi-Du marked this pull request as ready for review October 21, 2024 16:09

Yuqi-Du requested a review from a team as a code owner October 21, 2024 16:09

Yuqi-Du changed the title ~~Initial update on tables using OperationAttempt~~ Update on tables using OperationAttempt Oct 21, 2024

Yuqi-Du added 2 commits October 21, 2024 13:38

fix $unset, add IT

6bdaf83

Base automatically changed from ajm/delete-command-operation-attempt to main October 21, 2024 23:17

Merge remote-tracking branch 'origin/main' into ajm/update-command-op…

e9a4f5e

…eration-attempt # Conflicts: # src/main/java/io/stargate/sgv2/jsonapi/service/operation/DeleteAttemptPage.java # src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/DeleteAttemptBuilder.java

Yuqi-Du added 6 commits October 22, 2024 16:03

add updateAnalyzer,

08b660e

pair up with unit tests and IT tests

add updateAnalyzer,

147c5ad

pair up with unit tests and IT tests

Merge branch 'main' into ajm/update-command-operation-attempt

ebddd1d

won't add status if there is exception

08bdc19

Merge remote-tracking branch 'origin/main' into ajm/update-command-op…

1d84c2d

…eration-attempt

Merge branch 'ajm/update-command-operation-attempt' of github.com:sta…

d674679

…rgate/jsonapi into ajm/update-command-operation-attempt

Yuqi-Du mentioned this pull request Oct 23, 2024

Bug? - UpdateOne for table, with a filter Exception, status map is still returned. #1569

Closed

fix IT

14ec39f

amorton requested review from a team and removed request for a team October 23, 2024 17:50

amorton commented Oct 23, 2024

View reviewed changes

Yuqi-Du added 2 commits October 23, 2024 13:11

Merge remote-tracking branch 'origin/main' into ajm/update-command-op…

65f1144

…eration-attempt # Conflicts: # src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/TestData.java

fix comments, add IT

8b09404

Yuqi-Du approved these changes Oct 24, 2024

View reviewed changes

Merge branch 'main' into ajm/update-command-operation-attempt

755c980

Yuqi-Du merged commit ef33b5a into main Oct 24, 2024
3 checks passed

Yuqi-Du deleted the ajm/update-command-operation-attempt branch October 24, 2024 18:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update on tables using OperationAttempt #1552

Update on tables using OperationAttempt #1552

amorton commented Oct 16, 2024

Yuqi-Du commented Oct 21, 2024 •

edited

Loading

Yuqi-Du commented Oct 21, 2024

amorton left a comment

Update on tables using OperationAttempt #1552

Update on tables using OperationAttempt #1552

Conversation

amorton commented Oct 16, 2024

Yuqi-Du commented Oct 21, 2024 • edited Loading

Yuqi-Du commented Oct 21, 2024

amorton left a comment

Choose a reason for hiding this comment

Yuqi-Du commented Oct 21, 2024 •

edited

Loading