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

Function to split an IP prefix into subnets #23515

Closed
wants to merge 1 commit into from
Closed

Function to split an IP prefix into subnets #23515

wants to merge 1 commit into from

Conversation

matt-calder
Copy link
Contributor

@matt-calder matt-calder commented Aug 24, 2024 •
edited
Loading

Description

This change introduces a function ip_prefix_subnets that, given an input ip_prefix and a subnet prefix_length, splits the IP prefix into subnets of the input prefix length. The functions return an array of new prefixes of the new input prefix length.

This is a complimentary sister function to the ip_prefix_collapse function.

Motivation and Context

This change addresses issue 23514.

Impact

This change introduces a new user-facing function ip_prefix_subnets.

Test Plan

Unit tests

mvn clean install -Dtest=TestIpPrefixFunctions -Dmaven.javadoc.skip=true -DskipUI -T1C -fn -pl presto-main

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.
== RELEASE NOTES ==

General Changes
* Add function :func:`ip_prefix_subnets` that splits the input prefix into subnets the size of the new prefix length. ... :pr:`23515`

steveburnett
steveburnett previously approved these changes Aug 26, 2024
View reviewed changes
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)

Pull branch, local doc build, looks good. Thanks!

@matt-calder matt-calder reopened this Aug 30, 2024
@matt-calder matt-calder reopened this Aug 30, 2024
steveburnett
steveburnett previously approved these changes Sep 3, 2024
View reviewed changes
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

(Re-review after close and reopening.)

LGTM! (docs)

Pull branch, local doc build, looks good. Thanks!

steveburnett
steveburnett previously approved these changes Sep 3, 2024
View reviewed changes
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

(rerereview by request)

LGTM! (docs)

Pull branch, local doc build, looks good. Thanks!

@matt-calder matt-calder closed this Sep 4, 2024
@matt-calder matt-calder reopened this Sep 4, 2024
@matt-calder matt-calder closed this Sep 5, 2024
@matt-calder matt-calder reopened this Sep 5, 2024
steveburnett
steveburnett previously approved these changes Sep 6, 2024
View reviewed changes
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)

Pull updated branch, new local doc build, looks good. Thanks!

@matt-calder
initial commit for ip_prefix_subnets function
cf05a91 
tests and docs

reviewer feedback 2

improved documentation

converting comment styles

feedback

changing return type when prefix_length arg is less specific that input prefix

reviewer feedback

removing whitespace
@matt-calder
Copy link
Contributor Author

I was unable to squash and merge this correctly so this branch is FUBAR. Abandoning for new PR.

@matt-calder matt-calder mentioned this pull request Sep 15, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steveburnett steveburnett steveburnett left review comments

@presto-oss presto-oss Awaiting requested review from presto-oss presto-oss is a code owner automatically assigned from prestodb/committers

@elharo elharo Awaiting requested review from elharo elharo is a code owner

@yingsu00 yingsu00 Awaiting requested review from yingsu00

@kaikalur kaikalur Awaiting requested review from kaikalur

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@matt-calder @elharo @kaikalur @steveburnett @yingsu00