Skip to content

Commit

Permalink
Merge pull request #187 from muzarski/enable-named-parameters-tests
Browse files Browse the repository at this point in the history 
binding: fix by-name parameter binding and enable corresponding IT
  • Loading branch information
@muzarski
muzarski authored Dec 11, 2024
2 parents bfae66b + 658ebb0 commit cb0077b
Show file tree
Hide file tree
Showing 8 changed files with 398 additions and 148 deletions.
2 changes: 2 additions & 0 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ SCYLLA_TEST_FILTER := $(subst ${SPACE},${EMPTY},ClusterTests.*\
:SerialConsistencyTests.*\
:HeartbeatTests.*\
:PreparedTests.*\
:NamedParametersTests.*\
:CassandraTypes/CassandraTypesTests/*.Integration_Cassandra_*\
:BatchSingleNodeClusterTests*:BatchCounterSingleNodeClusterTests*:BatchCounterThreeNodeClusterTests*\
:ErrorTests.*\
Expand Down Expand Up @@ -40,6 +41,7 @@ CASSANDRA_TEST_FILTER := $(subst ${SPACE},${EMPTY},ClusterTests.*\
:SerialConsistencyTests.*\
:HeartbeatTests.*\
:PreparedTests.*\
:NamedParametersTests.*\
:CassandraTypes/CassandraTypesTests/*.Integration_Cassandra_*\
:ErrorTests.*\
:SslClientAuthenticationTests*:SslNoClusterTests*:SslNoSslOnClusterTests*:SslTests*\
Expand Down
14 changes: 9 additions & 5 deletions scylla-rust-wrapper/src/batch.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::cass_types::CassConsistency;
use crate::cass_types::{make_batch_type, CassBatchType};
use crate::exec_profile::PerStatementExecProfile;
use crate::retry_policy::CassRetryPolicy;
use crate::statement::{CassStatement, Statement};
use crate::statement::{BoundStatement, CassStatement};
use crate::types::*;
use crate::value::CassCqlValue;
use scylla::batch::Batch;
Expand Down Expand Up @@ -166,11 +166,15 @@ pub unsafe extern "C" fn cass_batch_add_statement(
let statement = BoxFFI::as_ref(statement);

match &statement.statement {
Statement::Simple(q) => state.batch.append_statement(q.query.clone()),
Statement::Prepared(p) => state.batch.append_statement(p.statement.clone()),
BoundStatement::Simple(q) => {
state.batch.append_statement(q.query.clone());
state.bound_values.push(q.bound_values.clone());
}
BoundStatement::Prepared(p) => {
state.batch.append_statement(p.statement.statement.clone());
state.bound_values.push(p.bound_values.clone());
}
};

state.bound_values.push(statement.bound_values.clone());

CassError::CASS_OK
}
10 changes: 9 additions & 1 deletion scylla-rust-wrapper/src/cass_error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use scylla::transport::errors::*;
// Re-export error types.
pub(crate) use crate::cass_error_types::{CassError, CassErrorSource};
use crate::query_error::CassErrorResult;
use crate::statement::UnknownNamedParameterError;

pub trait ToCassError {
fn to_cass_error(&self) -> CassError;
Expand Down Expand Up @@ -94,7 +95,14 @@ impl ToCassError for BadQuery {
BadQuery::ValuesTooLongForKey(_usize, _usize2) => CassError::CASS_ERROR_LAST_ENTRY,
BadQuery::BadKeyspaceName(_bad_keyspace_name) => CassError::CASS_ERROR_LAST_ENTRY,
BadQuery::Other(_other_query) => CassError::CASS_ERROR_LAST_ENTRY,
BadQuery::SerializationError(_) => CassError::CASS_ERROR_LAST_ENTRY,
BadQuery::SerializationError(e) => {
if e.downcast_ref::<UnknownNamedParameterError>().is_some() {
// It means that our custom `UnknownNamedParameterError` was returned.
CassError::CASS_ERROR_LIB_NAME_DOES_NOT_EXIST
} else {
CassError::CASS_ERROR_LAST_ENTRY
}
}
BadQuery::TooManyQueriesInBatchStatement(_) => CassError::CASS_ERROR_LAST_ENTRY,
// BadQuery is non_exhaustive
// For now, since all other variants return LAST_ENTRY,
Expand Down
9 changes: 6 additions & 3 deletions scylla-rust-wrapper/src/prepared.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
cass_error::CassError,
cass_types::{get_column_type, CassDataType},
query_result::CassResultMetadata,
statement::{CassStatement, Statement},
statement::{BoundPreparedStatement, BoundStatement, CassStatement},
types::size_t,
};
use scylla::prepared_statement::PreparedStatement;
Expand Down Expand Up @@ -88,11 +88,14 @@ pub unsafe extern "C" fn cass_prepared_bind(

// cloning prepared statement's arc, because creating CassStatement should not invalidate
// the CassPrepared argument
let statement = Statement::Prepared(prepared);

let statement = BoundStatement::Prepared(BoundPreparedStatement {
statement: prepared,
bound_values: vec![Unset; bound_values_size],
});

BoxFFI::into_ptr(Box::new(CassStatement {
statement,
bound_values: vec![Unset; bound_values_size],
paging_state: PagingState::start(),
// Cpp driver disables paging by default.
paging_enabled: false,
Expand Down
33 changes: 20 additions & 13 deletions scylla-rust-wrapper/src/session.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ use crate::metadata::create_table_metadata;
use crate::metadata::{CassKeyspaceMeta, CassMaterializedViewMeta, CassSchemaMeta};
use crate::prepared::CassPrepared;
use crate::query_result::{CassResult, CassResultKind, CassResultMetadata};
use crate::statement::CassStatement;
use crate::statement::Statement;
use crate::statement::{BoundStatement, CassStatement, SimpleQueryRowSerializer};
use crate::types::{cass_uint64_t, size_t};
use crate::uuid::CassUuid;
use scylla::frame::types::Consistency;
Expand Down Expand Up @@ -258,7 +257,6 @@ pub unsafe extern "C" fn cass_session_execute(
let statement_opt = BoxFFI::as_ref(statement_raw);
let paging_state = statement_opt.paging_state.clone();
let paging_enabled = statement_opt.paging_enabled;
let bound_values = statement_opt.bound_values.clone();
let request_timeout_ms = statement_opt.request_timeout_ms;

let mut statement = statement_opt.statement.clone();
Expand All @@ -282,8 +280,8 @@ pub unsafe extern "C" fn cass_session_execute(
.await?;

match &mut statement {
Statement::Simple(query) => query.query.set_execution_profile_handle(handle),
Statement::Prepared(prepared) => Arc::make_mut(prepared)
BoundStatement::Simple(query) => query.query.set_execution_profile_handle(handle),
BoundStatement::Prepared(prepared) => Arc::make_mut(&mut prepared.statement)
.statement
.set_execution_profile_handle(handle),
}
Expand All @@ -304,10 +302,15 @@ pub unsafe extern "C" fn cass_session_execute(
QueryError,
>;
let query_res: QueryRes = match statement {
Statement::Simple(query) => {
BoundStatement::Simple(query) => {
// We don't store result metadata for Queries - return None.
let maybe_result_metadata = None;

let bound_values = SimpleQueryRowSerializer {
bound_values: query.bound_values,
name_to_bound_index: query.name_to_bound_index,
};

if paging_enabled {
session
.query_single_page(query.query, bound_values, paging_state)
Expand All @@ -326,19 +329,23 @@ pub unsafe extern "C" fn cass_session_execute(
})
}
}
Statement::Prepared(prepared) => {
BoundStatement::Prepared(prepared) => {
// Clone result metadata, so we don't need to construct it from scratch in
// `CassResultMetadata::from_column_specs` - it requires a lot of allocations for complex types.
let maybe_result_metadata = Some(Arc::clone(&prepared.result_metadata));
let maybe_result_metadata = Some(Arc::clone(&prepared.statement.result_metadata));

if paging_enabled {
session
.execute_single_page(&prepared.statement, bound_values, paging_state)
.execute_single_page(
&prepared.statement.statement,
prepared.bound_values,
paging_state,
)
.await
.map(|(qr, psr)| (qr, psr, maybe_result_metadata))
} else {
session
.execute_unpaged(&prepared.statement, bound_values)
.execute_unpaged(&prepared.statement.statement, prepared.bound_values)
.await
.map(|result| {
(
Expand Down Expand Up @@ -385,9 +392,9 @@ pub unsafe extern "C" fn cass_session_prepare_from_existing(

CassFuture::make_raw(async move {
let query = match &statement {
Statement::Simple(q) => q,
Statement::Prepared(ps) => {
return Ok(CassResultValue::Prepared(ps.clone()));
BoundStatement::Simple(q) => q,
BoundStatement::Prepared(ps) => {
return Ok(CassResultValue::Prepared(ps.statement.clone()));
}
};

Expand Down
Loading

0 comments on commit cb0077b

Please sign in to comment.