Skip to content

Conversation

sploiselle
Copy link
Contributor

@sploiselle sploiselle commented Aug 16, 2016

This change is Reviewable

@sploiselle sploiselle force-pushed the sql-feature-matrix branch 2 times, most recently from 5d839fb to 96c7abd Compare August 16, 2016 21:23
@RaduBerinde
Copy link
Member

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion.


sql-feature-support.md, line 60 [r1] (raw file):

| Component | Supported | Type | Details |
|-----------|-----------|------|---------|
| `INNER JOIN` || Standard | [`JOIN` blog post](https://www.cockroachlabs.com/blog/cockroachdbs-first-join/) |

Given our current implementation, we should mark these with a footnote instead of a full "check". The footnote can say that these operations are supported only for small data sets. @knz what do you think?


Comments from Reviewable

@knz
Copy link
Contributor

knz commented Aug 16, 2016

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion.


sql-feature-support.md, line 60 [r1] (raw file):

Previously, RaduBerinde wrote…

Given our current implementation, we should mark these with a footnote instead of a full "check". The footnote can say that these operations are supported only for small data sets. @knz what do you think?

Yes I agree with you. Let's make it a yellow checkmark or a round blue info icon. At least avoid green.

Comments from Reviewable

@sploiselle
Copy link
Contributor Author

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion.


sql-feature-support.md, line 60 [r1] (raw file):

Previously, knz (kena) wrote…

Yes I agree with you. Let's make it a yellow checkmark or a round blue info icon. At least avoid green.

Done. Moved this to the **Statements** section since all of the `JOIN` types are supported and had the same info and lengthy note. Avoided obfuscating the info by changing the "Supported" column from ✓ to Functional, with the specification about small data sets in the "Details" column.

Comments from Reviewable

@RaduBerinde
Copy link
Member

Very nice, thank you! :lgtm:


Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@jseldess
Copy link
Contributor

Excellent work, @sploiselle! LGTM.

@nvanbenschoten, should introspection be added? If so, can you suggest an approach?


Reviewed 2 of 2 files at r1.
Review status: 1 of 2 files reviewed at latest revision, 3 unresolved discussions.


sql-feature-support.md, line 7 [r1] (raw file):

## Overview
Making CockroachDB easy to use was a top priority for us, so we chose to implement SQL. However, even though SQL has a standard, no database implements all of it, nor do any of them have standard implementations of all features.

Let's continue this priority until the end of time: "...easy to use is a top priority for us..."


sql-feature-support.md, line 11 [r1] (raw file):

To understand which standard SQL features we support (as well as common extensions to the standard), use the table below.

- __Component__ lists the components that are commonly considered part of SQL.

nit: Would you mind using **Component** as the markdown for bold, for consistency with other pages?


Comments from Reviewable

@sploiselle sploiselle merged commit b0cccae into gh-pages Aug 18, 2016
@sploiselle sploiselle deleted the sql-feature-matrix branch August 18, 2016 17:27
@nvb
Copy link
Contributor

nvb commented Aug 18, 2016

@jseldess We could add a section for supported methods of introspection, but I foresee a page/section like https://www.postgresql.org/docs/9.5/static/information-schema.html being more immediately useful. Each subsection would provide a brief overview of a supported introspection table, along with it's schema.

@jseldess
Copy link
Contributor

Thanks, @nvanbenschoten. Noted.

Simran-B added a commit to Simran-B/docs that referenced this pull request Aug 21, 2025
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.

5 participants