-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
Add rocksdb_create_column_families_with_options to C API. #12896
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1145,6 +1145,34 @@ rocksdb_column_family_handle_t** rocksdb_create_column_families( | |
return c_handles; | ||
} | ||
|
||
rocksdb_column_family_handle_t** rocksdb_create_column_families_with_options( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know the other APIs don't have method comments, but maybe we could start ? In particular state that this method takes a set of options per CF. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but wasn't be better to add doc comments to c.h file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are correct, at least in this case where we are documenting the API. Thanks for doing that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we implement a TTL variant, or is that enough scope ? I prefer to restrict the scope but worth thinking about. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the first C API. I would like to start small and then we can add more later. Once API is out, it's hard to remove it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree. |
||
rocksdb_t* db, int num_column_families, | ||
const char* const* column_family_names, | ||
const rocksdb_options_t* const* column_family_options, size_t* lencfs, | ||
char** errptr) { | ||
std::vector<ColumnFamilyHandle*> handles; | ||
|
||
std::vector<ColumnFamilyDescriptor> column_families; | ||
column_families.reserve(num_column_families); | ||
for (int i = 0; i < num_column_families; i++) { | ||
column_families.emplace_back( | ||
std::string(column_family_names[i]), | ||
ColumnFamilyOptions(column_family_options[i]->rep)); | ||
} | ||
SaveError(errptr, db->rep->CreateColumnFamilies(column_families, &handles)); | ||
|
||
*lencfs = handles.size(); | ||
rocksdb_column_family_handle_t** c_handles = | ||
static_cast<rocksdb_column_family_handle_t**>( | ||
malloc(sizeof(rocksdb_column_family_handle_t*) * handles.size())); | ||
for (size_t i = 0; i != handles.size(); ++i) { | ||
c_handles[i] = new rocksdb_column_family_handle_t; | ||
c_handles[i]->rep = handles[i]; | ||
c_handles[i]->immortal = false; | ||
} | ||
return c_handles; | ||
} | ||
|
||
void rocksdb_create_column_families_destroy( | ||
rocksdb_column_family_handle_t** list) { | ||
free(list); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approve of changing the parameter order so as not to allow confusion of options[] and options* ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @alanpaxton, I don't understand what are you saying. Can you please describe it more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are different
rocksdb_create_column_famil[y|ies]
methods with different orders of parameters. I was confused by the different orders of parameters. But I think your function has the "correct" order of parameters for what it does.