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

[#1384] feat(api): Add NullType for representing NULL values #1388

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

mchades
Copy link
Contributor

@mchades mchades commented Jan 8, 2024

What changes were proposed in this pull request?

Add NullType for representing NULL values

Why are the changes needed?

Fix: #1384

Does this PR introduce any user-facing change?

yes, users now can use null literal

How was this patch tested?

UTs added

@mchades mchades self-assigned this Jan 8, 2024
@mchades mchades force-pushed the literals-3 branch 2 times, most recently from d3a56fa to 429a4ee Compare January 9, 2024 12:36
@mchades mchades changed the title [#1384] Add NullType for representing NULL values #1384 [#1384] feat(api): Add NullType for representing NULL values #1384 Jan 9, 2024
@mchades mchades requested a review from jerryshao January 9, 2024 12:37
@mchades mchades marked this pull request as ready for review January 9, 2024 12:38
@mchades mchades requested a review from yuqi1129 January 9, 2024 12:43
@mchades mchades changed the title [#1384] feat(api): Add NullType for representing NULL values #1384 [#1384] feat(api): Add NullType for representing NULL values Jan 9, 2024
@@ -13,6 +13,8 @@

/** The helper class to create literals to pass into Gravitino. */
public class Literals {
/** Used to represent a null literal. */
public static final Literal<Types.NullType> NULL = new LiteralImpl<>(null, Types.NullType.get());
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it a typical way to use NullType for Null value?

Copy link
Contributor Author

@mchades mchades Jan 10, 2024

Choose a reason for hiding this comment

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

I think so. There are some refer:

Copy link
Contributor

@jerryshao jerryshao left a comment

Choose a reason for hiding this comment

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

LGTM.

@jerryshao jerryshao merged commit 3551ada into apache:main Jan 10, 2024
11 checks passed
@mchades mchades deleted the literals-3 branch November 22, 2024 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Subtask] Add NullType for representing NULL values
2 participants