Skip to content

[MINOR][SQL]Change ThreadLocal.withInitial to Scala lambda syntax#23295

Closed
10110346 wants to merge 1 commit intoapache:masterfrom
10110346:lambda_threadlocal
Closed

[MINOR][SQL]Change ThreadLocal.withInitial to Scala lambda syntax#23295
10110346 wants to merge 1 commit intoapache:masterfrom
10110346:lambda_threadlocal

Conversation

@10110346
Copy link
Contributor

@10110346 10110346 commented Dec 12, 2018

What changes were proposed in this pull request?

Currently, Scala has updated to Scala 2.12.8 , so we can change this to Scala lambda syntax

How was this patch tested?

Existing tests.
And Manual tested:

== Physical Plan ==
*(3) HashAggregate(keys=[attribute#237L], functions=[sum(cnt#227L)])
+- Exchange hashpartitioning(attribute#237L, 5)
   +- *(2) HashAggregate(keys=[attribute#237L], functions=[partial_sum(cnt#227L)])
      +- *(2) HashAggregate(keys=[nested#223.attribute#243L], functions=[count(1)])
         +- Exchange hashpartitioning(nested#223.attribute#243L, 5)
            +- *(1) HashAggregate(keys=[nested#223.attribute AS nested#223.attribute#243L], functions=[partial_count(1)])
               +- *(1) Project [nested#223]
                  +- *(1) Scan ExistingRDD[nested#223,value#224L]

@SparkQA
Copy link

SparkQA commented Dec 12, 2018

Test build #100003 has finished for PR 23295 at commit 361ed5c.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Dec 12, 2018

Test build #100014 has finished for PR 23295 at commit 361ed5c.

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

@10110346
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Dec 13, 2018

Test build #100059 has finished for PR 23295 at commit 361ed5c.

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

@10110346
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Dec 13, 2018

Test build #100081 has finished for PR 23295 at commit 361ed5c.

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

@srowen
Copy link
Member

srowen commented Dec 13, 2018

We still build against 2.11 too. Unless you have built with 2.11 and this works, we can't merge this.

@10110346
Copy link
Contributor Author

The ThreadLocal.withInitial lambda syntax is supported in java1.8, and this is already used in ReadAheadInputStream class.

@srowen
Copy link
Member

srowen commented Dec 14, 2018

This isn't Java code though. It may well work, but it needs to be tested against 2.11. The pull request builder doesn't test this. Have you? I remember some odd issues with Scala's willingness to match closures to single-arg function args, and given this comment, I think we need to be sure first.

@10110346
Copy link
Contributor Author

10110346 commented Dec 14, 2018

I see ,I have a mistake, this is scala code, it can not work for scala 2.11
thanks @srowen, I will close this PR

@10110346 10110346 closed this Dec 14, 2018
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