Skip to content

Conversation

ericharmeling
Copy link
Contributor

Fixes #5947.

  • Added column to data type landing page for vectorized support
  • Added note to page for each data type not supported by vectorized execution

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

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

Thanks for the update! A few comments from me.

Reviewed 24 of 24 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @ericharmeling)


v19.2/data-types.md, line 18 at r1 (raw file):

[`BYTES`](bytes.html) | A string of binary characters. | `b'\141\061\142\062\143\063'` | Supported
[`COLLATE`](collate.html) | The `COLLATE` feature lets you sort [`STRING`](string.html) values according to language- and country-specific rules, known as collations.  | `'a1b2c3' COLLATE en` | Not supported
[`DATE`](date.html) | A date.  | `DATE '2016-01-25'` | Not supported

DATE type is supported.


v19.2/data-types.md, line 19 at r1 (raw file):

[`COLLATE`](collate.html) | The `COLLATE` feature lets you sort [`STRING`](string.html) values according to language- and country-specific rules, known as collations.  | `'a1b2c3' COLLATE en` | Not supported
[`DATE`](date.html) | A date.  | `DATE '2016-01-25'` | Not supported
[`DECIMAL`](decimal.html) | An exact, fixed-point number.  | `1.2345` | Not supported

DECIMAL is partially supported (we do not support the case when DECIMALs need to be sent over the network - i.e. we don't have serialization yet).


v19.2/data-types.md, line 28 at r1 (raw file):

[`STRING`](string.html) | A string of Unicode characters. | `'a1b2c3'` | Supported
[`TIME`](time.html) | A time of day in UTC.  | `TIME '01:23:45.123456'` | Not supported
[`TIMESTAMP`<br>`TIMESTAMPTZ`](timestamp.html) | A date and time pairing in UTC. | `TIMESTAMP '2016-01-25 10:10:10'`<br>`TIMESTAMPTZ '2016-01-25 10:10:10-05:00'` | Supported

In 19.2 only Timestamp is supported (not TimestampTZ) (since 19.2.1, same as UUID).


v20.1/data-types.md, line 19 at r1 (raw file):

[`COLLATE`](collate.html) | The `COLLATE` feature lets you sort [`STRING`](string.html) values according to language- and country-specific rules, known as collations.  | `'a1b2c3' COLLATE en` | Not supported
[`DATE`](date.html) | A date.  | `DATE '2016-01-25'` | Not supported
[`DECIMAL`](decimal.html) | An exact, fixed-point number.  | `1.2345` | Not supported

Same partial support at the moment (although this should be fully supported in the next alpha).

Eric Harmeling added 2 commits January 15, 2020 10:29
Added note to page for each data type not supported by vectorized execution
Copy link
Contributor Author

@ericharmeling ericharmeling left a comment

Choose a reason for hiding this comment

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

TFTR @yuzefovich ! Please see my comments and the latest commit, which addresses your review comments.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @yuzefovich)


v19.2/data-types.md, line 18 at r1 (raw file):

Previously, yuzefovich wrote…

DATE type is supported.

Done.


v19.2/data-types.md, line 19 at r1 (raw file):

Previously, yuzefovich wrote…

DECIMAL is partially supported (we do not support the case when DECIMALs need to be sent over the network - i.e. we don't have serialization yet).

OK please review the latest commit on this.


v19.2/data-types.md, line 28 at r1 (raw file):
OK.

(since 19.2.1, same as UUID)

I'm a little confused by this. It looks like UUID is supported in 19.2: cockroachdb/cockroach#42414

Unless you are saying that 19.2.0 does not support UUID, but 19.2.1 does. In that case, we only document the latest version of a particular release (e.g. 19.2 docs should be correct for the latest 19.2.x, and not for 19.2.x versions prior to the latest).


v20.1/data-types.md, line 19 at r1 (raw file):

DECIMALs need to be sent over the network - i.e. we don't have serialization yet

Is this no longer the case as of yesterday? cockroachdb/cockroach#43311

Will this change be backported to 19.2 as well?

Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 9 of 9 files at r2.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @ericharmeling and @yuzefovich)


v19.2/data-types.md, line 28 at r1 (raw file):

Previously, ericharmeling (Eric Harmeling) wrote…

OK.

(since 19.2.1, same as UUID)

I'm a little confused by this. It looks like UUID is supported in 19.2: cockroachdb/cockroach#42414

Unless you are saying that 19.2.0 does not support UUID, but 19.2.1 does. In that case, we only document the latest version of a particular release (e.g. 19.2 docs should be correct for the latest 19.2.x, and not for 19.2.x versions prior to the latest).

Yeah, that's what I meant - the support for both Timestamp and UUID was added in 19.2.1 (those are not supported in 19.2.0). I see the point about the latest minor release, my remark then indeed is quite confusing :)


v20.1/data-types.md, line 19 at r1 (raw file):

Previously, ericharmeling (Eric Harmeling) wrote…

DECIMALs need to be sent over the network - i.e. we don't have serialization yet

Is this no longer the case as of yesterday? cockroachdb/cockroach#43311

Will this change be backported to 19.2 as well?

Yes, as of yesterday for 20.1.

We do not plan to backport that change to 19.2 (to the best of my knowledge).

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.

Additional column to depict data types supported by vectorized execution
4 participants