Skip to content

Conversation

@wzhfy
Copy link
Contributor

@wzhfy wzhfy commented May 8, 2017

What changes were proposed in this pull request?

There's no SQL tab in Spark UI since SQLExecution.withNewExecutionId was removed from def hiveResultString in this pr. We need to add it back except for insert command, because it starts a new execution through FileFormatWriter.

How was this patch tested?

Test by hand.

@wzhfy
Copy link
Contributor Author

wzhfy commented May 8, 2017

@cloud-fan

Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, sorry I forgot to delete it...

Copy link
Member

Choose a reason for hiding this comment

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

hiveResultString is for testing only

Copy link
Contributor Author

@wzhfy wzhfy May 8, 2017

Choose a reason for hiding this comment

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

it's called by SparkSQLDriver.run

Copy link
Member

Choose a reason for hiding this comment

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

Uh... I did not realize it...

@wzhfy wzhfy force-pushed the newExecutionId branch from 9502212 to 1adaf6e Compare May 8, 2017 07:41
}
}

trait InsertTableCommand extends RunnableCommand
Copy link
Member

Choose a reason for hiding this comment

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

This might be too broad for this purpose. It could be misunderstood later. Should we add a comment for it?

Copy link
Contributor Author

@wzhfy wzhfy May 8, 2017

Choose a reason for hiding this comment

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

OK I'll add a comment for it

@SparkQA
Copy link

SparkQA commented May 8, 2017

Test build #76567 has finished for PR 17897 at commit 1adaf6e.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@wzhfy
Copy link
Contributor Author

wzhfy commented May 8, 2017

retest this please

@cloud-fan
Copy link
Contributor

oh, seems it's included by #17540 ?

@wzhfy
Copy link
Contributor Author

wzhfy commented May 8, 2017

@cloud-fan OK, let me close this.

@wzhfy wzhfy closed this May 8, 2017
@SparkQA
Copy link

SparkQA commented May 8, 2017

Test build #76575 has finished for PR 17897 at commit 1adaf6e.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

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.

5 participants