feat: new inet functions #12575
feat: new inet functions #12575
Conversation
There was a problem hiding this comment.
hey @panga, thanks a lot for opening this PR! Everything looks good.
Could you please include some non-happy path test cases like more complex scenarios or failure cases?
To ensure comprehensive testing coverage and prevent issues from future changes, it'd be nice to cover the following scenarios across all the new functions such as:
- subnet mask scenarios:
- a single host mask (
/32for IPv4,/128for IPv6) - a subnet mask covering all addresses (
/0for both IPv4 and IPv6) - invalid mask values (e.g.
NULLinput, or bigger number than 32)
- a single host mask (
- some edge cases:
192.168.1.255/24or2001:0db8::/960.0.0.0/19or::42
- invalid address input values or invalid mask lengths
999.999.999.999/24, the existence of characters orNULLvalues
To test unexpected/incorrect usage, you can use:
statement error
<statement>
----
<error_msg>
| SELECT netmask(INET '192.168.1.5/16') | ||
| ---- | ||
| 255.255.0.0/16 | ||
|
|
There was a problem hiding this comment.
GenericExecutor::ExecuteUnary applies a unary operation to each element in args.data[0]. Please also include a statement to test this but for many inputs.
Something like:
SELECT * FROM ( VALUES (netmask('10.10.10.1/12')), (broadcast('10.10.10.2/8')), (network('10.10.10.3')));
There was a problem hiding this comment.
You can also use the above if you want to group some of the test scenarios I mentioned before, or you can create a table with different inet values and then test the functionality on them (it'd also be nice to have a test on tables).
e.g.
CREATE TABLE ips(ip INET);
INSERT INTO ips VALUES('0.255.0.1/24'::INET), ('192.168.32.2/12'::INET), ('10.10.10.3/4'::INET);
SELECT netmask(ip) FROM ips;
There was a problem hiding this comment.
The requested new test cases were added.
| @@ -30,6 +30,39 @@ SELECT family('::ffff:127.0.0.1/17') | |||
| ---- | |||
| 6 | |||
|
|
|||
There was a problem hiding this comment.
Could you please also include the above scenarios for the IPv6?
| query IIII | ||
| SELECT tbl.id, tbl2.id, i, j FROM tbl JOIN tbl2 ON (i=j) ORDER BY tbl.id | ||
| ---- | ||
| 1 3 127.0.0.1 127.0.0.1 | ||
| 5 3 127.0.0.1 127.0.0.1 | ||
|
|
||
| # subset |
There was a problem hiding this comment.
Could you please include some test cases that you check (for both operators):
- equal subnets
- one subnet which fully contains another
- partial overlap
- different prefix lengths, but the same network address
- edge case:
0.0.0.0/0>>=192.168.1.0/0
| 5 3 ::1 ::1 | ||
|
|
||
| # subset |
There was a problem hiding this comment.
To ensure thorough testing, please also include here similar testing as in the ipv4 test.
chrisiou
added
the
Needs Documentation
|
@chrisiou thank you so much for the review. I've added the requested test cases. |
|
Thanks! |
Merge pull request duckdb/duckdb#12575 from panga/inet_ops
Currently, INET extension is missing functions for INET range operations, related issue #10947.
However, this PR aims to add the subnet contains operations and other helper functions.
BUILD_INETinMakefile.netmaskComputes the network mask for the address's network.networkReturns the network part of the address, zeroing out whatever is to the right of the netmask.broadcastComputes the broadcast address for the address's network.<<=Is subnet contained by or equal to subnet?>>=Does subnet contain or equal subnet?Note: I'm not familiar with duckdb code and it is my first contribution.