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

Improve the ergonomics of creating field and list array accesses #7193

Closed
alamb opened this issue Aug 4, 2023 · 2 comments · Fixed by #7215
Closed

Improve the ergonomics of creating field and list array accesses #7193

alamb opened this issue Aug 4, 2023 · 2 comments · Fixed by #7215
Assignees
Labels
enhancement New feature or request

Comments

@alamb
Copy link
Contributor

alamb commented Aug 4, 2023

Is your feature request related to a problem or challenge?

#6936 from @izveigor added a really nice set of array functionality 🏆 to access individual array elements as well as ranges!

However when I was testing the a bit of a mismatch in the GetFieldAccess struct which is now overloaded to return a named field in a struct, an element in a list or a range of a list.

More commentary here #6936 (comment)

You can see for example, the code to manipulate it here is pretty tricky: https://github.com/influxdata/influxdb_iox/pull/8419/files#diff-2ea572c49a95b9cbfa0647dbb38795bff1aa4dc788518bc8e83f80814f061b1f

Describe the solution you'd like

I think it would be a clearer API to separate out the FieldAccess for struct fields and ListAccess for list element acess

Perhaps something like

enum GetFieldAccess {
  /// returns the field `struct[field]`. For example `struct["name"]`
  NamedStructField { 
    name: ScalarValue,
  },
  /// single list index
  // list[i]
  ListIndex: {
    key: Expr
  },
  /// list range `list[i:j]`
  ListRange: {
    start: Expr
    stop: Expr
  },
}

Describe alternatives you've considered

No response

Additional context

No response

@alamb alamb added the enhancement New feature or request label Aug 4, 2023
@alamb alamb self-assigned this Aug 4, 2023
@izveigor
Copy link
Contributor

izveigor commented Aug 4, 2023

@alamb The implementation via enum is actually a great idea! Can I take this ticket if no one minds?
And also I think we can improve our spaghetti-code in array_expressions. It maybe we can design some patterns to describe all actions with arrays 🤔. What do you think about it, @alamb?

@alamb
Copy link
Contributor Author

alamb commented Aug 4, 2023

@izveigor that would be great if you are willing 🙏

I am somewhat interested in getting this in before we release DataFusion 29 (probably at least a week away) so we don't release the current structures and thus have two breaking changes

Thus if you have time to work on it soon this would be great. Otherwise I will be happy to do this work as well

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.

2 participants