Categorize unimplemented functions #382
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR reorganizes unimplemented functions based on their categorization from issue #373, moving functions that are not planned to be implemented to a new deleted_functions.h header while adding warnings to functions that are planned but not yet implemented.
Key changes:
- Move "not planned" functions from
cassandra.hto newdeleted_functions.hheader - Add warnings to "planned" functions in
cassandra.habout their unimplemented status - Update test files to include
deleted_functions.hand remove deprecated logging functions
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
include/deleted_functions.h |
New header containing function declarations moved from cassandra.h that are not planned for implementation |
include/cassandra.h |
Removed deleted functions and added warning comments to planned but unimplemented functions |
tests/src/integration/integration.hpp |
Added include for deleted_functions.h |
tests/src/integration/objects/cluster.hpp |
Added include for deleted_functions.h |
src/testing.hpp |
Added include for deleted_functions.h |
src/testing_unimplemented.cpp |
Added extern C wrapper around unimplemented function stubs |
scylla-rust-wrapper/src/logging.rs |
Removed deprecated logging functions |
scylla-rust-wrapper/src/api.rs |
Removed deprecated logging function exports |
tests/src/integration/tests/test_dbaas.cpp |
Deleted entire file containing DBaaS-specific tests |
examples/schema_meta/schema_meta.c |
Removed calls to deleted metadata field functions |
examples/perf/perf.c |
Removed call to deleted cass_cluster_set_queue_size_io |
examples/execution_profiles/execution_profiles.c |
Removed call to deleted cass_statement_add_key_index and updated replication strategy |
examples/cloud/cloud.c |
Deleted entire cloud example file |
examples/cloud/CMakeLists.txt |
Deleted cloud example CMake file |
examples/cloud/.gitignore |
Deleted cloud example gitignore |
Makefile |
Updated default ScyllaDB version and enabled more examples to run |
Comments suppressed due to low confidence (1)
examples/schema_meta/schema_meta.c:327
- The variables 'name' and 'value' are declared but never initialized after the function calls were removed. This will cause undefined behavior when accessing uninitialized variables.
const CassValue* value;
print_indent(indent);
printf("%.*s: ", (int)name_length, name);
print_schema_value(value);
printf("\n");
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Lorak-mmk
requested changes
Oct 2, 2025
The `extern "C"` block was missing, which is necessary for C linkage of the functions in this file. The bug hadn't manifested yet because these functions were declared in `cassandra.h` with `extern "C"`, but it's best practice to have it in the implementation file as well. This would manifest once we deleted the declarations in `cassandra.h`.
DBaaS is purely a DataStax Enterprise-specific feature, which is not supported by the driver. The test is hence not applicable.
The cloud example is not relevant neither for ScyllaDB nor Cassandra, and it uses functions which are not planned to be implemented.
wprzytula
force-pushed
the
categorize-unimplemented-functions
branch
from
ad9a34a to
dfcdb63
Compare
|
Rebased on master. |
Three examples (execution_profiles, perf, schema_meta) have been updated to not use functions that are never going to be implemented in the driver. This also allows us to enable these examples in the Makefile and run them regularly.
There is a set of functions in cassandra.h that are never going to be implemented in the CPP RS Driver. The reasons vary and are described below. These functions are moved to a separate header file, `deleted_functions.h`, which is only included by the tests (to allow building them with Rust stubs of those functions). The driver code, as well as the public header `cassandra.h`, do not include `deleted_functions.h`, so these functions are effectively removed from the public API. This is the full list of those functions, together with a short description of why they are not planned: **Never to be implemented functions** Custom types - `cass_collection_append_custom` - `cass_collection_append_custom_n` - `cass_statement_bind_custom` - `cass_statement_bind_custom_by_name` - `cass_statement_bind_custom_by_name_n` - `cass_statement_bind_custom_n` - `cass_tuple_set_custom` - `cass_tuple_set_custom_n` - `cass_value_get_custom` **DataStax-specific features** - `cass_cluster_set_cloud_secure_connection_bundle` - `cass_cluster_set_cloud_secure_connection_bundle_n` - `cass_cluster_set_cloud_secure_connection_bundle_no_ssl_lib_init` - `cass_cluster_set_cloud_secure_connection_bundle_no_ssl_lib_init_n` - `cass_cluster_set_monitor_reporting_interval` **Deprecated in CPP Driver** - `cass_cluster_set_queue_size_event` - `cass_cluster_set_max_connections_per_host` - `cass_cluster_set_reconnect_wait_time` - `cass_cluster_set_max_concurrent_creation` - `cass_cluster_set_max_concurrent_requests_threshold` - `cass_cluster_set_max_requests_per_flush` - `cass_cluster_set_write_bytes_high_water_mark` - `cass_cluster_set_write_bytes_low_water_mark` - `cass_cluster_set_pending_requests_high_water_mark` - `cass_cluster_set_pending_requests_low_water_mark` - `cass_log_cleanup` - `cass_log_set_queue_size` **Ancient features** - `cass_cluster_set_no_compact` - `cass_cluster_set_use_hostname_resolution` - I see no point in disabling hostname resolution. **Incompatible with the Rust Driver's architecture** - `cass_cluster_set_new_request_ratio` - `cass_cluster_set_max_reusable_write_objects` - `cass_cluster_set_queue_size_io` **Metadata raw string accessors** Semantics of these require internal storage of metadata in a raw string form, which is not true in Rust Driver. - `cass_aggregate_meta_field_by_name` - `cass_aggregate_meta_field_by_name_n` - `cass_function_meta_field_by_name` - `cass_function_meta_field_by_name_n` - `cass_index_meta_field_by_name` - `cass_index_meta_field_by_name_n` - `cass_keyspace_meta_field_by_name` - `cass_keyspace_meta_field_by_name_n` - `cass_table_meta_field_by_name` - `cass_table_meta_field_by_name_n` - `cass_materialized_view_meta_field_by_name` - `cass_materialized_view_meta_field_by_name_n` - `cass_column_meta_field_by_name` - `cass_column_meta_field_by_name_n` - `cass_iterator_fields_from_aggregate_meta` - `cass_iterator_fields_from_column_meta` - `cass_iterator_fields_from_function_meta` - `cass_iterator_fields_from_index_meta` - `cass_iterator_fields_from_keyspace_meta` - `cass_iterator_fields_from_materialized_view_meta` - `cass_iterator_fields_from_table_meta` - `cass_iterator_get_meta_field_name` - `cass_iterator_get_meta_field_value` **Purposely unimplemented** - `cass_statement_add_key_index` - binding values to unprepared statements is risky and unsupported. - `cass_cluster_set_prepare_on_up_or_add_host` - cluster events are unreliable and this optimisation is unlikely to work well. **Request tracing** Explanation: the semantics is weird. CPP Driver waits for tracing info to become available by performing queries to tracing tables with the parameters specified by the following functions. The problem is, **it does not expose** the results of the query; it's the user's responsibility to fetch them themselves again. This is inefficient and real pain. Therefore, we prefer the driver to provide no functionalities regarding server-side tracing, so the user must query tracing tables themselves, than to provide such a weird and incomplete semantics. - `cass_cluster_set_tracing_consistency` - `cass_cluster_set_tracing_max_wait_time` - `cass_cluster_set_tracing_retry_wait_time`
For each function in cassandra.h that is not yet implemented, add a warning comment in its documentation to inform users. We do not want to remove those functions from the header file, as they are part of the public API and may be implemented in the future. For now, use of them will result in linker errors, but at least no runtime errors will occur.
wprzytula
force-pushed
the
categorize-unimplemented-functions
branch
from
dfcdb63 to
5238fea
Compare
Lorak-mmk
requested changes
Oct 3, 2025
There was a problem hiding this comment.
I don't think we understood each other when it comes to this example.
I understand that we won't implement metadata raw string accessors because:
- It is difficult (requires duplicating work already done by Rust Driver, or exposing some new APIs from Rust Driver)
- They have no good use case and it is better to use normal metadata API that we implemented in the driver.
Now I see that in this example you removed usage of raw string metadata API, but did not replace it with normal metadata API. Why? It should be replaced, if not for other reason then to prove that our decision to not implement raw API was correct.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR treats the unimplemented functions according to their categorization in #373.
cassandra.htodeleted_functions.h. The new header is only included in tests for them to compile and correctly link against the Rust stubs (which are present only for tests).Bonus:
Default ScyllaDB version for tests is bumped to 2025.3.
Fixes: #373
Pre-review checklist
[ ] I have implemented Rust unit tests for the features/changes introduced.[ ] I have enabled appropriate tests inMakefilein{SCYLLA,CASSANDRA}_(NO_VALGRIND_)TEST_FILTER.Fixes:annotations to PR description.