Skip to content

Conversation

@ChenjunZou
Copy link

@ChenjunZou ChenjunZou commented Dec 13, 2017

What changes were proposed in this pull request?

since hive 2.0+ upgrades log4j to log4j2,a lot of changes are made working on it.
as spark is not to ready to update its inner hive version(1.2.1) , so I manage to make little changes.
the function registerCurrentOperationLog is moved from SQLOperstion to its parent class ExecuteStatementOperation so spark can use it.

How was this patch tested?

manual test

Closes #19721 from ChenjunZou/operation-log.

## What changes were proposed in this pull request?
since hive 2.0+  upgrades log4j to log4j2,a lot of [changes](https://issues.apache.org/jira/browse/HIVE-11304) are made working on it.
as spark is not to ready to update its inner hive version(1.2.1) , so I manage to make little changes.
the function registerCurrentOperationLog  is moved from SQLOperstion to its parent class ExecuteStatementOperation so spark can use it.

## How was this patch tested?
manual test

Author: zouchenjun <[email protected]>

Closes apache#19721 from ChenjunZou/operation-log.
@HyukjinKwon
Copy link
Member

ok to test

if (isOperationLogEnabled) {
if (operationLog == null) {
LOG.warn("Failed to get current OperationLog object of Operation: " +
getHandle().getHandleIdentifier());
Copy link
Member

Choose a reason for hiding this comment

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

I think the indentation should be double spaced.

Copy link
Member

Choose a reason for hiding this comment

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

This was resolved by me when I did the merge.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you @gatorsmile.

Copy link
Author

@ChenjunZou ChenjunZou Dec 15, 2017

Choose a reason for hiding this comment

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

@SparkQA
Copy link

SparkQA commented Dec 13, 2017

Test build #84825 has finished for PR 19961 at commit b39d36f.

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

@HyukjinKwon
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Dec 14, 2017

Test build #84933 has finished for PR 19961 at commit b39d36f.

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

@gatorsmile
Copy link
Member

gatorsmile commented Dec 14, 2017

LGTM

Thanks! Merged to master.

@asfgit asfgit closed this in 0ea2d8c Dec 14, 2017
@gatorsmile
Copy link
Member

gatorsmile commented Dec 15, 2017

Now, SparkR started failing, after we remerge this PR. It sounds like it is related to the merge of this PR. I have to revert it. Sorry for that.

@HyukjinKwon
Copy link
Member

It indeed looked somehow related as the CRAN was broken at the similar time. BTW, seems unrelated at the end. Let me bring it back.

@ChenjunZou ChenjunZou deleted the spark-22496 branch August 16, 2019 05:33
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.

4 participants