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

INET data type #2439

Merged
merged 5 commits into from
Feb 6, 2018
Merged

INET data type #2439

merged 5 commits into from
Feb 6, 2018

Conversation

lnhsingh
Copy link
Contributor

Closes #2016.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@lnhsingh lnhsingh changed the title inet datatype docs - DRAFT inet datatype docs Feb 5, 2018
<div id="toc"></div>

## Syntax

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nvanbenschoten - how do you determine if a data type can be an interpreted literal, an annotated string literal, or coerced to type INET? Do any of these apply to INET?

Copy link
Member

Choose a reason for hiding this comment

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

All three of them apply to INET! I think the easiest way to determine this is to try it out:

CREATE TABLE t (a INET);
INSERT INTO t VALUES (INET '190.0.0.0'), ('190.0.0.0':::INET), ('190.0.0.0');

Copy link
Member

Choose a reason for hiding this comment

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

EDIT: the first one doesn't actually look like it works.. I'll have to fix that during my QA process (cockroachdb/cockroach#22390). I think it's safe to assume that all three will work by the time this is published.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

v2.0/inet.md Outdated
Format | Description
-------|-------------
IPv4 | Standard [RFC791](https://tools.ietf.org/html/rfc791)-specified format of 4 octets expressed individually in decimal numbers and separated by periods. Optionally, the address can be followed by a subnet mask.<br><br> Examples: `'190.0.0.0'`, `'190.0.0.0/24'`
IPv4-mapped IPv6 | IPv6 address mapped to a IPv4 address. <br><br>Example: `'::ffff:192.168.0.1/24'`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nvanbenschoten - do you think this should this be called out as its own format? Or should it be included as an option in the IPv6 description?

Copy link
Member

Choose a reason for hiding this comment

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

I'd include it as an option in the IPv6 description.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

+--------------------------------------+--------------------------+---------------------------+
~~~

## Supported Casting & Conversion
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nvanbenschoten - similar to above, how do you determine what INET can be cast to?

Copy link
Member

Choose a reason for hiding this comment

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

I think the easiest way is to look at the code: https://github.com/cockroachdb/cockroach/blob/94a2182a5e00fc0963c0f20c1dc278b12a9df7d2/pkg/sql/sem/tree/expr.go#L1254.

inetCastTypes is a list of all the types that INET can be cast to. It looks like only STRINGs can be cast to INET. I'll be auditing all of this as part of my QA process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@@ -723,6 +723,12 @@
"/${VERSION}/data-types.html"
]
},
{
"title": "<code>INET</code>",
Copy link
Member

Choose a reason for hiding this comment

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

Is there a method to the sorting of this list? It looks arbitrary to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch - I'll update to be alphabetical.

@@ -10,6 +10,7 @@ CockroachDB supports the following data types. Click a type for more details.

Type | Description | Example
-----|-------------|--------
[`INET`](inet.html) | A IPv4 or IPv6 address. | `192.168.0.1`
Copy link
Member

Choose a reason for hiding this comment

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

Do we want a <span class="version-tag">New in v2.0:</span> tag? Are we doing those yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

Copy link
Member

Choose a reason for hiding this comment

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

Are we going to remove the New in v1.1 tags from the 2.0 docs?

Copy link
Contributor Author

@lnhsingh lnhsingh Feb 6, 2018

Choose a reason for hiding this comment

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

I've been leaving the 1.1 flags in the 2.0 docs. I think (if we remove them) we'll do it all at once.

@@ -219,7 +219,7 @@ interpreted literal. These are special cases of

For more information about the allowable format of interpreted
literals, refer to the "Syntax" section of the respective data types:
[`DATE`](date.html#syntax), [`INTERVAL`](interval.html#syntax),
[`DATE`](date.html#syntax), [`INTERVAL`](interval.html#syntax), [`TIME`](time.html#syntax),
[`TIMESTAMP`/`TIMESTAMPTZ`](timestamp.html#syntax).
Copy link
Member

Choose a reason for hiding this comment

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

Should we add INET here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

<div id="toc"></div>

## Syntax

Copy link
Member

Choose a reason for hiding this comment

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

All three of them apply to INET! I think the easiest way to determine this is to try it out:

CREATE TABLE t (a INET);
INSERT INTO t VALUES (INET '190.0.0.0'), ('190.0.0.0':::INET), ('190.0.0.0');

v2.0/inet.md Outdated
Format | Description
-------|-------------
IPv4 | Standard [RFC791](https://tools.ietf.org/html/rfc791)-specified format of 4 octets expressed individually in decimal numbers and separated by periods. Optionally, the address can be followed by a subnet mask.<br><br> Examples: `'190.0.0.0'`, `'190.0.0.0/24'`
IPv4-mapped IPv6 | IPv6 address mapped to a IPv4 address. <br><br>Example: `'::ffff:192.168.0.1/24'`
Copy link
Member

Choose a reason for hiding this comment

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

I'd include it as an option in the IPv6 description.

v2.0/inet.md Outdated
IPv4-mapped IPv6 | IPv6 address mapped to a IPv4 address. <br><br>Example: `'::ffff:192.168.0.1/24'`
IPv6 | Standard [RFC8200](https://tools.ietf.org/html/rfc8200)-specified format of 8 colon-separated groups of 4 hexadecimal digits. Optionally, the address can be followed by a subnet mask.<br><br> Example: `'2001:4f8:3:ba:2e0:81ff:fe22:d1f1'`, `'2001:4f8:3:ba:2e0:81ff:fe22:d1f1/120'`

{{site.data.alerts.callout_info}}IPv4 addresses will sort before IPv6 addresses, including IPv4 addresses mapped to IPv6 addresses.{{site.data.alerts.end}}
Copy link
Member

Choose a reason for hiding this comment

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

Good idea to include this callout.

<div id="toc"></div>

## Syntax

Copy link
Member

Choose a reason for hiding this comment

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

EDIT: the first one doesn't actually look like it works.. I'll have to fix that during my QA process (cockroachdb/cockroach#22390). I think it's safe to assume that all three will work by the time this is published.

+--------------------------------------+--------------------------+---------------------------+
~~~

## Supported Casting & Conversion
Copy link
Member

Choose a reason for hiding this comment

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

I think the easiest way is to look at the code: https://github.com/cockroachdb/cockroach/blob/94a2182a5e00fc0963c0f20c1dc278b12a9df7d2/pkg/sql/sem/tree/expr.go#L1254.

inetCastTypes is a list of all the types that INET can be cast to. It looks like only STRINGs can be cast to INET. I'll be auditing all of this as part of my QA process.

@lnhsingh
Copy link
Contributor Author

lnhsingh commented Feb 5, 2018

Thanks for the review @nvanbenschoten!

Lauren added 2 commits February 5, 2018 16:46
INET first draft

add INET to sidebar

syntax

minor edits
@lnhsingh lnhsingh changed the title inet datatype docs INET data type Feb 5, 2018
Copy link
Contributor

@jseldess jseldess left a comment

Choose a reason for hiding this comment

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

This is great, @lhirata! LGTM, with a few minor comments.

v2.0/inet.md Outdated
Format | Description
-------|-------------
IPv4 | Standard [RFC791](https://tools.ietf.org/html/rfc791)-specified format of 4 octets expressed individually in decimal numbers and separated by periods. Optionally, the address can be followed by a subnet mask.<br><br> Examples: `'190.0.0.0'`, `'190.0.0.0/24'`
IPv6 | Standard [RFC8200](https://tools.ietf.org/html/rfc8200)-specified format of 8 colon-separated groups of 4 hexadecimal digits. An IPv6 address can be mapped to an IPv4 address. Optionally, the address can be followed by a subnet mask.<br><br> Example: `'2001:4f8:3:ba:2e0:81ff:fe22:d1f1'`, `'2001:4f8:3:ba:2e0:81ff:fe22:d1f1/120'`, `'::ffff:192.168.0.1/24'`
Copy link
Contributor

Choose a reason for hiding this comment

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

Example: > Examples:

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: You also don't need a space after a <br>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

v2.0/inet.md Outdated
> INSERT INTO computers
VALUES
('192.168.0.1', 'info@cockroachlabs.com', '2018-01-31'),
('192.168.0.2/10', 'lauren@cockroachlabs.com', '2018-01-31');
Copy link
Contributor

Choose a reason for hiding this comment

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

At end of line, change ; to ,; otherwise, this statement fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

v2.0/inet.md Outdated
-----|--------
`STRING` | Converts to format `'Address/subnet'`.

## See Also
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make these a bulleted list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@nvanbenschoten
Copy link
Member

LGTM!

@lnhsingh lnhsingh merged commit 5db7f05 into master Feb 6, 2018
@lnhsingh lnhsingh deleted the inet-docs branch February 6, 2018 18:15
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.

4 participants