You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "roeap (via GitHub)" <gi...@apache.org> on 2023/05/28 07:25:46 UTC
[GitHub] [arrow-rs] roeap opened a new pull request, #4300: Sql infos list map
roeap opened a new pull request, #4300:
URL: https://github.com/apache/arrow-rs/pull/4300
Draft as it dempends on #4293 by @alamb, which contains tests useful to validate this implementation.
# Which issue does this PR close?
Closes #.
# Rationale for this change
Thus far the SqlInfo helpers in arrow-flight did not yet support complex types defined in the specs in the form of `Map<i32, List<i32>>`. This PR adds support for these types.
# What changes are included in this PR?
* support `int32_to_int32_list_map` type in SqlInfo dense union
# Are there any user-facing changes?
Users can now specify SqlInfos with the Map of i32 Lists type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] roeap commented on a diff in pull request #4300: feat(flight): support int32_to_int32_list_map in sql infos
Posted by "roeap (via GitHub)" <gi...@apache.org>.
roeap commented on code in PR #4300:
URL: https://github.com/apache/arrow-rs/pull/4300#discussion_r1210215229
##########
arrow-flight/src/sql/sql_info.rs:
##########
@@ -280,6 +348,22 @@ impl SqlInfoUnionBuilder {
/// let batch = info_list.encode().unwrap();
/// ```
///
+/// # Example
Review Comment:
Yes, I did a rebase, and this is likely an artefact of that! Sorry for missing that, will fix :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb merged pull request #4300: feat(flight): support int32_to_int32_list_map in sql infos
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #4300:
URL: https://github.com/apache/arrow-rs/pull/4300
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] roeap commented on a diff in pull request #4300: feat(flight): support int32_to_int32_list_map in sql infos
Posted by "roeap (via GitHub)" <gi...@apache.org>.
roeap commented on code in PR #4300:
URL: https://github.com/apache/arrow-rs/pull/4300#discussion_r1210239175
##########
arrow-flight/src/sql/sql_info.rs:
##########
@@ -280,6 +348,22 @@ impl SqlInfoUnionBuilder {
/// let batch = info_list.encode().unwrap();
/// ```
///
+/// # Example
Review Comment:
albeit repetition sometimes being helpful to learn things, went back to a single instance of the example 🤣.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on a diff in pull request #4300: feat(flight): support int32_to_int32_list_map in sql infos
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #4300:
URL: https://github.com/apache/arrow-rs/pull/4300#discussion_r1210251380
##########
arrow-flight/src/sql/sql_info.rs:
##########
@@ -280,6 +348,22 @@ impl SqlInfoUnionBuilder {
/// let batch = info_list.encode().unwrap();
/// ```
///
+/// # Example
Review Comment:
😆 👏
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on pull request #4300: feat(flight): support int32_to_int32_list_map in sql infos
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #4300:
URL: https://github.com/apache/arrow-rs/pull/4300#issuecomment-1566109344
Thank you @roeap -- this is amazing!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on a diff in pull request #4300: feat(flight): support int32_to_int32_list_map in sql infos
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #4300:
URL: https://github.com/apache/arrow-rs/pull/4300#discussion_r1210208735
##########
arrow-flight/src/sql/sql_info.rs:
##########
@@ -280,6 +348,22 @@ impl SqlInfoUnionBuilder {
/// let batch = info_list.encode().unwrap();
/// ```
///
+/// # Example
Review Comment:
This seems to be identical to the example above. Is that intended? -- I wonder if it got added accidentally during merge 🤔
##########
arrow-flight/src/sql/sql_info.rs:
##########
@@ -386,6 +474,15 @@ mod tests {
#[test]
fn test_sql_infos() {
+ let mut convert: HashMap<i32, Vec<i32>> = HashMap::new();
Review Comment:
👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org