-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
[RFC] Typing v2 #6189
[RFC] Typing v2 #6189
Conversation
c439c78
to
076e100
Compare
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 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): docs/RFCS/typing.md, line 368 [r1] (raw file): 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): docs/RFCS/typing.md, line 368 [r1] (raw file): Comments from Reviewable |
076e100
to
f4216cf
Compare
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): docs/RFCS/typing.md, line 323 [r2] (raw file): docs/RFCS/typing.md, line 328 [r2] (raw file): docs/RFCS/typing.md, line 331 [r2] (raw file): docs/RFCS/typing.md, line 351 [r2] (raw file): docs/RFCS/typing.md, line 393 [r2] (raw file): docs/RFCS/typing.md, line 450 [r2] (raw file): docs/RFCS/typing.md, line 495 [r2] (raw file): docs/RFCS/typing.md, line 525 [r2] (raw file): docs/RFCS/typing.md, line 567 [r2] (raw file): docs/RFCS/typing.md, line 645 [r2] (raw file): docs/RFCS/typing.md, line 683 [r2] (raw file): Comments from Reviewable |
@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): docs/RFCS/typing.md, line 323 [r2] (raw file): docs/RFCS/typing.md, line 328 [r2] (raw file): docs/RFCS/typing.md, line 331 [r2] (raw file): docs/RFCS/typing.md, line 351 [r2] (raw file): docs/RFCS/typing.md, line 393 [r2] (raw file): docs/RFCS/typing.md, line 450 [r2] (raw file): docs/RFCS/typing.md, line 495 [r2] (raw file): docs/RFCS/typing.md, line 525 [r2] (raw file): docs/RFCS/typing.md, line 567 [r2] (raw file): docs/RFCS/typing.md, line 645 [r2] (raw file): docs/RFCS/typing.md, line 683 [r2] (raw file): Comments from Reviewable |
e8c7541
to
48c3ff8
Compare
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful. docs/RFCS/typing.md, line 351 [r2] (raw file): Comments from Reviewable |
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful. docs/RFCS/typing.md, line 351 [r2] (raw file): One alternative that we briefly discussed was Comments from Reviewable |
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful. docs/RFCS/typing.md, line 351 [r2] (raw file): Comments from Reviewable |
You took out the max example, but it probably should have been GREATEST($1,$2), as well as GREATEST/LEAST($1,$2,$3) is illustrate the added complexity of variable-length argument lists. |
@JackKrupansky GREATEST/LEAST are handled by the homogeneity rule, not overload resolution. |
31f6625
to
62d1104
Compare
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). [ci skip]
62d1104
to
8879af7
Compare
[ci skip]
I think given the implementation has landed and tests suggest that this works, we should merge this PR. Thoughts? |
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