Skip to content

Commit 6515380

Browse files
muzarskimuzarski
committed
map: represent valid map type states via enum
It's not possible for a map type to have value type defined, when key type is not defined. A representation of map type before this commit, would technically allow us to create such state. Introduced a `MapDataType` enum which represents valid states of map data type.
1 parent 8ffac72 commit 6515380

File tree

5 files changed

+112
-93
lines changed

5 files changed

+112
-93
lines changed

‎scylla-rust-wrapper/src/cass_types.rs‎

Lines changed: 72 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,13 @@ impl Default for UDTDataType {
131131
}
132132
}
133133

134+
#[derive(Clone, Debug, PartialEq)]
135+
pub enum MapDataType {
136+
Untyped,
137+
Key(Arc<CassDataType>),
138+
KeyAndValue(Arc<CassDataType>, Arc<CassDataType>),
139+
}
140+
134141
#[derive(Clone, Debug, PartialEq)]
135142
pub enum CassDataType {
136143
Value(CassValueType),
@@ -146,10 +153,7 @@ pub enum CassDataType {
146153
frozen: bool,
147154
},
148155
Map {
149-
// None, None stands for untyped map.
150-
// Some, None stands for a map with an untyped value type.
151-
key_type: Option<Arc<CassDataType>>,
152-
val_type: Option<Arc<CassDataType>>,
156+
typ: MapDataType,
153157
frozen: bool,
154158
},
155159
// Empty vector stands for untyped tuple.
@@ -183,29 +187,22 @@ impl CassDataType {
183187
}
184188
_ => false,
185189
},
186-
CassDataType::Map {
187-
key_type: k,
188-
val_type: v,
189-
..
190-
} => match other {
191-
CassDataType::Map {
192-
key_type: k_other,
193-
val_type: v_other,
194-
..
195-
} => match ((k, v), (k_other, v_other)) {
190+
CassDataType::Map { typ: t, .. } => match other {
191+
CassDataType::Map { typ: t_other, .. } => match (t, t_other) {
196192
// See https://github.com/scylladb/cpp-driver/blob/master/src/data_type.hpp#L218
197193
// In cpp-driver the types are held in a vector.
198194
// The logic is following:
199195

200196
// If either of vectors is empty, skip the typecheck.
201-
((None, None), _) => true,
202-
(_, (None, None)) => true,
197+
(MapDataType::Untyped, _) => true,
198+
(_, MapDataType::Untyped) => true,
203199

204200
// Otherwise, the vectors should have equal length and we perform the typecheck for subtypes.
205-
((Some(k), None), (Some(k_other), None)) => k.typecheck_equals(k_other),
206-
((Some(k), Some(v)), (Some(k_other), Some(v_other))) => {
207-
k.typecheck_equals(k_other) && v.typecheck_equals(v_other)
208-
}
201+
(MapDataType::Key(k), MapDataType::Key(k_other)) => k.typecheck_equals(k_other),
202+
(
203+
MapDataType::KeyAndValue(k, v),
204+
MapDataType::KeyAndValue(k_other, v_other),
205+
) => k.typecheck_equals(k_other) && v.typecheck_equals(v_other),
209206
_ => false,
210207
},
211208
_ => false,
@@ -278,16 +275,18 @@ pub fn get_column_type_from_cql_type(
278275
frozen: *frozen,
279276
},
280277
CollectionType::Map(key, value) => CassDataType::Map {
281-
key_type: Some(Arc::new(get_column_type_from_cql_type(
282-
key,
283-
user_defined_types,
284-
keyspace_name,
285-
))),
286-
val_type: Some(Arc::new(get_column_type_from_cql_type(
287-
value,
288-
user_defined_types,
289-
keyspace_name,
290-
))),
278+
typ: MapDataType::KeyAndValue(
279+
Arc::new(get_column_type_from_cql_type(
280+
key,
281+
user_defined_types,
282+
keyspace_name,
283+
)),
284+
Arc::new(get_column_type_from_cql_type(
285+
value,
286+
user_defined_types,
287+
keyspace_name,
288+
)),
289+
),
291290
frozen: *frozen,
292291
},
293292
CollectionType::Set(set) => CassDataType::Set {
@@ -340,10 +339,19 @@ impl CassDataType {
340339
}
341340
}
342341
CassDataType::Map {
343-
key_type, val_type, ..
342+
typ: MapDataType::Untyped,
343+
..
344+
} => None,
345+
CassDataType::Map {
346+
typ: MapDataType::Key(k),
347+
..
348+
} => (index == 0).then_some(k),
349+
CassDataType::Map {
350+
typ: MapDataType::KeyAndValue(k, v),
351+
..
344352
} => match index {
345-
0 => key_type.as_ref(),
346-
1 => val_type.as_ref(),
353+
0 => Some(k),
354+
1 => Some(v),
347355
_ => None,
348356
},
349357
CassDataType::Tuple(v) => v.get(index),
@@ -361,17 +369,28 @@ impl CassDataType {
361369
}
362370
},
363371
CassDataType::Map {
364-
key_type, val_type, ..
372+
typ: MapDataType::KeyAndValue(_, _),
373+
..
374+
} => Err(CassError::CASS_ERROR_LIB_BAD_PARAMS),
375+
CassDataType::Map {
376+
typ: MapDataType::Key(k),
377+
frozen,
365378
} => {
366-
if key_type.is_some() && val_type.is_some() {
367-
Err(CassError::CASS_ERROR_LIB_BAD_PARAMS)
368-
} else if key_type.is_none() {
369-
*key_type = Some(sub_type);
370-
Ok(())
371-
} else {
372-
*val_type = Some(sub_type);
373-
Ok(())
374-
}
379+
*self = CassDataType::Map {
380+
typ: MapDataType::KeyAndValue(k.clone(), sub_type),
381+
frozen: *frozen,
382+
};
383+
Ok(())
384+
}
385+
CassDataType::Map {
386+
typ: MapDataType::Untyped,
387+
frozen,
388+
} => {
389+
*self = CassDataType::Map {
390+
typ: MapDataType::Key(sub_type),
391+
frozen: *frozen,
392+
};
393+
Ok(())
375394
}
376395
CassDataType::Tuple(types) => {
377396
types.push(sub_type);
@@ -423,8 +442,10 @@ pub fn get_column_type(column_type: &ColumnType) -> CassDataType {
423442
frozen: false,
424443
},
425444
ColumnType::Map(key, value) => CassDataType::Map {
426-
key_type: Some(Arc::new(get_column_type(key.as_ref()))),
427-
val_type: Some(Arc::new(get_column_type(value.as_ref()))),
445+
typ: MapDataType::KeyAndValue(
446+
Arc::new(get_column_type(key.as_ref())),
447+
Arc::new(get_column_type(value.as_ref())),
448+
),
428449
frozen: false,
429450
},
430451
ColumnType::Set(boxed_type) => CassDataType::Set {
@@ -475,8 +496,7 @@ pub unsafe extern "C" fn cass_data_type_new(value_type: CassValueType) -> *const
475496
},
476497
CassValueType::CASS_VALUE_TYPE_TUPLE => CassDataType::Tuple(Vec::new()),
477498
CassValueType::CASS_VALUE_TYPE_MAP => CassDataType::Map {
478-
key_type: None,
479-
val_type: None,
499+
typ: MapDataType::Untyped,
480500
frozen: false,
481501
},
482502
CassValueType::CASS_VALUE_TYPE_UDT => CassDataType::UDT(UDTDataType::new()),
@@ -673,9 +693,11 @@ pub unsafe extern "C" fn cass_data_type_sub_type_count(data_type: *const CassDat
673693
CassDataType::Value(..) => 0,
674694
CassDataType::UDT(udt_data_type) => udt_data_type.field_types.len() as size_t,
675695
CassDataType::List { typ, .. } | CassDataType::Set { typ, .. } => typ.is_some() as size_t,
676-
CassDataType::Map {
677-
key_type, val_type, ..
678-
} => key_type.is_some() as size_t + val_type.is_some() as size_t,
696+
CassDataType::Map { typ, .. } => match typ {
697+
MapDataType::Untyped => 0,
698+
MapDataType::Key(_) => 1,
699+
MapDataType::KeyAndValue(_, _) => 2,
700+
},
679701
CassDataType::Tuple(v) => v.len() as size_t,
680702
CassDataType::Custom(..) => 0,
681703
}

‎scylla-rust-wrapper/src/collection.rs‎

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::cass_error::CassError;
2-
use crate::cass_types::CassDataType;
2+
use crate::cass_types::{CassDataType, MapDataType};
33
use crate::types::*;
44
use crate::value::CassCqlValue;
55
use crate::{argconv::*, value};
@@ -18,8 +18,7 @@ static UNTYPED_SET_TYPE: CassDataType = CassDataType::Set {
1818
frozen: false,
1919
};
2020
static UNTYPED_MAP_TYPE: CassDataType = CassDataType::Map {
21-
key_type: None,
22-
val_type: None,
21+
typ: MapDataType::Untyped,
2322
frozen: false,
2423
};
2524

@@ -47,23 +46,27 @@ impl CassCollection {
4746
}
4847
}
4948
}
50-
CassDataType::Map {
51-
key_type: k_typ,
52-
val_type: v_typ,
53-
..
54-
} => {
49+
50+
CassDataType::Map { typ, .. } => {
5551
// Cpp-driver does the typecheck only if both map types are present...
5652
// However, we decided not to mimic this behaviour (which is probably a bug).
5753
// We will do the typecheck if just the key type is defined as well (half-typed maps).
58-
if let Some(k_typ) = k_typ {
59-
if index % 2 == 0 && !value::is_type_compatible(value, k_typ) {
60-
return CassError::CASS_ERROR_LIB_INVALID_VALUE_TYPE;
54+
match typ {
55+
MapDataType::Key(k_typ) => {
56+
if index % 2 == 0 && !value::is_type_compatible(value, k_typ) {
57+
return CassError::CASS_ERROR_LIB_INVALID_VALUE_TYPE;
58+
}
6159
}
62-
}
63-
if let Some(v_typ) = v_typ {
64-
if index % 2 != 0 && !value::is_type_compatible(value, v_typ) {
65-
return CassError::CASS_ERROR_LIB_INVALID_VALUE_TYPE;
60+
MapDataType::KeyAndValue(k_typ, v_typ) => {
61+
if index % 2 == 0 && !value::is_type_compatible(value, k_typ) {
62+
return CassError::CASS_ERROR_LIB_INVALID_VALUE_TYPE;
63+
}
64+
if index % 2 != 0 && !value::is_type_compatible(value, v_typ) {
65+
return CassError::CASS_ERROR_LIB_INVALID_VALUE_TYPE;
66+
}
6667
}
68+
// Skip the typecheck for untyped map.
69+
MapDataType::Untyped => (),
6770
}
6871
}
6972
_ => return CassError::CASS_ERROR_LIB_INVALID_VALUE_TYPE,
@@ -214,7 +217,7 @@ mod tests {
214217

215218
use crate::{
216219
cass_error::CassError,
217-
cass_types::{CassDataType, CassValueType},
220+
cass_types::{CassDataType, CassValueType, MapDataType},
218221
collection::{
219222
cass_collection_append_double, cass_collection_append_float, cass_collection_free,
220223
},
@@ -255,8 +258,7 @@ mod tests {
255258
// untyped map (via cass_collection_new_from_data_type - collection's type is Some(untyped_map)).
256259
{
257260
let dt = Arc::new(CassDataType::Map {
258-
key_type: None,
259-
val_type: None,
261+
typ: MapDataType::Untyped,
260262
frozen: false,
261263
});
262264

@@ -285,10 +287,9 @@ mod tests {
285287
// half-typed map (key-only)
286288
{
287289
let dt = Arc::new(CassDataType::Map {
288-
key_type: Some(Arc::new(CassDataType::Value(
290+
typ: MapDataType::Key(Arc::new(CassDataType::Value(
289291
CassValueType::CASS_VALUE_TYPE_BOOLEAN,
290292
))),
291-
val_type: None,
292293
frozen: false,
293294
});
294295

@@ -325,12 +326,12 @@ mod tests {
325326
// typed map
326327
{
327328
let dt = Arc::new(CassDataType::Map {
328-
key_type: Some(Arc::new(CassDataType::Value(
329-
CassValueType::CASS_VALUE_TYPE_BOOLEAN,
330-
))),
331-
val_type: Some(Arc::new(CassDataType::Value(
332-
CassValueType::CASS_VALUE_TYPE_SMALL_INT,
333-
))),
329+
typ: MapDataType::KeyAndValue(
330+
Arc::new(CassDataType::Value(CassValueType::CASS_VALUE_TYPE_BOOLEAN)),
331+
Arc::new(CassDataType::Value(
332+
CassValueType::CASS_VALUE_TYPE_SMALL_INT,
333+
)),
334+
),
334335
frozen: false,
335336
});
336337
let dt_ptr = Arc::into_raw(dt);

‎scylla-rust-wrapper/src/query_result.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::argconv::*;
22
use crate::cass_error::CassError;
3-
use crate::cass_types::{cass_data_type_type, CassDataType, CassValueType};
3+
use crate::cass_types::{cass_data_type_type, CassDataType, CassValueType, MapDataType};
44
use crate::inet::CassInet;
55
use crate::metadata::{
66
CassColumnMeta, CassKeyspaceMeta, CassMaterializedViewMeta, CassSchemaMeta, CassTableMeta,
@@ -1239,7 +1239,7 @@ pub unsafe extern "C" fn cass_value_primary_sub_type(
12391239
} => list.get_value_type(),
12401240
CassDataType::Set { typ: Some(set), .. } => set.get_value_type(),
12411241
CassDataType::Map {
1242-
key_type: Some(key),
1242+
typ: MapDataType::Key(key) | MapDataType::KeyAndValue(key, _),
12431243
..
12441244
} => key.get_value_type(),
12451245
_ => CassValueType::CASS_VALUE_TYPE_UNKNOWN,
@@ -1254,7 +1254,7 @@ pub unsafe extern "C" fn cass_value_secondary_sub_type(
12541254

12551255
match val.value_type.as_ref() {
12561256
CassDataType::Map {
1257-
val_type: Some(value),
1257+
typ: MapDataType::KeyAndValue(_, value),
12581258
..
12591259
} => value.get_value_type(),
12601260
_ => CassValueType::CASS_VALUE_TYPE_UNKNOWN,

‎scylla-rust-wrapper/src/session.rs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::argconv::*;
22
use crate::batch::CassBatch;
33
use crate::cass_error::*;
4-
use crate::cass_types::{get_column_type, CassDataType, UDTDataType};
4+
use crate::cass_types::{get_column_type, CassDataType, MapDataType, UDTDataType};
55
use crate::cluster::build_session_builder;
66
use crate::cluster::CassCluster;
77
use crate::exec_profile::{CassExecProfile, ExecProfileName, PerStatementExecProfile};
@@ -388,17 +388,16 @@ fn get_column_value(column: CqlValue, column_type: &Arc<CassDataType>) -> Value
388388
(
389389
CqlValue::Map(map),
390390
CassDataType::Map {
391-
key_type: Some(key_typ),
392-
val_type: Some(value_type),
391+
typ: MapDataType::KeyAndValue(key_type, value_type),
393392
..
394393
},
395394
) => CollectionValue(Collection::Map(
396395
map.into_iter()
397396
.map(|(key, val)| {
398397
(
399398
CassValue {
400-
value_type: key_typ.clone(),
401-
value: Some(get_column_value(key, key_typ)),
399+
value_type: key_type.clone(),
400+
value: Some(get_column_value(key, key_type)),
402401
},
403402
CassValue {
404403
value_type: value_type.clone(),

‎scylla-rust-wrapper/src/value.rs‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ mod tests {
402402
use scylla::frame::value::{CqlDate, CqlDecimal, CqlDuration};
403403

404404
use crate::{
405-
cass_types::{CassDataType, CassValueType, UDTDataType},
405+
cass_types::{CassDataType, CassValueType, MapDataType, UDTDataType},
406406
value::{is_type_compatible, CassCqlValue},
407407
};
408408

@@ -630,8 +630,7 @@ mod tests {
630630
});
631631

632632
let data_type_bool_float_map = Arc::new(CassDataType::Map {
633-
key_type: Some(data_type_bool.clone()),
634-
val_type: Some(data_type_float.clone()),
633+
typ: MapDataType::KeyAndValue(data_type_bool.clone(), data_type_float.clone()),
635634
frozen: false,
636635
});
637636

@@ -820,18 +819,16 @@ mod tests {
820819
});
821820

822821
let data_type_untyped_map = Arc::new(CassDataType::Map {
823-
key_type: None,
824-
val_type: None,
822+
typ: MapDataType::Untyped,
825823
frozen: false,
826824
});
827825
let data_type_typed_key_float_map = Arc::new(CassDataType::Map {
828-
key_type: Some(data_type_float.clone()),
829-
val_type: None,
826+
typ: MapDataType::Key(data_type_float.clone()),
827+
830828
frozen: false,
831829
});
832830
let data_type_float_int_map = Arc::new(CassDataType::Map {
833-
key_type: Some(data_type_float.clone()),
834-
val_type: Some(data_type_int.clone()),
831+
typ: MapDataType::KeyAndValue(data_type_float.clone(), data_type_int.clone()),
835832
frozen: false,
836833
});
837834

0 commit comments

Comments
 (0)