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.

Sign up for GitHub

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

Jump to bottom

Fix map column can not be non-nullable for java #14003

Merged
merged 9 commits into from
Sep 6, 2023
Merged

Fix map column can not be non-nullable for java #14003

merged 9 commits into from
Sep 6, 2023

Conversation

res-life
Copy link
Contributor

@res-life res-life commented Aug 30, 2023 •
edited
Loading

Description

Make map column non-nullable for java.

Changes:

  • Add a new method to pass nullable; Deprecate the old one.
  • Update the tests.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
    Update existing tests
  • The documentation is up to date with these changes.
    Bug fix, no need to update document.

Make map column nullable
20c1090 
Signed-off-by: Chong Gao <[email protected]>
@res-life res-life added the Java Affects Java cuDF API. label Aug 30, 2023
@res-life res-life requested a review from a team as a code owner August 30, 2023 08:42
@copy-pr-bot
Copy link

copy-pr-bot bot commented Aug 30, 2023

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@res-life res-life added bug Something isn't working 4 - Needs cuDF (Java) Reviewer non-breaking Non-breaking change labels Aug 30, 2023
@res-life res-life changed the title Fix map column can not be nullable for java Fix map column can not be non-nullable for java Aug 30, 2023
revans2
revans2 reviewed Aug 30, 2023
View reviewed changes
Copy link
Contributor

@revans2 revans2 left a comment

Choose a reason for hiding this comment

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

Technically this is not a breaking change for CUDF, but because the plugin code fails if it is using a deprecated API it will break the spark plugin build. Could you please have a PR ready to go in the Spark plugin so that the time we are broken is very short.

java/src/main/java/ai/rapids/cudf/ColumnWriterOptions.java Show resolved Hide resolved
@sameerz sameerz mentioned this pull request Aug 30, 2023
Closed
@res-life res-life mentioned this pull request Aug 31, 2023
Merged
@res-life
Copy link
Contributor Author

res-life commented Aug 31, 2023 •
edited
Loading

Could you please have a PR ready to go in the Spark plugin so that the time we are broken is very short.

Corresponding PR: NVIDIA/spark-rapids#9147

revans2
revans2 approved these changes Aug 31, 2023
View reviewed changes
Copy link
Contributor

@revans2 revans2 left a comment

Choose a reason for hiding this comment

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

I would like to see the changes the Spark plugin PR approved before this is merged in.

@res-life
Copy link
Contributor Author

res-life commented Sep 4, 2023

ok to test

@res-life
Copy link
Contributor Author

res-life commented Sep 4, 2023

build

@res-life
Copy link
Contributor Author

res-life commented Sep 4, 2023

ok to test

@res-life
Copy link
Contributor Author

res-life commented Sep 4, 2023

rerurn tests

@firestarman
Copy link
Contributor

/ok to test

@revans2
Copy link
Contributor

revans2 commented Sep 5, 2023

/ok to test

@res-life
Copy link
Contributor Author

res-life commented Sep 6, 2023

@gpucibot merge

@rapids-bot
Copy link

rapids-bot bot commented Sep 6, 2023

@res-life, the @gpucibot merge command has been replaced with /merge.

Please re-comment this PR with /merge and use this new command in the future.

@res-life
Copy link
Contributor Author

res-life commented Sep 6, 2023

/merge

@rapids-bot rapids-bot bot merged commit c82a708 into rapidsai:branch-23.10 Sep 6, 2023
@res-life res-life deleted the fix-map-nullable branch September 6, 2023 02:57
@vyasr vyasr added 4 - Needs Review Waiting for reviewer to review or respond and removed 4 - Needs cuDF (Java) Reviewer labels Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ttnghia ttnghia ttnghia left review comments

@revans2 revans2 revans2 approved these changes

Assignees
No one assigned
Labels
4 - Needs Review Waiting for reviewer to review or respond bug Something isn't working Java Affects Java cuDF API. non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@res-life @firestarman @revans2 @ttnghia @vyasr