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.

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

sql: ensure sub-plans of CREATE TABLE AS and CREATE VIEW are always closed #10527

Merged
merged 2 commits into from
Nov 8, 2016

Conversation

knz
Copy link
Contributor

@knz knz commented Nov 7, 2016

Addresses an issue found while investigating #10466:

#10466 (comment)


This change is Reviewable

@a-robinson
Copy link
Contributor

:lgtm: other than one question


Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


pkg/sql/create.go, line 634 at r1 (raw file):

      n.sourcePlan = nil

      desiredTypesFromSelect := make([]parser.Type, len(resultColumns))

Is removing this logic related? Why is it not needed anymore? It might deserve a callout in the commit message (or a separate commit if it is indeed unrelated)


Comments from Reviewable

knz added 2 commits November 8, 2016 10:06
Prior to this patch, the code for CREATE AS would give the column
signature of the source table as "desired type" for the internal
insertNode it uses to populate the new table. This is conceptually
problematic, as the source table is the source to the insertNode, and
not its consumer. It is also entirely ineffectual in this case,
because the internal insertNode used by CREATE TABLE AS does not use a
RETURNING clause and thus cannot used the "desired types" provided by
its context.

This patch removes this connection.
@knz
Copy link
Contributor Author

knz commented Nov 8, 2016

TFYR!


Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.


pkg/sql/create.go, line 634 at r1 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

Is removing this logic related? Why is it not needed anymore? It might deserve a callout in the commit message (or a separate commit if it is indeed unrelated)

Done.

Comments from Reviewable

@knz knz merged commit 4e9b3ec into cockroachdb:master Nov 8, 2016
@knz knz deleted the fix-create2 branch November 8, 2016 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants