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: Support comparison for ipprefix [3/n] #11546

Closed

Conversation

yuandagits
Copy link
Contributor

Summary:
We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS, TINYINT>. Since IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

Differential Revision: D65521072

@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 15, 2024
@facebook-github-bot
Copy link
Contributor

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

Copy link

netlify bot commented Nov 15, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit c210b19
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/675729a823098800086bbe56

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

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

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

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

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

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

@yuandagits yuandagits changed the title Support comparison for ipprefix [3/n] feat: Support comparison for ipprefix [3/n] Nov 20, 2024
yuandagits added a commit to yuandagits/velox that referenced this pull request Dec 2, 2024
Summary:

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

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

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

@facebook-github-bot
Copy link
Contributor

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

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

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

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

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

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

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

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

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

@yuandagits yuandagits requested a review from Yuhta December 6, 2024 15:21
yuandagits added a commit to yuandagits/velox that referenced this pull request Dec 6, 2024
Summary:

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

Differential Revision: D65521072
yuandagits added a commit to yuandagits/velox that referenced this pull request Dec 6, 2024
Summary:

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

Differential Revision: D65521072
yuandagits added a commit to yuandagits/velox that referenced this pull request Dec 6, 2024
Summary:

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

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

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

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

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

Differential Revision: D65521072
yuandagits added a commit to yuandagits/velox that referenced this pull request Dec 6, 2024
Summary:

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

Differential Revision: D65521072
yuandagits added a commit to yuandagits/velox that referenced this pull request Dec 6, 2024
Summary:

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

Differential Revision: D65521072
auto left = makeArrayOfRowVector(
IPPREFIX(),
{{ipprefix("2001:0db8:0000:0000:0000:ff00:0042:8329/128")},
{ipprefix("1.2.3.4/32")},
Copy link
Contributor

Choose a reason for hiding this comment

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

Presto has this:

> select ipprefix '1.2.3.4/24' = ipprefix '1.2.3.5/24';
 _col0
-------
 true
(1 row)

Can we make sure Velox behaves the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated with the new test case, thanks Jimmy!

Copy link
Contributor

Choose a reason for hiding this comment

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

The result should be true

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed offline, we are masking the extra bits to zeros like Java did when we creating new ipprefix, so we don't need this test case (it's invalid input).

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

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

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

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

@yuandagits yuandagits requested a review from Yuhta December 6, 2024 23:16
yuandagits added a commit to yuandagits/velox that referenced this pull request Dec 8, 2024
Summary:

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

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

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

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

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

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

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

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

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

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

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

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

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

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

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

Summary:

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

Reviewed By: Yuhta

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

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

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

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

Reviewed By: Yuhta

Differential Revision: D65521072
yuandagits added a commit to yuandagits/velox that referenced this pull request Dec 10, 2024
Summary:

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

Reviewed By: Yuhta

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

This pull request has been merged in c12207a.

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

We need to support comparison for IPPrefix, which has underlying type of RowVector<HUGEINT, TINYINT>.

Since the first 16 bytes of IPPrefix is pretty much the same as IPAddress, we can just convert the underlying type of IPPrefix to RowVector<IPADDRESS,  TINYINT>. Since  IPADDRESS already has its custom comparison defined, and rowVector by default compares left to right, this takes of comparison for us for free.

Reviewed By: Yuhta

Differential Revision: D65521072

fbshipit-source-id: 3e21e2fa4ff5d1306dc5dde601168bb659e54303
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.

3 participants