[SPARK-5969][PySpark] Fix descending pyspark.rdd.sortByKey.

[foxik]

The samples should always be sorted in ascending order, because bisect.bisect_left is used on it. The reverse order of the result is already achieved in rangePartitioner by reversing the found index.

The current implementation also work, but always uses only two partitions -- the first one and the last one (because the bisect_left return returns either "beginning" or "end" for a descending sequence).

[AmplabJenkins]

Can one of the admins verify this patch?

[JoshRosen]

Could you add a regression test for this issue? It looks like you have one in the JIRA ticket, so adding one hopefully should not be much work. Take a look at python/pyspark/tests.py for examples of this. Make sure to add a comment referencing the JIRA number.

[foxik]

I added the regression test. It also tests that sortByKey returns sorted sequence and tests also ascending sequence, which are not strictly necessary for SPARK-5969, but I added them anyway.

[JoshRosen]

Jenkins, this is ok to test.

[SparkQA]

Test build #28315 has started for PR 4761 at commit bc2647f.

• This patch merges cleanly.

[SparkQA]

Test build #28315 has finished for PR 4761 at commit bc2647f.

• This patch fails Python style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28315/
Test FAILed.

[foxik]

I have amended the regression test commit to pass lint-python.

[SparkQA]

Test build #28334 has started for PR 4761 at commit 95896b5.

• This patch merges cleanly.

[SparkQA]

Test build #28334 has finished for PR 4761 at commit 95896b5.

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

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28334/
Test PASSed.

[JoshRosen]

@davies, does this look good to you? Sorry for letting this patch fall off my radar (slowly getting caught up on a backlog of reviews). If things look good, I can fix the merge conflict (which is probably just a conflict in tests) and get this committed.

[davies]

LGTM

[JoshRosen]

Alright, merging this into master (1.4.0) now. Thanks!

[JoshRosen]

Should this be backported anywhere?

[davies]

This is a bug since the beginning (0.8), could we back port it for all 1.0+ branches?

[JoshRosen]

I've cherry-picked the fix into branch-1.2 (1.2.3) and branch-1.3 (1.3.2). I'm going to omit the the pre-1.2 backports for now because I hit a test merge conflict and also because I think it's unlikely that we're going to release another 1.1.x version.

[davies]

Make sense, thank you!