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

Support Map as a ScalarValue #11128

Closed
Tracked by #11429
Blizzara opened this issue Jun 26, 2024 · 4 comments · Fixed by #11224
Closed
Tracked by #11429

Support Map as a ScalarValue #11128

Blizzara opened this issue Jun 26, 2024 · 4 comments · Fixed by #11224
Assignees
Labels
enhancement New feature or request

Comments

@Blizzara
Copy link
Contributor

Is your feature request related to a problem or challenge?

ScalarValue does not currently support Maps, see e.g. #8262 (comment).

#6485 is probably related as well, though I'm not 100% sure

I ran into this when looking to implement Substrait -> DF conversion for MapType

Describe the solution you'd like

Implement ScalarValue::Map and all the related functionality

Describe alternatives you've considered

No response

Additional context

No response

@Blizzara Blizzara added the enhancement New feature or request label Jun 26, 2024
Blizzara added a commit to Blizzara/datafusion that referenced this issue Jun 26, 2024
This only supports Map _type_, not Map literals,
since Map isn't yet implemented in ScalarValue
(apache#11128)
@goldmedal
Copy link
Contributor

take

@goldmedal
Copy link
Contributor

I'm interested in this issue.
As @alamb said in #8262 (comment) :

l I think ScalarValue::Map can follow the model of ScalarValue::List (as Maps are just lists of structs, as I undertand):

I think I can used a list of structs with the same type struct to implemented a basic version like:

List<Struct<K, V>>

Then, provide the HashMap (or others) implementation for the physical plan.
Does it make sense?

@alamb
Copy link
Contributor

alamb commented Jun 27, 2024

I think ScalarValue::Map should follow the same pattern as ScalarValue::List ScalarValue::Struct

List(Arc<ListArray>),

So that would mean a single element MapArray: https://docs.rs/arrow/latest/arrow/array/struct.MapArray.html

enum ScalarValue {
  ...
  Map(Arc<MapArray>)
  ...
}

Does that make sense?

@goldmedal
Copy link
Contributor

enum ScalarValue {
  ...
  Map(Arc<MapArray>)
  ...
}

Does that make sense?

Many thanks! It looks good to me. I didn't notice this struct. I will try to follow this pattern to implement this.

alamb pushed a commit that referenced this issue Jun 27, 2024
* feat: Support Map type in Substrait conversions

This only supports Map _type_, not Map literals,
since Map isn't yet implemented in ScalarValue
(#11128)

* fix clippy
findepi pushed a commit to findepi/datafusion that referenced this issue Jul 16, 2024
* feat: Support Map type in Substrait conversions

This only supports Map _type_, not Map literals,
since Map isn't yet implemented in ScalarValue
(apache#11128)

* fix clippy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants