sql: make DISTSQL option to EXPLAIN ANALYZE optional

[asubiotto]

Reduce user surprise by not requiring a DISTSQL option in EXPLAIN
ANALYZE.

Release note (sql change): EXPLAIN ANALYZE is now a valid
equivalent of EXPLAIN ANALYZE (DISTSQL)

[cockroach-teamcity]

This change is 

[cockroach-teamcity]

This change is 

[asubiotto]

cc @jseldess @knz @RaduBerinde

[asubiotto]

cc @jseldess @knz @RaduBerinde

[asubiotto]

Planning to backport to 2.1

[asubiotto]

Planning to backport to 2.1

[knz]

LGTM modulo test location

[jordanlewis]

Outcome looks good, but have some implementation nits

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/sql/parser/sql.y, line 2237 at r1 (raw file):

| EXPLAIN ANALYZE preparable_stmt
  {
    $$.val = &tree.Explain{Options: []string{"DISTSQL", $2}, Statement: $3.stmt()}

I don't think this is right as I don't think the statement will roundtrip now - formatting this statement will be EXPLAIN ANALYZE (DISTSQL) instead of what it started as, just EXPLAIN ANALYZE. I think you should do this transformation later in the code, not in the parser.

Also you should add a parse roundtrip test for this (these live in parse_test.go).

[jseldess]

Thanks, @asubiotto! cc @lhirata.

[jseldess]

Thanks, @asubiotto! cc @lhirata.

[knz]

I disagree, the parser is the right place.

[knz]

I disagree, the parser is the right place.

[jordanlewis]

I disagree, the parser is the right place.

Ok, overruled! LGTM if you add that parse test.

[jordanlewis]

I disagree, the parser is the right place.

Ok, overruled! LGTM if you add that parse test.

[asubiotto]

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/sql/logictest/testdata/logic_test/explain_analyze, line 14 at r1 (raw file):

Previously, knz (kena) wrote…

since this is an alias please put in in TestParse2 in parse_test.go

Done.

---

pkg/sql/parser/sql.y, line 2237 at r1 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

I don't think this is right as I don't think the statement will roundtrip now - formatting this statement will be EXPLAIN ANALYZE (DISTSQL) instead of what it started as, just EXPLAIN ANALYZE. I think you should do this transformation later in the code, not in the parser.

Also you should add a parse roundtrip test for this (these live in parse_test.go).

Added to alias tests in TestParse2

[asubiotto]

bors r+

[asubiotto]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)

[craig]

Build succeeded

• GitHub CI (Cockroach)