feat(function): Implement map_delete function

[shamb0]

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

Implement map_delete function

• Addresses part of 15295

Tests

• Unit Test
• Logic Test
• Benchmark Test
• No Test - Explain why

Type of change

• Bug Fix (non-breaking change which fixes an issue)
• New Feature (non-breaking change which adds functionality)
• Breaking Change (fix or feature that could cause existing functionality not to work as expected)
• Documentation Update
• Refactoring
• Performance Improvement
• Other (please describe):

---

This change is 

[shamb0]

Hi @b41sh,

• I've completed the first draft of map_delete. Could you please review the code and provide feedback when you get a chance?

• Let me know if there are any areas that need improvement or alternative approaches to consider.

Thanks!

[b41sh]

Hi @b41sh,

• I've completed the first draft of map_delete. Could you please review the code and provide feedback when you get a chance?
• Let me know if there are any areas that need improvement or alternative approaches to consider.

Thanks!

Hi @shamb0 , thanks for continuing to contribute code.
Since the map_delete function has a variable number of arguments and may have more than one key argument, it is better to use Factory function instead of Builtin function. You can refer to these Factory functions as examples.

https://github.com/datafuselabs/databend/blob/main/src/query/functions/src/scalars/string_multi_args.rs#L88
https://github.com/datafuselabs/databend/blob/main/src/query/functions/src/scalars/string_multi_args.rs#L247
https://github.com/datafuselabs/databend/blob/main/src/query/functions/src/scalars/geo.rs#L311

There are many more implementations of the factory function, and you can find more examples by searching the code base for registry.register_function_factory

The factory function is not written in the same way as the builtin function, it can support more flexible argument types, but it needs to deal with more problems, there are mainly the following steps:

First of all, you need to check whether the input arguments are supported types, for the map_delete function, the number of arguments is at least two, the type of the first argument must be map, the type of the second and later arguments must be the same as the type of the map key, and can not be nullable. If the arguments do not meet these conditions, return NULL, otherwise, we can get args_type and return_type, used to generate FunctionSignature.

The second part of writing the logic for executing the function is that the input args are of type ValueRef, and each argument could be a Scalar or a Column, which needs to be handled in a different way. You also need a builder to store the result data, there are many examples in this part, you can refer to the implementation of other functions.

[shamb0]

Hi @b41sh,

• I've completed the first draft of map_delete. Could you please review the code and provide feedback when you get a chance?
• Let me know if there are any areas that need improvement or alternative approaches to consider.

Thanks!

Hi @shamb0 , thanks for continuing to contribute code. Since the map_delete function has a variable number of arguments and may have more than one key argument, it is better to use Factory function instead of Builtin function. You can refer to these Factory functions as examples.

https://github.com/datafuselabs/databend/blob/main/src/query/functions/src/scalars/string_multi_args.rs#L88 https://github.com/datafuselabs/databend/blob/main/src/query/functions/src/scalars/string_multi_args.rs#L247 https://github.com/datafuselabs/databend/blob/main/src/query/functions/src/scalars/geo.rs#L311

There are many more implementations of the factory function, and you can find more examples by searching the code base for registry.register_function_factory

The factory function is not written in the same way as the builtin function, it can support more flexible argument types, but it needs to deal with more problems, there are mainly the following steps:

First of all, you need to check whether the input arguments are supported types, for the map_delete function, the number of arguments is at least two, the type of the first argument must be map, the type of the second and later arguments must be the same as the type of the map key, and can not be nullable. If the arguments do not meet these conditions, return NULL, otherwise, we can get args_type and return_type, used to generate FunctionSignature.

The second part of writing the logic for executing the function is that the input args are of type ValueRef, and each argument could be a Scalar or a Column, which needs to be handled in a different way. You also need a builder to store the result data, there are many examples in this part, you can refer to the implementation of other functions.

Thanks @b41sh, for sharing those details! I'll take a good look at the requirements and circle back with any questions or updates.

[b41sh]

Hi @shamb0 It's been a long time since last update, have you made any progress recently?

[shamb0]

Hi @shamb0 It's been a long time since last update, have you made any progress recently?

Hi @b41sh,

Sorry for the delay. I've been tied up with some personal matters. I have some updates that are partially complete and will aim to make a full commit in the next few days.

[shamb0]

Hi @b41sh,

I've refactored the implementation to use a factory function instead of a built-in function. The proto implementation is ready for your review. Currently, it only supports the key variant of type DataType::String. Since the DataType collection is extensive, could you please suggest which other variants we should support next?

[b41sh]

map_delete function have at least two arguments, we should check the arguments this way

if arg_types.len() < 2 {

[b41sh]

this function can be removed, we can use factory function handle both MapType and EmptyMapType.

[b41sh]

The first argument can be MapType, Nullable MapType or EmptyMapType, we can write it like this:

match args_type[0].remove_nullable() {
    DataType::Map(box DataType::Tuple(type_tuple)) => ..
    DataType::EmptyMap => ..
    _ => {
        return None;
    }
}

[shamb0]

@b41sh Could you clarify how we should handle DataType::EmptyMap? Since there’s nothing to process, can we simply return None?

[b41sh]

other arguments must have the same data type as the key type in MapType, and for EmptyMapType, we should check that other arguments are the same type and is the allowed key type, like string, number, ..

[b41sh]

we can use args_type[0].clone() here.

[b41sh]

The result domain is the same as the domain of the first argument.

[b41sh]

the value can be a scalar or column value, we can get the keys in this way

    let mut keys = HashSet::new();
    for j in 1..args.len() {
        let key = match &args[j] {
            ValueRef::Scalar(scalar) => scalar.clone(),
            ValueRef::Column(col) => unsafe { col.index_unchecked(i) },
        };
        keys.insert(key);
    }

[b41sh]

use this method to get the map value in each row

    let map = match &args[0] {
        ValueRef::Scalar(scalar) => scalar.clone(),
        ValueRef::Column(col) => unsafe { col.index_unchecked(i) },
    };

and the type of map value may be Null, Map or EmptyMap, we should consider to handler each case.

[b41sh]

we can use the return_type to create the builder.

let mut builder = ColumnBuilder::with_capacity(&return_type, len);

return_type is same as args_type[0], we can use args_type here if modify the eval as following:

eval: Box::new(move |args, ctx| {

[b41sh]

Hi @b41sh,

I've refactored the implementation to use a factory function instead of a built-in function. The proto implementation is ready for your review. Currently, it only supports the key variant of type DataType::String. Since the DataType collection is extensive, could you please suggest which other variants we should support next?

Hi @shamb0 Thank you for your modification. There are still some minor problems that need to be modified. You can have a look at them when you are free.

[shamb0]

Hi @b41sh,
I've refactored the implementation to use a factory function instead of a built-in function. The proto implementation is ready for your review. Currently, it only supports the key variant of type DataType::String. Since the DataType collection is extensive, could you please suggest which other variants we should support next?

Hi @shamb0 Thank you for your modification. There are still some minor problems that need to be modified. You can have a look at them when you are free.

Hi @b41sh,

Thank you for the review comments. I'll incorporate the suggested changes and have an updated version ready for your review soon.