Skip to content

Commit

Permalink
map_from_key_values & map_from_entries
Browse files Browse the repository at this point in the history
  • Loading branch information
xxchan committed Aug 18, 2024
1 parent 3a757a6 commit b92834f
Show file tree
Hide file tree
Showing 15 changed files with 209 additions and 72 deletions.
139 changes: 116 additions & 23 deletions e2e_test/batch/types/map.slt.part
Original file line number Diff line number Diff line change
Expand Up @@ -8,66 +8,66 @@ create table t (m map (float, float));
db error: ERROR: Failed to run the query

Caused by:
invalid map key type: double precision
Bind error: invalid map key type: double precision


query error
select map_from_entries(array[1.0,2.0,3.0], array[1,2,3]);
select map_from_key_values(array[1.0,2.0,3.0], array[1,2,3]);
----
db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: Failed to bind expression: map_from_entries(ARRAY[1.0, 2.0, 3.0], ARRAY[1, 2, 3])
1: Failed to bind expression: map_from_key_values(ARRAY[1.0, 2.0, 3.0], ARRAY[1, 2, 3])
2: Expr error
3: invalid map key type: numeric


query error
select map_from_entries(array[1,1,3], array[1,2,3]);
select map_from_key_values(array[1,1,3], array[1,2,3]);
----
db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: Expr error
2: error while evaluating expression `map_from_entries('{1,1,3}', '{1,2,3}')`
2: error while evaluating expression `map_from_key_values('{1,1,3}', '{1,2,3}')`
3: map keys must be unique


query ?
select map_from_entries(array[1,2,3], array[1,null,3]);
select map_from_key_values(array[1,2,3], array[1,null,3]);
----
{1:1,2:NULL,3:3}


query error
select map_from_entries(array[1,null,3], array[1,2,3]);
select map_from_key_values(array[1,null,3], array[1,2,3]);
----
db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: Expr error
2: error while evaluating expression `map_from_entries('{1,NULL,3}', '{1,2,3}')`
2: error while evaluating expression `map_from_key_values('{1,NULL,3}', '{1,2,3}')`
3: map keys must not be NULL


query error
select map_from_entries(array[1,3], array[1,2,3]);
select map_from_key_values(array[1,3], array[1,2,3]);
----
db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: Expr error
2: error while evaluating expression `map_from_entries('{1,3}', '{1,2,3}')`
2: error while evaluating expression `map_from_key_values('{1,3}', '{1,2,3}')`
3: map keys and values have different length


query error
select map_from_entries(array[1,2], array[1,2]) = map_from_entries(array[2,1], array[2,1]);
select map_from_key_values(array[1,2], array[1,2]) = map_from_key_values(array[2,1], array[2,1]);
----
db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: Failed to bind expression: map_from_entries(ARRAY[1, 2], ARRAY[1, 2]) = map_from_entries(ARRAY[2, 1], ARRAY[2, 1])
1: Failed to bind expression: map_from_key_values(ARRAY[1, 2], ARRAY[1, 2]) = map_from_key_values(ARRAY[2, 1], ARRAY[2, 1])
2: function equal(map(integer,integer), map(integer,integer)) does not exist


