Skip to content

Reduce complex type usage #223

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Reduce complex type usage #223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 9 additions & 13 deletions lib/client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ const ExecOptions = require("./execution-options");
const promiseUtils = require("./promise-utils");
const rust = require("../index");
const ResultSet = require("./types/result-set.js");
const { parseParams, convertHints } = require("./types/cql-utils.js");
const {
parseParams,
minimalParseParams,
convertHints,
} = require("./types/cql-utils.js");
const queryOptions = require("./query-options.js");
const { PreparedCache } = require("./cache.js");

Expand Down Expand Up @@ -342,13 +346,9 @@ class Client extends events.EventEmitter {
query instanceof rust.PreparedStatementWrapper
? query
: await this.rustClient.prepareStatement(query);
let parsedParams = parseParams(
statement.getExpectedTypes(),
params,
);
let parsedParams = minimalParseParams(params);
result = await this.rustClient.executePreparedUnpaged(
statement,
parsedParams,
[statement, parsedParams],
rustOptions,
);
} else {
Expand Down Expand Up @@ -525,13 +525,9 @@ class Client extends events.EventEmitter {
if (execOptions.isPrepared()) {
// Execute prepared statement, as requested by the user
const statement = await this.rustClient.prepareStatement(query);
let parsedParams = parseParams(
statement.getExpectedTypes(),
params,
);
let parsedParams = minimalParseParams(params);
result = await this.rustClient.executeSinglePage(
statement,
parsedParams,
[statement, parsedParams],
rustOptions,
pageState,
);
Expand Down
43 changes: 42 additions & 1 deletion lib/types/cql-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,47 @@ function wrapValueBasedOnType(type, value) {
return [type, value];
}

function wrapValueLight(value) {
switch (true) {
// For these types, no action is required
case value instanceof Uuid:
case rust.CqlType.Uuid:
value = value.getInternal();
break;

default:
break;
// // Or not yet implemented type
// throw new ReferenceError(
// `[INTERNAL DRIVER ERROR] Unknown type: ${type}`,
// );
}
return value;
}

function minimalParseParams(params) {
if (!params) return [];
let res = [];
for (let i = 0; i < params.length; i++) {
if (params[i] === types.unset) {
// Rust driver works only with version >= 4 of CQL [Source?], so unset will always be supported.
// Currently we don't support unset values and we treat them as NULL values.
// TODO: Fix this
params[i] = null;
}

// undefined as null depends on encodingOptions.useUndefinedAsUnset option
// TODO: Add support for this option
if (params[i] === null || params[i] === undefined) {
res.push([]);
continue;
}

res.push(wrapValueLight(params[i]));
}
return res;
}

/**
* Parses array of params into expected format according to preparedStatement expected types
*
Expand Down Expand Up @@ -421,8 +462,8 @@ function rustConvertHint(object) {
}

module.exports.parseParams = parseParams;
module.exports.minimalParseParams = minimalParseParams;
module.exports.convertHints = convertHints;
module.exports.rustConvertHint = rustConvertHint;

// For unit test usage
module.exports.getWrapped = getWrapped;
60 changes: 57 additions & 3 deletions src/requests/parameter_wrappers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use napi::{
use scylla::value::{Counter, CqlTimestamp, CqlTimeuuid, CqlValue, MaybeUnset};

use crate::{
requests::request::PreparedStatementWrapper,
types::{
duration::DurationWrapper,
inet::InetAddressWrapper,
Expand All @@ -16,7 +17,12 @@ use crate::{
};

pub struct ParameterWrapper {
pub(crate) row: Option<MaybeUnset<CqlValue>>,
pub(crate) value: Option<MaybeUnset<CqlValue>>,
}

pub struct ArgumentsWrapper<'a> {
pub(crate) row: Vec<ParameterWrapper>,
pub(crate) statement: &'a PreparedStatementWrapper,
}

/// Converts an array of values into Vec of CqlValue based on the provided type.
Expand Down Expand Up @@ -178,7 +184,7 @@ impl FromNapiValue for ParameterWrapper {
let parsing_error = || {
napi::Error::new(
Status::InvalidArg,
"Unexpected data when parsing parameters row".to_owned(),
"Unexpected data when parsing parameters value".to_owned(),
)
};

Expand All @@ -200,6 +206,54 @@ impl FromNapiValue for ParameterWrapper {
}
};

Ok(ParameterWrapper { row: val })
Ok(ParameterWrapper { value: val })
}
}

impl FromNapiValue for ArgumentsWrapper<'_> {
unsafe fn from_napi_value(
env: napi::sys::napi_env,
napi_val: napi::sys::napi_value,
) -> napi::Result<Self> {
let parsing_error = || {
napi::Error::new(
Status::InvalidArg,
"Unexpected data when parsing parameters row".to_owned(),
)
};

let elem = unsafe { Array::from_napi_value(env, napi_val)? };
// println!("Light parameters parsing");

if elem.len() != 2 {
return Err(parsing_error());
}
let statement = elem
.get::<&PreparedStatementWrapper>(0)?
.ok_or_else(parsing_error)?;

let arguments = elem.get::<Array>(1)?.ok_or_else(parsing_error)?;
let expected_types = statement.get_expected_types();

// expected types is limited by max number of columns
if arguments.len() != expected_types.len().try_into().unwrap() {
return Err(parsing_error());
}
let mut values = vec![];

for (i, typ) in expected_types.into_iter().enumerate() {
values.push(ParameterWrapper {
value: Some(MaybeUnset::Set(cql_value_from_napi_value(
&typ,
&arguments,
i.try_into().unwrap(),
)?)),
});
}

Ok(ArgumentsWrapper {
row: values,
statement,
})
}
}
24 changes: 12 additions & 12 deletions src/session.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use scylla::value::{CqlValue, MaybeUnset};

use crate::options;
use crate::paging::{PagingResult, PagingStateWrapper};
use crate::requests::parameter_wrappers::ParameterWrapper;
use crate::requests::parameter_wrappers::{ArgumentsWrapper, ParameterWrapper};
use crate::requests::request::QueryOptionsWrapper;
use crate::utils::{bigint_to_i64, js_error};
use crate::{
Expand Down Expand Up @@ -96,7 +96,7 @@ impl SessionWrapper {
) -> napi::Result<QueryResultWrapper> {
let statement: Statement = apply_statement_options(query.into(), options)?;
let params_vec: Vec<Option<MaybeUnset<CqlValue>>> =
params.into_iter().map(|e| e.row).collect();
params.into_iter().map(|e| e.value).collect();
let query_result = self
.inner
.get_session()
Expand Down Expand Up @@ -136,13 +136,12 @@ impl SessionWrapper {
#[napi]
pub async fn execute_prepared_unpaged(
&self,
query: &PreparedStatementWrapper,
params: Vec<ParameterWrapper>,
params: ArgumentsWrapper<'_>,
options: &QueryOptionsWrapper,
) -> napi::Result<QueryResultWrapper> {
let params_vec: Vec<Option<MaybeUnset<CqlValue>>> =
params.into_iter().map(|e| e.row).collect();
let query = apply_prepared_options(query.prepared.clone(), options)?;
params.row.into_iter().map(|e| e.value).collect();
let query = apply_prepared_options(params.statement.prepared.clone(), options)?;
QueryResultWrapper::from_query(
self.inner
.get_session()
Expand All @@ -163,7 +162,7 @@ impl SessionWrapper {
) -> napi::Result<QueryResultWrapper> {
let params_vec: Vec<Vec<Option<MaybeUnset<CqlValue>>>> = params
.into_iter()
.map(|e| e.into_iter().map(|f| f.row).collect())
.map(|e| e.into_iter().map(|f| f.value).collect())
.collect();
QueryResultWrapper::from_query(
self.inner
Expand Down Expand Up @@ -192,7 +191,8 @@ impl SessionWrapper {
let paging_state = paging_state
.map(|e| e.inner.clone())
.unwrap_or(PagingState::start());
let values: Vec<Option<MaybeUnset<CqlValue>>> = params.into_iter().map(|e| e.row).collect();
let values: Vec<Option<MaybeUnset<CqlValue>>> =
params.into_iter().map(|e| e.value).collect();

let (result, paging_state_response) = self
.inner
Expand All @@ -217,16 +217,16 @@ impl SessionWrapper {
#[napi]
pub async fn execute_single_page(
&self,
query: &PreparedStatementWrapper,
params: Vec<ParameterWrapper>,
params: ArgumentsWrapper<'_>,
options: &QueryOptionsWrapper,
paging_state: Option<&PagingStateWrapper>,
) -> napi::Result<PagingResult> {
let paging_state = paging_state
.map(|e| e.inner.clone())
.unwrap_or(PagingState::start());
let values: Vec<Option<MaybeUnset<CqlValue>>> = params.into_iter().map(|e| e.row).collect();
let prepared = apply_prepared_options(query.prepared.clone(), options)?;
let values: Vec<Option<MaybeUnset<CqlValue>>> =
params.row.into_iter().map(|e| e.value).collect();
let prepared = apply_prepared_options(params.statement.prepared.clone(), options)?;

let (result, paging_state) = self
.inner
Expand Down
6 changes: 3 additions & 3 deletions src/tests/request_values_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ pub fn tests_from_value(test: String, value: ParameterWrapper) {
_ => CqlValue::Empty,
};
assert_eq!(
match value.row {
match value.value {
Some(v) => {
match v {
scylla::value::MaybeUnset::Unset => panic!("Expected set value"),
Expand All @@ -119,7 +119,7 @@ pub fn tests_from_value(test: String, value: ParameterWrapper) {

#[napi]
pub fn tests_parameters_wrapper_unset(value: ParameterWrapper) {
match value.row {
match value.value {
Some(v) => match v {
scylla::value::MaybeUnset::Unset => (),
scylla::value::MaybeUnset::Set(_) => panic!("Expected unset value"),
Expand All @@ -130,7 +130,7 @@ pub fn tests_parameters_wrapper_unset(value: ParameterWrapper) {

#[napi]
pub fn tests_parameters_wrapper_null(value: ParameterWrapper) {
if value.row.is_some() {
if value.value.is_some() {
panic!("Expected none value")
}
}
Loading