Skip to content

[SPARK-23602][SQL] PrintToStderr prints value also in interpreted mode#20773

Closed
mgaido91 wants to merge 3 commits intoapache:masterfrom
mgaido91:SPARK-23602
Closed

[SPARK-23602][SQL] PrintToStderr prints value also in interpreted mode#20773
mgaido91 wants to merge 3 commits intoapache:masterfrom
mgaido91:SPARK-23602

Conversation

@mgaido91
Copy link
Contributor

@mgaido91 mgaido91 commented Mar 8, 2018

What changes were proposed in this pull request?

PrintToStderr was doing what is it supposed to only when code generation is enabled.
The PR adds the same behavior in interpreted mode too.

How was this patch tested?

added UT


test("PrintToStderr") {
val errorStream = new java.io.ByteArrayOutputStream()
val systemErr = System.err
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you wrap setting the stderr in a try { ... } finally { ... }?

val outputEval = errorStream.toString
errorStream.reset()
// check with codegen
checkEvaluationWithoutCodegen(PrintToStderr(inputExpr), 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

checkEvaluationWithoutCodegen? Shouldn't we use checkEvaluationWithUnsafeProjection or checkEvaluationWithGeneratedMutableProjection?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, sorry, my bad

Copy link
Contributor

@hvanhovell hvanhovell left a comment

Choose a reason for hiding this comment

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

LGTM - pending jenkins.

@SparkQA
Copy link

SparkQA commented Mar 8, 2018

Test build #88095 has finished for PR 20773 at commit 9eaabf0.

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

@hvanhovell
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Mar 8, 2018

Test build #88096 has finished for PR 20773 at commit 208db42.

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

@SparkQA
Copy link

SparkQA commented Mar 8, 2018

Test build #88098 has finished for PR 20773 at commit 15262aa.

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

@hvanhovell
Copy link
Contributor

Merging to master. Thanks!

@asfgit asfgit closed this in e7bbca8 Mar 8, 2018
@SparkQA
Copy link

SparkQA commented Mar 8, 2018

Test build #88102 has finished for PR 20773 at commit 15262aa.

  • This patch passes all 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.

3 participants