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

deleteOne deleteMany for tables using OperationAttempt #1551

Merged
merged 23 commits into from
Oct 21, 2024

Conversation

amorton
Copy link
Contributor

@amorton amorton commented Oct 16, 2024

WIP waiting for WhereCQLClauseAnalyzer to get strategy for delete.

NOTE: no integration tests yet, and we need a where analysis strategy for deleteOne and deleteMany

For deleteOne, the full PK must be specified. For deleteMany a partial PK (with full partition keys) is acceptable.

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

amorton added 16 commits October 9, 2024 15:56
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
@amorton amorton requested a review from a team as a code owner October 16, 2024 03:07
@amorton amorton marked this pull request as draft October 16, 2024 03:08
…eration-attempt

# Conflicts:
#	src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/WhereCQLClauseAnalyzer.java
@Yuqi-Du
Copy link
Contributor

Yuqi-Du commented Oct 21, 2024

Added Integration tests for DeleteOne, DeleteMany
For DeleteOne, a full primary key must be specified in filter,
For DeleteMany, partial Primary key is allowed, but still needs to be a valid primary key filtering.
Also, common rule for both is, must have filter, must filter on existed column, can not filter on non-PK column.

Base automatically changed from ajm/warnings-full-error to main October 21, 2024 19:20
@amorton amorton marked this pull request as ready for review October 21, 2024 22:56
@amorton amorton merged commit 884e025 into main Oct 21, 2024
3 checks passed
@amorton amorton deleted the ajm/delete-command-operation-attempt branch October 21, 2024 23:17
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

Successfully merging this pull request may close these issues.

2 participants