[RFC] Typing v2 #6189

nvanbenschoten · Apr 20, 2016

After beginning to implement the previous proposal, a few of the typing
rules were discovered to be undesirable. This proposal introduces a
third typing system called Summer (which has already been mostly implemented).
The new system is an extension of Morty, with the primary difference being
the notion of a "desired type" which is propagated downwards while type
checking. In practice, this eliminates the need for an expensive and arguably
excessive reliance on exact arithmetic throughout the evaluation of statements
and removes the need for a reliance on unclear and difficult to reason about implicit
type coercions.

[ci skip]

This change is

One criteria I would suggest is how easy or difficult it will be to explain to a MySQL or Postgres user how they will have to change their queries to migrate to CockroachDB.

Not to imply that there can't be any differences, but at least to highlight a comprehensive set of test cases and show that they are either fully compatible or give comprehensible diagnostic messages that make it easy enough to convert from MySQL or Postgres.

Granted, all of the rules and nuances will need to be fully documented - in user-speak - but the longer the doc is, the less likely that an average user will read it. If the new system automatically does the right thing 98% of the time and gives a clear message the other 2% of the time that is easy to follow to eliminate the error without reading the fine print in the doc, you'll be set. The debate can be about whether 98% should be 99.5% or if 95% (19 out of 20 queries) is good enough.

JackKrupansky · Apr 20, 2016

One criteria I would suggest is how easy or difficult it will be to explain to a MySQL or Postgres user how they will have to change their queries to migrate to CockroachDB.

Not to imply that there can't be any differences, but at least to highlight a comprehensive set of test cases and show that they are either fully compatible or give comprehensible diagnostic messages that make it easy enough to convert from MySQL or Postgres.

Granted, all of the rules and nuances will need to be fully documented - in user-speak - but the longer the doc is, the less likely that an average user will read it. If the new system automatically does the right thing 98% of the time and gives a clear message the other 2% of the time that is easy to follow to eliminate the error without reading the fine print in the doc, you'll be set. The debate can be about whether 98% should be 99.5% or if 95% (19 out of 20 queries) is good enough.

One of the old issues is decimal vs. floating point, so I have to ask whether decimal constants will be implemented as a precursor to the new typing scheme.

Although supporting decimal constants does not eliminate float/decimal conversion issues since parameters will have their type inferred as decimal or float or integer from say an INSERT and then possibly used in a comparison to the other type - for example used in a comparison to a constant that lexically may type as float or integer because the length permits it. It depends on what rules the lexical analysis uses to determine whether an integer might be large enough to be a decimal or whether a number with a decimal point is always parsed as a decimal or only if it is out of range of a float value.

JackKrupansky · Apr 21, 2016

One of the old issues is decimal vs. floating point, so I have to ask whether decimal constants will be implemented as a precursor to the new typing scheme.

Although supporting decimal constants does not eliminate float/decimal conversion issues since parameters will have their type inferred as decimal or float or integer from say an INSERT and then possibly used in a comparison to the other type - for example used in a comparison to a constant that lexically may type as float or integer because the length permits it. It depends on what rules the lexical analysis uses to determine whether an integer might be large enough to be a decimal or whether a number with a decimal point is always parsed as a decimal or only if it is out of range of a float value.

@JackKrupansky

@JackKrupansky Untyped constants are a major part of this proposal. I encourage you to read about the constant folding phase of type checking system. The short answer is that constants (numeric and string) will be folded as far as possible before being typed, and will then be given a type based on their possible representations and their surrounding context.

nvanbenschoten · Apr 21, 2016

@JackKrupansky Untyped constants are a major part of this proposal. I encourage you to read about the constant folding phase of type checking system. The short answer is that constants (numeric and string) will be folded as far as possible before being typed, and will then be given a type based on their possible representations and their surrounding context.

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

docs/RFCS/typing.md, line 365 [r1] (raw file):
hard to understand this sentence, grammar seems off

docs/RFCS/typing.md, line 368 [r1] (raw file):
same here

Comments from Reviewable

RaduBerinde · Apr 21, 2016

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

docs/RFCS/typing.md, line 365 [r1] (raw file):
hard to understand this sentence, grammar seems off

docs/RFCS/typing.md, line 368 [r1] (raw file):
same here

Comments from Reviewable

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

docs/RFCS/typing.md, line 365 [r1] (raw file):
Done.

docs/RFCS/typing.md, line 368 [r1] (raw file):
Better?

Comments from Reviewable

nvanbenschoten · Apr 21, 2016

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

docs/RFCS/typing.md, line 365 [r1] (raw file):
Done.

docs/RFCS/typing.md, line 368 [r1] (raw file):
Better?

Comments from Reviewable

This is hugely detailed, though a bit overwhelming. The details here are almost certainly going to rot versus the implementation. And when you provide too much detail it can obscure the higher-level design. There is a balance between too much and too little information in an RFC and I think this one went a bit too far. This is a minor critique, though, and certainly nothing that should be changed now. A lot of the detail here will flow nicely into a blog post.

Review status: 0 of 1 files reviewed at latest revision, 12 unresolved discussions, all commit checks successful.

docs/RFCS/typing.md, line 322 [r2] (raw file):
s/more desirable/more desirable results/g

docs/RFCS/typing.md, line 323 [r2] (raw file):
s/while all the while/while/g

docs/RFCS/typing.md, line 328 [r2] (raw file):
s/recursion/recursion./g

docs/RFCS/typing.md, line 331 [r2] (raw file):
s/resolution/resolution./g

