{"action":"created","issue":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/22571","repository_url":"https://api.github.com/repos/cockroachdb/cockroach","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/22571/labels{/name}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/22571/comments","events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/22571/events","html_url":"https://github.com/cockroachdb/cockroach/pull/22571","id":296160884,"number":22571,"title":"sql: experimental execution path through the optimizer","user":{"login":"RaduBerinde","id":16544120,"avatar_url":"https://avatars1.githubusercontent.com/u/16544120?v=4","gravatar_id":"","url":"https://api.github.com/users/RaduBerinde","html_url":"https://github.com/RaduBerinde","followers_url":"https://api.github.com/users/RaduBerinde/followers","following_url":"https://api.github.com/users/RaduBerinde/following{/other_user}","gists_url":"https://api.github.com/users/RaduBerinde/gists{/gist_id}","starred_url":"https://api.github.com/users/RaduBerinde/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RaduBerinde/subscriptions","organizations_url":"https://api.github.com/users/RaduBerinde/orgs","repos_url":"https://api.github.com/users/RaduBerinde/repos","events_url":"https://api.github.com/users/RaduBerinde/events{/privacy}","received_events_url":"https://api.github.com/users/RaduBerinde/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":10,"created_at":"2018-02-11T04:55:32Z","updated_at":"2018-02-12T15:08:02Z","closed_at":null,"author_association":"OWNER","pull_request":{"url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/22571","html_url":"https://github.com/cockroachdb/cockroach/pull/22571","diff_url":"https://github.com/cockroachdb/cockroach/pull/22571.diff","patch_url":"https://github.com/cockroachdb/cockroach/pull/22571.patch"},"body":"#### sql: experimental execution path through the optimizer\r\n\r\nIn order to be test the optimizer end-to-end, we wire the executor to\r\nuse the optimizer instead of the normal path (controlled by a session\r\nvariable).\r\n\r\nAs part of the implementation, we make `execEngine` implement\r\n`Catalog` using the regular code path to look up a table. The\r\nassociation between ExecEngine and Catalog is a bit unexpected - but\r\nthey share the context of the planner (most importantly the txn). We\r\nwill probably have to improve the interface (perhaps make a \"parent\"\r\ninterface which creates the planner and has accessors for the\r\n`Catalog` and the `ExecEngine`).\r\n\r\nFinally, we fix a problem in the optbuilder panic handling: the\r\nintention was to only catch `builderError` but we catch all `error`s\r\nbecause `error` is an interface (so `type builderError error` is the same\r\ninterface); we change `builderError` to a type.\r\n\r\nRelease note: None.\r\n"},"comment":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/comments/364950247","html_url":"https://github.com/cockroachdb/cockroach/pull/22571#issuecomment-364950247","issue_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/22571","id":364950247,"user":{"login":"RaduBerinde","id":16544120,"avatar_url":"https://avatars1.githubusercontent.com/u/16544120?v=4","gravatar_id":"","url":"https://api.github.com/users/RaduBerinde","html_url":"https://github.com/RaduBerinde","followers_url":"https://api.github.com/users/RaduBerinde/followers","following_url":"https://api.github.com/users/RaduBerinde/following{/other_user}","gists_url":"https://api.github.com/users/RaduBerinde/gists{/gist_id}","starred_url":"https://api.github.com/users/RaduBerinde/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RaduBerinde/subscriptions","organizations_url":"https://api.github.com/users/RaduBerinde/orgs","repos_url":"https://api.github.com/users/RaduBerinde/repos","events_url":"https://api.github.com/users/RaduBerinde/events{/privacy}","received_events_url":"https://api.github.com/users/RaduBerinde/received_events","type":"User","site_admin":false},"created_at":"2018-02-12T15:08:02Z","updated_at":"2018-02-12T15:08:02Z","author_association":"OWNER","body":"TFTR!\n\n---\n\nReview status: 7 of 17 files reviewed at latest revision, 8 unresolved discussions.\n\n---\n\n*[pkg/sql/executor.go, line 2085 at r8](https://reviewable.io:443/reviews/cockroachdb/cockroach/22571#-L59S2YLFwxjvq6QFic8:-L59XQdonmnYjrMnVXPT:b-p4ybu) ([raw file](https://github.com/cockroachdb/cockroach/blob/808f7e9d52a2a072534e05f4241c9784aea22659/pkg/sql/executor.go#L2085)):*\n<details><summary><i>Previously, rytaft wrote…</i></summary><blockquote>\n\nYou're running this start timer command twice\n</blockquote></details>\n\nOops, fixed.\n\n---\n\n*[pkg/sql/executor_opt_interface.go, line 168 at r6](https://reviewable.io:443/reviews/cockroachdb/cockroach/22571#-L59PMjc5l_QC8xSV_RR:-L59XYGsnpOgQ-s-2kHd:b-896fix) ([raw file](https://github.com/cockroachdb/cockroach/blob/001c9a577f27b8875f0c065539b5a395baadc40a/pkg/sql/executor_opt_interface.go#L168)):*\n<details><summary><i>Previously, rytaft wrote…</i></summary><blockquote>\n\n`opt.Catalog` -> `optbase.Catalog`\n</blockquote></details>\n\nDone.\n\n---\n\n*[pkg/sql/opt/optbuilder/builder.go, line 174 at r8](https://reviewable.io:443/reviews/cockroachdb/cockroach/22571#-L59U5yLVwCBTKneZOsn:-L59XYlavRh9af3ukNOS:benz9lx) ([raw file](https://github.com/cockroachdb/cockroach/blob/808f7e9d52a2a072534e05f4241c9784aea22659/pkg/sql/opt/optbuilder/builder.go#L174)):*\n<details><summary><i>Previously, rytaft wrote…</i></summary><blockquote>\n\nJust so I know for the future- why is this better than `type builderError error`?\n</blockquote></details>\n\n`error` is an interface, so `builderError` was, for all intents and purposes, the same interface (everything that implements `error` also implements `builderError` and vice-versa)\n\n---\n\n*[pkg/sql/sessiondata/session_data.go, line 132 at r8](https://reviewable.io:443/reviews/cockroachdb/cockroach/22571#-L59UOH2KB1V1WbXuXfh:-L59Xk9BecYYwn7OTRWI:b-896fix) ([raw file](https://github.com/cockroachdb/cockroach/blob/808f7e9d52a2a072534e05f4241c9784aea22659/pkg/sql/sessiondata/session_data.go#L132)):*\n<details><summary><i>Previously, rytaft wrote…</i></summary><blockquote>\n\nFix comment - it's not about DistSQL\n</blockquote></details>\n\nDone.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/cockroachdb/cockroach/22571#-:-L59Yitx1oIjPjouz6uy:b-814g8k)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"RaduBerinde"}}