Skip to content

Conversation

@wzhfy
Copy link
Contributor

@wzhfy wzhfy commented Sep 13, 2017

What changes were proposed in this pull request?

Drop test tables and improve comments.

How was this patch tested?

Modified existing test.

@wzhfy
Copy link
Contributor Author

wzhfy commented Sep 13, 2017

cc @cloud-fan

@SparkQA
Copy link

SparkQA commented Sep 13, 2017

Test build #81698 has finished for PR 19213 at commit 0afc9f7.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

LGTM, please also address one more comment that drop testing tables.

@wzhfy wzhfy changed the title [SPARK-17642] [SQL] [FOLLOWUP] improve comments [SPARK-17642] [SQL] [FOLLOWUP] drop test tables and improve comments Sep 14, 2017
@SparkQA
Copy link

SparkQA commented Sep 14, 2017

Test build #81747 has finished for PR 19213 at commit d922c85.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

-- Describe a non-existent column
DESC desc_col_temp_table key1;

DROP VIEW desc_col_temp_table;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it's a view, can we name it desc_col_temp_view?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and it's better to put all DROP commands at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. I'll fix it right now.

@SparkQA
Copy link

SparkQA commented Sep 14, 2017

Test build #81774 has finished for PR 19213 at commit a995edc.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@asfgit asfgit closed this in ddd7f5e Sep 14, 2017
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.

3 participants