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

Add in support for explode on maps #3175

Merged
merged 4 commits into from
Aug 10, 2021
Merged

Conversation

revans2
Copy link
Collaborator

@revans2 revans2 commented Aug 9, 2021

This fixes #3110

revans2 added 2 commits August 9, 2021 14:21
Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
@revans2 revans2 added this to the Aug 2 - Aug 13 milestone Aug 9, 2021
@revans2 revans2 self-assigned this Aug 9, 2021
@revans2
Copy link
Collaborator Author

revans2 commented Aug 9, 2021

build

jlowe
jlowe previously approved these changes Aug 9, 2021
Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nits, lgtm.

@@ -695,7 +695,10 @@ static boolean typeConversionAllowed(ColumnView cv, DataType colType) {

static boolean typeConversionAllowed(Table table, DataType[] colTypes, int startCol, int endCol) {
final int numColumns = endCol - startCol;
assert numColumns == colTypes.length: "The number of columns and the number of types don't match";
assert numColumns == colTypes.length: "The number of columns and the number of types don't " +
"match. expected " + colTypes.length + " but found " + numColumns + ". (" + table +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"match. expected " + colTypes.length + " but found " + numColumns + ". (" + table +
"match. Expected " + colTypes.length + " but found " + numColumns + ". (" + table +

// SOME INPUT COLUMNS | POS COLUMN? | STRUCT(KEY, VALUE) | MORE INPUT COLUMNS
// scalastyle:on line.size.limit
val structPos = genOffset + numPos
withResource (ArrayBuffer.empty[ColumnVector] ) { newColumns =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: some odd extra whitespace here.

Suggested change
withResource (ArrayBuffer.empty[ColumnVector] ) { newColumns =>
withResource(ArrayBuffer.empty[ColumnVector]) { newColumns =>

@revans2
Copy link
Collaborator Author

revans2 commented Aug 10, 2021

build

@jlowe jlowe changed the title Add in support for expolde on maps Add in support for explode on maps Aug 10, 2021
@revans2 revans2 merged commit 92e0ecf into NVIDIA:branch-21.10 Aug 10, 2021
@revans2 revans2 deleted the explode_maps branch August 10, 2021 14:32
@sameerz sameerz added the feature request New feature or request label Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Support Map as input to explode and pos_explode
3 participants