Skip to content

implement cass_table_meta_column #228

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

Merged
merged 2 commits into from
Mar 13, 2025

Conversation

muzarski
Copy link
Collaborator

@muzarski muzarski commented Mar 11, 2025

The order of table columns in cpp-driver (and in DESCRIBE KEYSPACE from cqlsh):

  1. pk columns sorted by position
  2. ck columns sorted by position
  3. remaining columns sorted alphabetically

This is exactly how I implemented cass_table_meta_column.

My only concern is that every time the column is not a part of neither pk nor ck, we need to allocate a vector representing the remaining columns, and sort it. I wonder if it's worth adding such vector to CassTableMeta, so we do that only one time.

Advantage of other solution is obviously that we increase the performance of cass_table_meta_column - I'm not sure how often this method is called by the users.
Disadvantage is that we need to store another vector for each table metadata. It means more memory consumption by metadata structs, which is already big enough.

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • PR description sums up the changes and reasons why they should be introduced.
  • [ ] I have implemented Rust unit tests for the features/changes introduced.
  • I have enabled appropriate tests in .github/workflows/build.yml in gtest_filter.
  • I have enabled appropriate tests in .github/workflows/cassandra.yml in gtest_filter.

@muzarski muzarski self-assigned this Mar 11, 2025
@muzarski muzarski added this to the 0.4 milestone Mar 11, 2025
@muzarski muzarski requested review from wprzytula and Lorak-mmk and removed request for wprzytula March 11, 2025 16:34
@muzarski muzarski force-pushed the table-meta-column branch from 7c30297 to b0a88ef Compare March 11, 2025 16:43
@muzarski
Copy link
Collaborator Author

v1.1: Changed EXPECT_EQ to ASSERT_EQ in test.

@muzarski muzarski force-pushed the table-meta-column branch from b0a88ef to d65f06b Compare March 11, 2025 22:33
@muzarski
Copy link
Collaborator Author

v1.2: Addressed review comments

@muzarski muzarski requested review from wprzytula and Lorak-mmk March 11, 2025 22:34
@muzarski muzarski merged commit 33ed7f1 into scylladb:master Mar 13, 2025
11 checks passed
@muzarski muzarski deleted the table-meta-column branch March 13, 2025 12:36
@muzarski muzarski mentioned this pull request Apr 14, 2025
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.

3 participants