docs/RFCS/typing.md, line 351 [r2] (raw file):
I'm not clear: are you proposing an extension to the SQL syntax we support, or just the syntax used in this document?

docs/RFCS/typing.md, line 393 [r2] (raw file):
The switch from resolvable to resolved in the two names is confusing. Shouldn't the latter be resolvable as well?

docs/RFCS/typing.md, line 450 [r2] (raw file):
Have you found an example where this is useful?

docs/RFCS/typing.md, line 495 [r2] (raw file):
s/with an/with a/g

docs/RFCS/typing.md, line 525 [r2] (raw file):
s/candidate/candidates/g

docs/RFCS/typing.md, line 567 [r2] (raw file):
What is this supposed to return given that max() takes 1 argument, usually a column name? I think you need a different example function.

docs/RFCS/typing.md, line 645 [r2] (raw file):
The use of the term demanded is new here. You've used required elsewhere. Is this different?

docs/RFCS/typing.md, line 683 [r2] (raw file):
s/"1"/"$1"/g?

Comments from Reviewable

petermattis · Apr 25, 2016

This is hugely detailed, though a bit overwhelming. The details here are almost certainly going to rot versus the implementation. And when you provide too much detail it can obscure the higher-level design. There is a balance between too much and too little information in an RFC and I think this one went a bit too far. This is a minor critique, though, and certainly nothing that should be changed now. A lot of the detail here will flow nicely into a blog post.

Review status: 0 of 1 files reviewed at latest revision, 12 unresolved discussions, all commit checks successful.

docs/RFCS/typing.md, line 322 [r2] (raw file):
s/more desirable/more desirable results/g

docs/RFCS/typing.md, line 323 [r2] (raw file):
s/while all the while/while/g

docs/RFCS/typing.md, line 328 [r2] (raw file):
s/recursion/recursion./g

docs/RFCS/typing.md, line 331 [r2] (raw file):
s/resolution/resolution./g

docs/RFCS/typing.md, line 351 [r2] (raw file):
I'm not clear: are you proposing an extension to the SQL syntax we support, or just the syntax used in this document?

docs/RFCS/typing.md, line 393 [r2] (raw file):
The switch from resolvable to resolved in the two names is confusing. Shouldn't the latter be resolvable as well?

docs/RFCS/typing.md, line 450 [r2] (raw file):
Have you found an example where this is useful?

docs/RFCS/typing.md, line 495 [r2] (raw file):
s/with an/with a/g

docs/RFCS/typing.md, line 525 [r2] (raw file):
s/candidate/candidates/g

docs/RFCS/typing.md, line 567 [r2] (raw file):
What is this supposed to return given that max() takes 1 argument, usually a column name? I think you need a different example function.

docs/RFCS/typing.md, line 645 [r2] (raw file):
The use of the term demanded is new here. You've used required elsewhere. Is this different?

docs/RFCS/typing.md, line 683 [r2] (raw file):
s/"1"/"$1"/g?

Comments from Reviewable

@petermattis

@petermattis I agree that we may have put a little too much detail into this RFC amendment. That said, I think the reason for its focus on detail was pretty well warranted. The implementation of this new typing system (Summer) was an arguably significant deviation from the system previously described in this RFC. Because of this, it seemed important that we take a step back from the code and look to both prove its correctness and justify the changes we made to the old system. I think these changes succeed in both regards. Plus like you mentioned, this can all be pulled into a blog post.

Review status: 0 of 1 files reviewed at latest revision, 13 unresolved discussions, some commit checks pending.

Comments from Reviewable

nvanbenschoten · Apr 25, 2016

@petermattis I agree that we may have put a little too much detail into this RFC amendment. That said, I think the reason for its focus on detail was pretty well warranted. The implementation of this new typing system (Summer) was an arguably significant deviation from the system previously described in this RFC. Because of this, it seemed important that we take a step back from the code and look to both prove its correctness and justify the changes we made to the old system. I think these changes succeed in both regards. Plus like you mentioned, this can all be pulled into a blog post.

Review status: 0 of 1 files reviewed at latest revision, 13 unresolved discussions, some commit checks pending.

Comments from Reviewable

Review status: 0 of 1 files reviewed at latest revision, 12 unresolved discussions, some commit checks pending.

docs/RFCS/typing.md, line 322 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 323 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 328 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 331 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 351 [r2] (raw file):
Here we are proposing a new SQL extension syntax. It is not needed for the correctness of the typing system so it wont be included in the initial implementation. Still, it will be a nice tool for developers to use to gain a greater level of confidence in their SQL statements, if desired.

docs/RFCS/typing.md, line 393 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 450 [r2] (raw file):
It's very useful in allowing for better, more helpful error reporting

docs/RFCS/typing.md, line 495 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 525 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 567 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 645 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 683 [r2] (raw file):
Done.

Comments from Reviewable

nvanbenschoten · Apr 25, 2016

Review status: 0 of 1 files reviewed at latest revision, 12 unresolved discussions, some commit checks pending.

docs/RFCS/typing.md, line 322 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 323 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 328 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 331 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 351 [r2] (raw file):
Here we are proposing a new SQL extension syntax. It is not needed for the correctness of the typing system so it wont be included in the initial implementation. Still, it will be a nice tool for developers to use to gain a greater level of confidence in their SQL statements, if desired.

docs/RFCS/typing.md, line 393 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 450 [r2] (raw file):
It's very useful in allowing for better, more helpful error reporting

docs/RFCS/typing.md, line 495 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 525 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 567 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 645 [r2] (raw file):
Done.

docs/RFCS/typing.md, line 683 [r2] (raw file):
Done.

Comments from Reviewable