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

feat: Add ipprefix cast operators for ipaddr [2/n] #11481

Closed

Conversation

yuandagits
Copy link
Contributor

Summary:
Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Differential Revision: D65641994

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 8, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65641994

Copy link

netlify bot commented Nov 8, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 6edf852
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/6751fbb18f31620008ea7e3d

yuandagits added a commit to yuandagits/velox that referenced this pull request Nov 8, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Differential Revision: D65641994
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65641994

yuandagits added a commit to yuandagits/velox that referenced this pull request Nov 8, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Differential Revision: D65641994
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65641994

Copy link
Contributor

@mohsaka mohsaka left a comment

Choose a reason for hiding this comment

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

You can take a look at my updated PRs with addressed comments.

https://github.com/facebookincubator/velox/pull/11309/files
https://github.com/facebookincubator/velox/pull/11378/files

Also here is the functions one if you are doing that too
#11407

Will be going on vacation so feel free to get yours merged!👍

velox/functions/prestosql/types/IPPrefixType.cpp Outdated Show resolved Hide resolved
velox/functions/prestosql/types/IPPrefixType.cpp Outdated Show resolved Hide resolved
velox/functions/prestosql/types/IPPrefixType.cpp Outdated Show resolved Hide resolved
@yuandagits
Copy link
Contributor Author

You can take a look at my updated PRs with addressed comments.

https://github.com/facebookincubator/velox/pull/11309/files https://github.com/facebookincubator/velox/pull/11378/files

Also here is the functions one if you are doing that too #11407

Will be going on vacation so feel free to get yours merged!👍

thanks! I wasn't aware of your PRs 😅, I will take a look at your pointers

yuandagits added a commit to yuandagits/velox that referenced this pull request Nov 8, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65641994

@yuandagits yuandagits requested a review from mohsaka November 11, 2024 14:55
yuandagits added a commit to yuandagits/velox that referenced this pull request Nov 12, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
yuandagits added a commit to yuandagits/velox that referenced this pull request Nov 12, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
yuandagits added a commit to yuandagits/velox that referenced this pull request Nov 12, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
yuandagits added a commit to yuandagits/velox that referenced this pull request Nov 12, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
yuandagits added a commit to yuandagits/velox that referenced this pull request Nov 12, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
yuandagits added a commit to yuandagits/velox that referenced this pull request Nov 12, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65641994

yuandagits added a commit to yuandagits/velox that referenced this pull request Nov 12, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65641994

yuandagits added a commit to yuandagits/velox that referenced this pull request Nov 12, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65641994

yuandagits added a commit to yuandagits/velox that referenced this pull request Dec 3, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65641994

Copy link
Contributor

@mohsaka mohsaka left a comment

Choose a reason for hiding this comment

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

LGTM!

yuandagits added a commit to yuandagits/velox that referenced this pull request Dec 4, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65641994

yuandagits added a commit to yuandagits/velox that referenced this pull request Dec 5, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65641994

@yuandagits yuandagits requested a review from Yuhta December 5, 2024 00:38
yuandagits added a commit to yuandagits/velox that referenced this pull request Dec 5, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65641994

yuandagits added a commit to yuandagits/velox that referenced this pull request Dec 5, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65641994

yuandagits added a commit to yuandagits/velox that referenced this pull request Dec 5, 2024
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Differential Revision: D65641994
Summary:

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Reviewed By: Yuhta

Differential Revision: D65641994
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65641994

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 7205403.

athmaja-n pushed a commit to athmaja-n/velox that referenced this pull request Jan 10, 2025
Summary:
Pull Request resolved: facebookincubator#11481

Support casting ipprefix -> ipaddress, and ipaddress -> ipprefix.

We just need one new helper function, tryIpPrefixLengthFromIPAddressType, which takes in int128_t, gets the ippaddress, and then we check if the ippaddress is ipv4 or ipv6 mapped. We return 32 for ipv4 and 128 for ipv6 mapped.

Also applies suggestion from https://github.com/facebookincubator/velox/pull/11309/files

Reviewed By: Yuhta

Differential Revision: D65641994

fbshipit-source-id: aa43e1667affd73f263fedb33a101bc6aff7dea7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants