Skip to content
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.

Sign up for GitHub

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

Jump to bottom

opt: support all DML statements in CBO and deprecate DML part of HP #34848

Closed
7 tasks done
awoods187 opened this issue Feb 13, 2019 · 6 comments
Closed
7 tasks done

opt: support all DML statements in CBO and deprecate DML part of HP #34848

awoods187 opened this issue Feb 13, 2019 · 6 comments
Assignees
@RaduBerinde
Labels
A-sql-optimizer SQL logical planning and optimizations. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) X-anchored-telemetry The issue number is anchored by telemetry references.
Milestone
19.2

Comments

@awoods187
Copy link
Contributor

awoods187 commented Feb 13, 2019 •
edited by RaduBerinde
Loading

Remaining features that need to be implemented before we retire HP support for DML statements:
@awoods187 awoods187 added A-sql-optimizer SQL logical planning and optimizations. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) labels Mar 5, 2019
@knz knz added the X-anchored-telemetry The issue number is anchored by telemetry references. label Mar 13, 2019
@knz knz added this to the 19.2 milestone Mar 13, 2019
@RaduBerinde RaduBerinde changed the title opt: support remaining features in CBO opt: support all DML statements in CBO Apr 11, 2019
@RaduBerinde RaduBerinde self-assigned this Apr 11, 2019
@RaduBerinde
Copy link
Member

CC @rytaft @justinj could you take a look at all unimplemented error cases in the optbuilder and see if we missed anything? I only found the window functions and the view/sequence numeric refs.

@RaduBerinde
Copy link
Member

@awoods187 I repurposed the issue a bit to focus on removing the HP for DML and relational expressions inside other statements.

@rytaft
Copy link
Collaborator

rytaft commented Apr 15, 2019 •
edited
Loading

I ran the logic tests and printed out errors for all statements that were not supported by the optimizer but executed successfully with the HP:

unimplemented: arrays of jsonb not allowed
unimplemented: unsupported statement: *tree.AlterTable
unimplemented: unsupported statement: *tree.CreateIndex
unimplemented: unsupported statement: *tree.DropTable
unimplemented: unsupported statement: *tree.RenameDatabase
unimplemented: unsupported statement: *tree.RenameIndex
unimplemented: unsupported statement: *tree.RenameTable
unimplemented: unsupported statement: *tree.ShowClusterSetting
unimplemented: unsupported statement: *tree.ShowColumns
unimplemented: unsupported statement: *tree.ShowConstraints
unimplemented: unsupported statement: *tree.ShowCreate
unimplemented: unsupported statement: *tree.ShowDatabases
unimplemented: unsupported statement: *tree.ShowGrants
unimplemented: unsupported statement: *tree.ShowIndex
unimplemented: unsupported statement: *tree.ShowJobs
unimplemented: unsupported statement: *tree.ShowQueries
unimplemented: unsupported statement: *tree.ShowRanges
unimplemented: unsupported statement: *tree.ShowSchemas
unimplemented: unsupported statement: *tree.ShowSequences
unimplemented: unsupported statement: *tree.ShowSessions
unimplemented: unsupported statement: *tree.ShowSyntax
unimplemented: unsupported statement: *tree.ShowTableStats
unimplemented: unsupported statement: *tree.ShowTables
unimplemented: unsupported statement: *tree.ShowTransactionStatus
unimplemented: unsupported statement: *tree.ShowUsers
unimplemented: unsupported statement: *tree.ShowVar
unimplemented: unsupported statement: *tree.ShowZoneConfig
unimplemented: unsupported statement: *tree.Truncate
unimplemented: window functions are not supported
unsupported statement: *tree.AlterSequence
unsupported statement: *tree.AlterTable
unsupported statement: *tree.AlterUserSetPassword
unsupported statement: *tree.CancelQueries
unsupported statement: *tree.CancelSessions
unsupported statement: *tree.CommentOnColumn
unsupported statement: *tree.CommentOnTable
unsupported statement: *tree.ControlJobs
unsupported statement: *tree.CreateDatabase
unsupported statement: *tree.CreateIndex
unsupported statement: *tree.CreateSequence
unsupported statement: *tree.CreateStats
unsupported statement: *tree.CreateUser
unsupported statement: *tree.CreateView
unsupported statement: *tree.Deallocate
unsupported statement: *tree.Discard
unsupported statement: *tree.DropDatabase
unsupported statement: *tree.DropIndex
unsupported statement: *tree.DropSequence
unsupported statement: *tree.DropTable
unsupported statement: *tree.DropUser
unsupported statement: *tree.DropView
unsupported statement: *tree.Grant
unsupported statement: *tree.Relocate
unsupported statement: *tree.RenameDatabase
unsupported statement: *tree.RenameIndex
unsupported statement: *tree.RenameTable
unsupported statement: *tree.Revoke
unsupported statement: *tree.Scatter
unsupported statement: *tree.Scrub
unsupported statement: *tree.SetClusterSetting
unsupported statement: *tree.SetSessionCharacteristics
unsupported statement: *tree.SetTransaction
unsupported statement: *tree.SetVar
unsupported statement: *tree.SetZoneConfig
unsupported statement: *tree.ShowClusterSetting
unsupported statement: *tree.ShowColumns
unsupported statement: *tree.ShowConstraints
unsupported statement: *tree.ShowCreate
unsupported statement: *tree.ShowDatabases
unsupported statement: *tree.ShowFingerprints
unsupported statement: *tree.ShowGrants
unsupported statement: *tree.ShowIndex
unsupported statement: *tree.ShowRoleGrants
unsupported statement: *tree.ShowRoles
unsupported statement: *tree.ShowSchemas
unsupported statement: *tree.ShowSequences
unsupported statement: *tree.ShowTableStats
unsupported statement: *tree.ShowTables
unsupported statement: *tree.ShowUsers
unsupported statement: *tree.ShowVar
unsupported statement: *tree.Split
unsupported statement: *tree.Truncate

I think we may want to support some of the SHOW commands, since those can be used inside DML queries. Seems like we also still need to support arrays of JSON in addition to window functions. Perhaps also some of the non-DDL statements like Scatter, Scrub, Split and Relocate.

@RaduBerinde
Copy link
Member

Thanks @rytaft! Yeah I agree on the SHOW commands, they fall under "Statements that can be used as a data source for a SELECT". I added a bullet for the Scatter, etc.

@RaduBerinde
Copy link
Member

I looked through the grammar.

A few other statements that incorporate a select clause: CANCEL JOBS, CANCEL QUERIES, CANCEL SESSIONS, PAUSE JOBS, RESUME JOBS.

Statements that can be SELECTed from (used as a data source): Backup, CreateChangeFeed, CancelQueries, CancelSessions, Export, Import, Relocate, Restore, Scatter, Scrub, Split, SHOW variations.

@RaduBerinde RaduBerinde changed the title opt: support all DML statements in CBO opt: support all DML statements in CBO and deprecate DML part of HP Apr 18, 2019
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Apr 21, 2019
@RaduBerinde
sql: restrict statements that can be used as row sources
c72e9bd 
We currently support all preparable statements as row sources using
the `SELECT FROM [ ... ]` syntax. Many of these statements are not
really useful here and each one will require work when we deprecate
the heuristic planner.

This change restricts the set of statements that can be used as row
sources to DML statements, SHOW statements, and EXPLAIN. There were no
specific tests for this functionality for any of the statements that
were removed. If we find that we need any of them, we can add them
back on a case-by-case basis.

Release note (sql change): Only SELECT, INSERT, UPDATE, UPSERT,
DELETE, SHOW, EXPLAIN are supported as data sources using the `SELECT
...  FROM [ ... ]` syntax.

Informs cockroachdb#34848.
@RaduBerinde RaduBerinde mentioned this issue Apr 21, 2019
Merged
craig bot pushed a commit that referenced this issue Apr 22, 2019
@RaduBerinde
Merge #36977
4e23bc0 
36977: sql: restrict statements that can be used as row sources r=RaduBerinde a=RaduBerinde

