implement the Map type

[petar-dambovaliev]

Please, review and i can make changes, if needed.

[suharev7]

Thank you so much, great work. But I noticed a few issues:

• According to the documentation the keys for maps in clickhouse can be String, Integer, or FixedString, but you added a hash implementation only for String. It could panic when someone tries to use mappings with keys other than String.

• You didn't implement the iter method to get an iterator for the whole column. (If you meet some difficulties with it, send PR anyway, I'll implement it).

• And did you check if this implementation works with Nullable(Map(?, ?)) and Array(Map(?, ?))?.

[petar-dambovaliev]

Thanks for the review. I will work on those points. But i have a question.

Thank you so much, great work. But I noticed a few issues:

• According to the documentation the keys for maps in clickhouse can be String, Integer, or FixedString, but you added a hash implementation only for String. It could panic when someone tries to use mappings with keys other than String.

You are talking about the Value and ValueRef implementations of Hash, right? At that point, the FixedString type is Value::String, from what i can read the code. So the ones that need to be added are the integer types.

• You didn't implement the iter method to get an iterator for the whole column. (If you meet some difficulties with it, send PR anyway, I'll implement it).
• And did you check if this implementation works with Nullable(Map(?, ?)) and Array(Map(?, ?))?.

[petar-dambovaliev]

@suharev7 i've added the hash implementations and implemented iter. Can you please check if the implementation is ok? I don't have much experience with unsafe.

[suharev7]

It's incorrect implementation. Tests below illustrate this.

#[test]
fn test_double_iteration() {
    let value = Value::from(HashMap::from([(1_u8, 2_u8)]));

    let expected = vec![HashMap::from([(&1_u8, &2_u8)])];

    let mut block = Block::<Simple>::new();
    block.push(row! {vals: value}).unwrap();

    for _ in 0..2 {
        let actual = block
            .get_column("vals")
            .unwrap()
            .iter::<HashMap<u8, u8>>()
            .unwrap()
            .collect::<Vec<_>>();

        assert_eq!(&actual, &expected);
    }
}

#[test]
fn test_iteration_included_map() {
    let value = Value::from(HashMap::from([
        (10_u16, HashMap::from([(100_u16, 3000_u16)])),
        (20_u16, HashMap::from([(800_u16, 5000_u16), (900, 6000)])),
    ]));

    let mut block = Block::<Simple>::new();
    block.push(row! {vals: value}).unwrap();

    let actual = block
        .get_column("vals")
        .unwrap()
        .iter::<HashMap<u16, HashMap<u16, u16>>>()
        .unwrap()
        .collect::<Vec<_>>();

    let expected = vec![HashMap::from([
        (&10_u16, HashMap::from([(&100_u16, &3000_u16)])),
        (
            &20_u16,
            HashMap::from([(&800_u16, &5000_u16), (&900, &6000)]),
        ),
    ])];

    assert_eq!(actual, expected);
}

I added fixes by myself. Anyway, thank you. That was a lot of work.

[petar-dambovaliev]

@suharev7 can i ask what this is doing? I saw it in your fixes but i do not understand it.

1 | (((props >> 1) - 1) << 1),

[suharev7]

The least bit of the props parameter stores an indication that the information about key should be extracted. The remaining bits of the props parameter store the embedded depth of the map from which needed to extract the information.

1 | (((props >> 1) - 1) << 1),

This line extracts this embedded depth, reduces it by one, and then adds a flag.

[caibirdme]

I want to add a Map(String, String) column

let mut b = Block::new();
let m: HashMap<String, String> = gen_random_map();
b.add_column("xxx", vec![m.clone(), m.clone(), m.clone()]);

But failed. I digged into the code and found this requirements

impl<V> ColumnFrom for Vec<HashMap<String, V>>
where
    V: Copy
        + From<Value>
        + HasSqlType
        + Marshal
        + StatBuffer
        + Unmarshal<V>
        + Sync
        + Send
        + 'static,
    Value: From<V>,

V must impl StatBuffer Marshal Unmarshal , and only i8~i64 u8~u64 impl these traits, which means we can only add Map(String, integer)

---

Don't know why there're so strict requirements, only very few places are rely on them