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

[MINOR] Use Literal constants #638

Closed

Conversation

jaceklaskowski
Copy link
Contributor

That should lower memory requirements (as no extra objects are created) and improve readability

Signed-off-by: Jacek Laskowski jacek@japila.pl

That should lower memory requirements (as no extra objects are created) and improve readability

Signed-off-by: Jacek Laskowski <jacek@japila.pl>
Comment on lines 580 to 587
Literal(null)
} else {
Literal.create(unescapePathName(raw), StringType)
}
}
} else {
if (raw == DEFAULT_PARTITION_NAME) {
Literal.create(null, NullType)
Literal(null)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Maybe it's better to keep the create() call in these two places? It makes it more clear that converting to catalyst types are needed here, although for null it's no-op

@jaceklaskowski jaceklaskowski changed the title Use Literal constants [MINOR] Use Literal constants Apr 4, 2021
Copy link
Contributor

@mengtong-db mengtong-db left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! LGTM.

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.

2 participants