We currently support all preparable statements as row sources using
the `SELECT FROM [ ... ]` syntax. Many of these statements are not
really useful here and each one will require work when we deprecate
the heuristic planner.

This change restricts the set of statements that can be used as row
sources to DML statements, SHOW statements, and EXPLAIN. There were no
specific tests for this functionality for any of the statements that
were removed. If we find that we need any of them, we can add them
back on a case-by-case basis.

Release note (sql change): Only SELECT, INSERT, UPDATE, UPSERT,
DELETE, SHOW, EXPLAIN are supported as data sources using the `SELECT
...  FROM [ ... ]` syntax.

Informs #34848.

Co-authored-by: Radu Berinde <[email protected]>
@RaduBerinde
Copy link
Member

Statements that can be SELECTed from (used as a data source): Backup, CreateChangeFeed, CancelQueries, CancelSessions, Export, Import, Relocate, Restore, Scatter, Scrub, Split, SHOW variations.

It turns out that we don't care about most of these, and they were supported just as a consequence of how the grammar was written. #36977 reduces this list to SHOW variations.

@RaduBerinde RaduBerinde mentioned this issue Apr 24, 2019
Closed
@RaduBerinde RaduBerinde mentioned this issue Jun 26, 2019
Merged
craig bot pushed a commit that referenced this issue Jul 1, 2019
@RaduBerinde
Merge #38445
701719b 
38445: opt: opaque relational operator r=RaduBerinde a=RaduBerinde

#### optgen: panic early on unknown type

I had a case where adding an unknown type to a private resulted in a
cryptic segmentation violation. Normally an unknown type would
generate an error in the validator but in this case we don't get that
far.

This change makes optgen panic immediately when it encounters an
unknown type.

Release note: None

#### opt: opaque relational operator

We introduce `OpaqueRel`: a relational operator which contains a plan
generated outside of the optimizer.

External code registers an "opaque handler" for a specific AST node
type; the opaque handler generates the plan and returns it as "opaque
metadata". The opaque metadata is later passed to the exec factory.

In this change we add opaque handlers for all remaining SHOW variants
which use a custom planNode (and thus cannot be `delegate`d).

Informs #34848.

Release note: None


Co-authored-by: Radu Berinde <[email protected]>
@RaduBerinde RaduBerinde mentioned this issue Jul 1, 2019
Merged
craig bot pushed a commit that referenced this issue Jul 2, 2019
@RaduBerinde
Merge #38585
c5bbcbd 
38585: opt: add support for ALTER TABLE/INDEX SPLIT AT r=RaduBerinde a=RaduBerinde

#### opt: return index ordinal from ResolveTableIndex

Release note: None

#### optbuilder: add wrapper for building typed scalar

Release note: None

#### opt: add support for ALTER TABLE/INDEX SPLIT AT

Adding optimizer support for `tree.Split`. This statement has a
relational input, so it can't be implemented as an "opaque".

Release note: None

Informs #34848.

Co-authored-by: Radu Berinde <[email protected]>
@RaduBerinde RaduBerinde mentioned this issue Jul 3, 2019
Merged
craig bot pushed a commit that referenced this issue Jul 4, 2019
@RaduBerinde
Merge #38653
a2f104c 
38653: opt: support ALTER TABLE UNSPLIT and EXPERIMENTAL_RELOCATE r=RaduBerinde a=RaduBerinde

#### opt: support ALTER TABLE UNSPLIT

Support for ALTER TABLE UNSPLIT (which has two variants).

Release note: None

#### opt: support EXPERIMENTAL_RELOCATE

Also fix the formatting of details for split/unsplit.

Release note: None


Informs #34848.

Co-authored-by: Radu Berinde <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@RaduBerinde RaduBerinde

Labels
A-sql-optimizer SQL logical planning and optimizations. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) X-anchored-telemetry The issue number is anchored by telemetry references.
Projects
None yet
Milestone
19.2
Development

No branches or pull requests
4 participants
@knz @rytaft @RaduBerinde @awoods187