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

Use CQL Order By for non ANN sort #1635

Merged
merged 33 commits into from
Nov 4, 2024
Merged

Use CQL Order By for non ANN sort #1635

merged 33 commits into from
Nov 4, 2024

Conversation

amorton
Copy link
Contributor

@amorton amorton commented Nov 1, 2024

What this PR does:

Which issue(s) this PR fixes:
Fixes #1628

Checklist

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

amorton added 24 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
@amorton amorton requested a review from a team as a code owner November 1, 2024 20:57
The command used columns in the sort clause that are not part of the partition sorting, and so the query was sorted in memory.

The table ${keyspace}.${table} has the partition sorting columns: ${partitionSorting}.
The command sorted on the columns: ${sortColumns}.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See another comment in TableSortClauseResolver.java, ${sortColumns} has been filtered in stream, so not actually columns we use for in-memory sort.

Copy link
Contributor

@Yuqi-Du Yuqi-Du left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

several minor comments, approved.

amorton added a commit that referenced this pull request Nov 3, 2024
@amorton amorton merged commit 0a1db86 into main Nov 4, 2024
3 checks passed
@amorton amorton deleted the ajm/cluster-sort-tables branch November 4, 2024 00:49
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.

Turn table sort clause that uses clustering keys correctly into CQL order by
2 participants