Skip to content
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.

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

[C#] Implement MapType #35243

Closed
Platob opened this issue Apr 20, 2023 · 1 comment · Fixed by #37885
Closed

[C#] Implement MapType #35243

Platob opened this issue Apr 20, 2023 · 1 comment · Fixed by #37885

Comments

@Platob
Copy link
Contributor

Platob commented Apr 20, 2023

Describe the enhancement requested

Hello,

i started dev on new implementation of MapType, im wondering how it should be built ?

I think is a list of Struct which are like KeyValuePair c# class

using System.Collections.Generic;

namespace Apache.Arrow.Types
{
    public class MapType : NestedType
    {
        public override ArrowTypeId TypeId => ArrowTypeId.Map;
        public override string Name => "map";

        public Field KeyField => Fields[0];
        public Field ValueField => Fields[0];

        public IArrowType ValueDataType => Fields[0].DataType;

        public MapType(IArrowType keyType, IArrowType valueType, bool nullable)
            : this(new Field("key", keyType, false), new Field("value", keyType, false), nullable)
        {
        }

        public MapType(Field keyField, Field valueField, bool nullable)
            : this(new StructType(new List<Field>() { keyField, valueField }), nullable)
        {
        }

        public MapType(StructType structType, bool nullable) : this(new ListType(new Field("key_value", structType, nullable)), nullable)
        {
        }

        public MapType(ListType listType, bool nullable) : this(new Field("key_values", listType, nullable))
        {
        }

        public MapType(Field keyValueField) : base(keyValueField)
        {
        }

        public override void Accept(IArrowTypeVisitor visitor) => Accept(this, visitor);
    }
}

Component(s)

C#

@westonpace
Copy link
Member

westonpace commented Apr 20, 2023

I think is a list of Struct which are like KeyValuePair c# class

Yes, sounds like you have it correct. Here is the comment from schema.fbs:

/// A Map is a logical nested type that is represented as
///
/// List<entries: Struct<key: K, value: V>>
///
/// In this layout, the keys and values are each respectively contiguous. We do
/// not constrain the key and value types, so the application is responsible
/// for ensuring that the keys are hashable and unique. Whether the keys are sorted
/// may be set in the metadata for this field.
///
/// In a field with Map type, the field has a child Struct field, which then
/// has two children: key type and the second the value type. The names of the
/// child fields may be respectively "entries", "key", and "value", but this is
/// not enforced.
///
/// Map
///
/// - child[0] entries: Struct
/// -- child[0] key: K
/// -- child[1] value: V
///
/// Neither the "entries" field nor the "key" field may be nullable.
///
/// The metadata is structured so that Arrow systems without special handling
/// for Map can make Map an alias for List. The "layout" attribute for the Map
/// field must have the same contents as a List.

lidavidm pushed a commit that referenced this issue Oct 5, 2023
### What changes are included in this PR?

This change includes the original work by `@ Platob` from #35263, updated with the latest changes in the main branch. Additionally, it includes support in the C API for the Map type and fills a few gaps in functionality from the original change (for instance, ArrowArrayConcentrator support). Finally, it fixes a bug in the ArrowArrayConcentrator.Concatenate which prevented it from working correctly for structs. (This bug was discovered in the course of creating a test for concatenating maps.)

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Maps are now supported in the C# implementation.

Closes #35243 
* Closes: #35243

Lead-authored-by: Platob <nfillot.pro@gmail.com>
Co-authored-by: Curt Hagenlocher <curt@hagenlocher.org>
Signed-off-by: David Li <li.davidm96@gmail.com>
@lidavidm lidavidm added this to the 14.0.0 milestone Oct 5, 2023
JerAguilon pushed a commit to JerAguilon/arrow that referenced this issue Oct 23, 2023
### What changes are included in this PR?

This change includes the original work by `@ Platob` from apache#35263, updated with the latest changes in the main branch. Additionally, it includes support in the C API for the Map type and fills a few gaps in functionality from the original change (for instance, ArrowArrayConcentrator support). Finally, it fixes a bug in the ArrowArrayConcentrator.Concatenate which prevented it from working correctly for structs. (This bug was discovered in the course of creating a test for concatenating maps.)

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Maps are now supported in the C# implementation.

Closes apache#35243 
* Closes: apache#35243

Lead-authored-by: Platob <nfillot.pro@gmail.com>
Co-authored-by: Curt Hagenlocher <curt@hagenlocher.org>
Signed-off-by: David Li <li.davidm96@gmail.com>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
### What changes are included in this PR?

This change includes the original work by `@ Platob` from apache#35263, updated with the latest changes in the main branch. Additionally, it includes support in the C API for the Map type and fills a few gaps in functionality from the original change (for instance, ArrowArrayConcentrator support). Finally, it fixes a bug in the ArrowArrayConcentrator.Concatenate which prevented it from working correctly for structs. (This bug was discovered in the course of creating a test for concatenating maps.)

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Maps are now supported in the C# implementation.

Closes apache#35243 
* Closes: apache#35243

Lead-authored-by: Platob <nfillot.pro@gmail.com>
Co-authored-by: Curt Hagenlocher <curt@hagenlocher.org>
Signed-off-by: David Li <li.davidm96@gmail.com>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
### What changes are included in this PR?

This change includes the original work by `@ Platob` from apache#35263, updated with the latest changes in the main branch. Additionally, it includes support in the C API for the Map type and fills a few gaps in functionality from the original change (for instance, ArrowArrayConcentrator support). Finally, it fixes a bug in the ArrowArrayConcentrator.Concatenate which prevented it from working correctly for structs. (This bug was discovered in the course of creating a test for concatenating maps.)

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Maps are now supported in the C# implementation.

Closes apache#35243 
* Closes: apache#35243

Lead-authored-by: Platob <nfillot.pro@gmail.com>
Co-authored-by: Curt Hagenlocher <curt@hagenlocher.org>
Signed-off-by: David Li <li.davidm96@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants