[WIP] sql: refactor the statement pretty-printer and implement EXPLAIN(TYPES) #6406
Conversation
This is one of the cornerstones of the Summer typing algorithm. Type checking now takes a "desired" type, which is a hint to `Expr`s to resolve themselves as the type, if possible. It it up to the caller to assert that the type returned if correct, if it is actually required.
This commit adds a `TypedExpr` interface which is a superset of Expr but can also perform expression evaluation and can return type annotations.
This change creates the types `NumVal` and `StrVal`, which both implement the new `Constant` interface. The interface is a super-set of `Expr`, but includes extra methods for literal values.
This commit creates a new error type to hold commonly seen errors. It then uses this error in a few previously ambiguous situations.
Previously `(1) + 2` would not have folded to `3`, now it does.
This change pulls the constant related visitors into constant.go and cleans up a few tests.
This change creates the types `unaryOpOverload`, `binOpOverload`, and `cmpOpOverload`, each with a lookup method for an overload implementation for a given set of argument types. It also changes `ComparisonOp` to `ComparisonOperator` to stay consistent.
This commit cleans up a number of uses of the new `TypedExpr` interface, improves the type checking system, added new test cases, and adds in a few TODOs for future work.
|
Hi Raphael, This is awesome, thanks for working on this! I have a couple of suggestions, but nothing that needs to be handled in this PR necessarily. BTW I found a decent way to post PRs on top of not-yet-merged commits - I push those other commits into a temporary branch in the main cockroach repository, and then generate a PR between that branch and my work branch. Review status: 0 of 79 files reviewed at latest revision, 2 unresolved discussions, all commit checks successful. sql/parser/expr.go, line 89 [r9] (raw file): Later if we want to implement operator precedence into account the "flags" could keep track of the priorities of the preceding and following operators to determine if parens are really needed.OR sql/parser/format.go, line 32 [r9] (raw file): Even if we keep it as an int, I would define a Comments from Reviewable |
This patch replaces the pretty-printing using a tree of String() calls by a Formattable interface common to all syntax object. This interface allows caller code to pass formatting flags throughout the pretty printing. This can be used later to implement conditional printing of node details (e.g. typing information). A side effect is a performance improvement: prior to this patch String and bytes.Buffer objects would be created and destroyed throughout the recursion, causing pressure on memory allocation. With this patch a single Buffer object is used per top-level request to the pretty-printer.
This patch introduces a new features: the statement `EXPLAIN (TYPES)` to inspect the types of intermediate results in a query plan. Known limitation: `EXPLAIN(TYPES)` cannot be applied on statements containing placeholders ($xxx). As a side effect this patch also fixes a bug, where EXPLAIN(PLAN) would cause a panic when invoked on an INSERT statement.
|
Replaced by #6424 to ease reviewing. |
|
Radu, thanks for the suggestions, I have applied them in the new version
of the patch. See PR #6424.
|
This commit relies on #6358 being merged first. All the commits prior to the last (where the work of this PR is being done) can be ignored, as they really belong to the other PR.
This change is