Expand All @@ -83,32 +83,32 @@ create table t (

statement ok
insert into t values (
map_from_entries(array['a','b','c'], array[1.0,2.0,3.0]::float[]),
map_from_entries(array[1,2,3], array[true,false,true]),
map_from_entries(array['a','b'],
map_from_key_values(array['a','b','c'], array[1.0,2.0,3.0]::float[]),
map_from_key_values(array[1,2,3], array[true,false,true]),
map_from_key_values(array['a','b'],
array[
map_from_entries(array['a1'], array['a2']),
map_from_entries(array['b1'], array['b2'])
map_from_key_values(array['a1'], array['a2']),
map_from_key_values(array['b1'], array['b2'])
]
),
array[
map_from_entries(array['a','b','c'], array[1,2,3]),
map_from_entries(array['d','e','f'], array[4,5,6])
map_from_key_values(array['a','b','c'], array[1,2,3]),
map_from_key_values(array['d','e','f'], array[4,5,6])
],
row(
map_from_entries(array['a','b','c'], array[row(1),row(2),row(3)]::struct<x int>[])
map_from_key_values(array['a','b','c'], array[row(1),row(2),row(3)]::struct<x int>[])
)
);

# cast(map(character varying,integer)) -> map(character varying,double precision)
query ?
select map_from_entries(array['a','b','c'], array[1,2,3])::map(varchar,float);
select map_from_key_values(array['a','b','c'], array[1,2,3])::map(varchar,float);
----
{a:1,b:2,c:3}


statement ok
insert into t(m1) values (map_from_entries(array['a','b','c'], array[1,2,3]));
insert into t(m1) values (map_from_key_values(array['a','b','c'], array[1,2,3]));

query ????? rowsort
select * from t;
Expand Down Expand Up @@ -144,7 +144,7 @@ db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: Expr error
2: error while evaluating expression `map_from_entries('{a,a}', '{1,2}')`
2: error while evaluating expression `map_from_key_values('{a,a}', '{1,2}')`
3: map keys must be unique


Expand All @@ -165,3 +165,96 @@ select
MAP{1:'a',2:'b'}::MAP(VARCHAR,VARCHAR)
----
{} {1:a,2:b}

query error
select map_from_entries(array[]);
----
db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: Failed to bind expression: map_from_entries(ARRAY[])
2: Bind error: cannot determine type of empty array
HINT: Explicitly cast to the desired type, for example ARRAY[]::integer[].


query error
select map_from_entries(array[]::int[]);
----
db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: Failed to bind expression: map_from_entries(CAST(ARRAY[] AS INT[]))
2: Expr error
3: invalid map entries type, expected struct, got: integer


query error
select map_from_entries(array[]::struct<key float, value int>[]);
----
db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: Failed to bind expression: map_from_entries(CAST(ARRAY[] AS STRUCT<key FLOAT, value INT>[]))
2: Expr error
3: invalid map key type: double precision


query ?
select map_from_entries(array[]::struct<key int, value int>[]);
----
{}


query ?
select map_from_entries(array[row('a',1), row('b',2), row('c',3)]);
----
{a:1,b:2,c:3}


query error
select map_from_entries(array[row('a',1), row('a',2), row('c',3)]);
----
db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: Expr error
2: error while evaluating expression `map_from_entries('{"(a,1)","(a,2)","(c,3)"}')`
3: map keys must be unique


query error
select map_from_entries(array[row('a',1,2)]);
----
db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: Failed to bind expression: map_from_entries(ARRAY[ROW('a', 1, 2)])
2: Expr error
3: the underlying struct for map must have exactly two fields, got: StructType { field_names: [], field_types: [Varchar, Int32, Int32] }


query error
select map_from_entries(array[row(1.0,1)]);
----
db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: Failed to bind expression: map_from_entries(ARRAY[ROW(1.0, 1)])
2: Expr error
3: invalid map key type: numeric


query error
select map_from_entries(null);
----
db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: Failed to bind expression: map_from_entries(NULL)
2: Bind error: Cannot implicitly cast 'null:Varchar' to polymorphic type AnyArray


query ?
select map_from_entries(null::struct<key int, value int>[]);
----
NULL
1 change: 1 addition & 0 deletions proto/expr.proto
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ message ExprNode {
MAP_KEYS = 702;
MAP_VALUES = 703;
MAP_ENTRIES = 704;
MAP_FROM_KEY_VALUES = 705;

// Non-pure functions below (> 1000)
// ------------------------
Expand Down
33 changes: 24 additions & 9 deletions src/common/src/array/map_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl Array for MapArray {

fn data_type(&self) -> DataType {
let list_value_type = self.inner.values().data_type();
DataType::Map(MapType::from_list_entries(list_value_type))
DataType::Map(MapType::from_entries(list_value_type))
}
}

Expand Down Expand Up @@ -193,6 +193,8 @@ pub use scalar::{MapRef, MapValue};
/// We only check the invariants in the constructors.
/// After they are constructed, we assume the invariants holds.
mod scalar {
use std::collections::HashSet;

use super::*;

/// Refer to [`MapArray`] for the invariants of a map value.
Expand Down Expand Up @@ -221,20 +223,33 @@ mod scalar {

/// # Panics
/// Panics if [map invariants](`super::MapArray`) are violated.
pub fn from_list_entries(list: ListValue) -> Self {
pub fn from_entries(entries: ListValue) -> Self {
Self::try_from_entries(entries).unwrap()
}

/// Returns error if [map invariants](`super::MapArray`) are violated.
pub fn try_from_entries(entries: ListValue) -> Result<Self, String> {
// validates list type is valid
_ = MapType::from_list_entries(list.data_type());
// TODO: validate the values is valid
MapValue(list)
let _ = MapType::try_from_entries(entries.data_type())?;
let mut keys = HashSet::with_capacity(entries.len());
let struct_array = entries.into_array();
for key in struct_array.as_struct().field_at(0).iter() {
let Some(key) = key else {
return Err("map keys must not be NULL".to_string());
};
if !keys.insert(key) {
return Err("map keys must be unique".to_string());
}
}
Ok(MapValue(ListValue::new(struct_array)))
}

/// # Panics
/// Panics if [map invariants](`super::MapArray`) are violated.
/// Returns error if [map invariants](`super::MapArray`) are violated.
pub fn try_from_kv(key: ListValue, value: ListValue) -> Result<Self, String> {
if key.len() != value.len() {
return Err("map keys and values have different length".to_string());
}
let unique_keys = key.iter().unique().collect_vec();
let unique_keys: HashSet<_> = key.iter().unique().collect();
if unique_keys.len() != key.len() {
return Err("map keys must be unique".to_string());
}
Expand Down Expand Up @@ -411,7 +426,7 @@ impl MapValue {
deserializer: &mut memcomparable::Deserializer<impl Buf>,
) -> memcomparable::Result<Self> {
let list = ListValue::memcmp_deserialize(&datatype.clone().into_struct(), deserializer)?;
Ok(Self::from_list_entries(list))
Ok(Self::from_entries(list))
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/common/src/test_utils/rand_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl RandValue for ListValue {
impl RandValue for MapValue {
fn rand_value<R: Rng>(_rand: &mut R) -> Self {
// dummy value
MapValue::from_list_entries(ListValue::empty(&DataType::Struct(
MapValue::from_entries(ListValue::empty(&DataType::Struct(
MapType::struct_type_for_map(DataType::Varchar, DataType::Varchar),
)))
}
Expand Down
43 changes: 27 additions & 16 deletions src/common/src/types/map_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,26 +36,37 @@ impl MapType {
Self(Box::new((key, value)))
}

pub fn try_from_kv(key: DataType, value: DataType) -> Result<Self, anyhow::Error> {
pub fn try_from_kv(key: DataType, value: DataType) -> Result<Self, String> {
Self::check_key_type_valid(&key)?;
Ok(Self(Box::new((key, value))))
}

pub fn try_from_entries(list_entries_type: DataType) -> Result<Self, String> {
match list_entries_type {
DataType::Struct(s) => {
let Some((k, v)) = s.iter().collect_tuple() else {
return Err(format!(
"the underlying struct for map must have exactly two fields, got: {s:?}"
));
};
// the field names are not strictly enforced
// Currently this panics for SELECT * FROM t
// if cfg!(debug_assertions) {
// itertools::assert_equal(struct_type.names(), ["key", "value"]);
// }
Self::try_from_kv(k.1.clone(), v.1.clone())
}
_ => Err(format!(
"invalid map entries type, expected struct, got: {list_entries_type}"
)),
}
}

/// # Panics
/// Panics if the key type is not valid for a map, or the
/// entries type is not a valid struct type.
pub fn from_list_entries(list_entries_type: DataType) -> Self {
let struct_type = list_entries_type.as_struct();
let (k, v) = struct_type
.iter()
.collect_tuple()
.expect("the underlying struct for map must have exactly two fields");
// the field names are not strictly enforced
// Currently this panics for SELECT * FROM t
// if cfg!(debug_assertions) {
// itertools::assert_equal(struct_type.names(), ["key", "value"]);
// }
Self::from_kv(k.1.clone(), v.1.clone())
pub fn from_entries(list_entries_type: DataType) -> Self {
Self::try_from_entries(list_entries_type).unwrap()
}

/// # Panics
Expand Down Expand Up @@ -89,7 +100,7 @@ impl MapType {
///
/// Note that this isn't definitive.
/// Just be conservative at the beginning, but not too restrictive (like only allowing strings).
pub fn check_key_type_valid(data_type: &DataType) -> anyhow::Result<()> {
pub fn check_key_type_valid(data_type: &DataType) -> Result<(), String> {
let ok = match data_type {
DataType::Int16 | DataType::Int32 | DataType::Int64 => true,
DataType::Varchar => true,
Expand All @@ -111,7 +122,7 @@ impl MapType {
| DataType::Map(_) => false,
};
if !ok {
Err(anyhow::anyhow!("invalid map key type: {data_type}"))
Err(format!("invalid map key type: {data_type}"))
} else {
Ok(())
}
Expand All @@ -128,7 +139,7 @@ impl FromStr for MapType {
if let Some((key, value)) = s[4..s.len() - 1].split(',').collect_tuple() {
let key = key.parse().context("failed to parse map key type")?;
let value = value.parse().context("failed to parse map value type")?;
MapType::try_from_kv(key, value)
MapType::try_from_kv(key, value).map_err(|e| anyhow::anyhow!(e))
} else {
Err(anyhow::anyhow!("expect map(...,...)"))
}
Expand Down
2 changes: 1 addition & 1 deletion src/common/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ impl From<&PbDataType> for DataType {
// Map is physically the same as a list.
// So the first (and only) item is the list element type.
let list_entries_type: DataType = (&proto.field_type[0]).into();
DataType::Map(MapType::from_list_entries(list_entries_type))
DataType::Map(MapType::from_entries(list_entries_type))
}
PbTypeName::Int256 => DataType::Int256,
}
Expand Down
Loading

0 comments on commit b92834f

Please sign in to comment.