Skip to content

[SPARK-8232][SQL] Add sort_array support #7581

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
chenghao-intel wants to merge 5 commits into from
Closed

[SPARK-8232][SQL] Add sort_array support #7581

chenghao-intel wants to merge 5 commits into from

Conversation

@chenghao-intel
Copy link
Contributor

@chenghao-intel chenghao-intel commented Jul 22, 2015

Add expression sort_array support.

chenghao-intel
chenghao-intel reviewed Jul 22, 2015
View reviewed changes
...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala Outdated
Copy link
Contributor Author

@chenghao-intel chenghao-intel Jul 22, 2015

Choose a reason for hiding this comment

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

Use CodegenFallback,as the sorting in codegen is quite complex, as we need to generate the java code to access the catalyst data type. (e.g. Seq)

@SparkQA
Copy link

SparkQA commented Jul 22, 2015

Test build #38018 has finished for PR 7581 at commit f7974ce.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class SortArray(child: Expression)

@chenghao-intel
Copy link
Contributor Author

chenghao-intel commented Jul 22, 2015

cc @rxin

@chenghao-intel
Copy link
Contributor Author

chenghao-intel commented Jul 23, 2015

@rxin, can you review this for me?

davies
davies reviewed Jul 24, 2015
View reviewed changes
python/pyspark/sql/functions.py Outdated
Copy link
Contributor

@davies davies Jul 24, 2015

Choose a reason for hiding this comment

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

add a new line here, or the parameter can not be rendered correctly in doc.

@SparkQA
Copy link

SparkQA commented Jul 27, 2015

Test build #38489 has finished for PR 7581 at commit ceb8a73.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class SortArray(child: Expression)

davies
davies reviewed Jul 28, 2015
View reviewed changes
...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala Outdated
Copy link
Contributor

@davies davies Jul 28, 2015

Choose a reason for hiding this comment

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

Once we have checkInputDataTypes, do we still need ExpectsInputTypes?

Copy link
Contributor Author

@chenghao-intel chenghao-intel Jul 29, 2015

Choose a reason for hiding this comment

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

Yes, we probably need it, as the ExpectsInputTypes does not support the input type as ArrayType(AtomicType, _)

@SparkQA
Copy link

SparkQA commented Jul 29, 2015

Test build #38798 has finished for PR 7581 at commit c725f6f.

  • This patch fails Python style tests.
  • This patch does not merge cleanly.
  • This patch adds the following public classes (experimental):
    • case class SortArray(base: Expression, ascendingOrder: Expression)

@SparkQA
Copy link

SparkQA commented Jul 29, 2015

Test build #38802 has finished for PR 7581 at commit 384e373.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class SortArray(base: Expression, ascendingOrder: Expression)

@chenghao-intel
Copy link
Contributor Author

chenghao-intel commented Jul 30, 2015

@davies any more comments?

davies
davies reviewed Jul 30, 2015
View reviewed changes
...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala Outdated
Copy link
Contributor

@davies davies Jul 30, 2015

Choose a reason for hiding this comment

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

Should this be false?

Copy link
Contributor Author

@chenghao-intel chenghao-intel Jul 30, 2015

Choose a reason for hiding this comment

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

Good catch.

@SparkQA
Copy link

SparkQA commented Jul 30, 2015

Test build #39033 has finished for PR 7581 at commit 25fbede.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class SortArray(base: Expression, ascendingOrder: Expression)

@chenghao-intel
Copy link
Contributor Author

chenghao-intel commented Jul 31, 2015

Thank you @davies I've updated the code by using the ArrayData.

@SparkQA
Copy link

SparkQA commented Jul 31, 2015

Test build #39163 has finished for PR 7581 at commit 664c960.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class SortArray(base: Expression, ascendingOrder: Expression)

@davies
Copy link
Contributor

davies commented Aug 1, 2015

LGTM, will fix the conflict while merging.

@davies
Copy link
Contributor

davies commented Aug 1, 2015

Merged into master, thanks!

@asfgit asfgit closed this in 67ad4e2 Aug 1, 2015
@davies
Copy link
Contributor

davies commented Aug 1, 2015

There are logical conflicts, reverted, will fix conflict again.

@davies davies mentioned this pull request Aug 1, 2015
Closed
asfgit pushed a commit that referenced this pull request Aug 1, 2015
@chenghao-intel @davies
[SPARK-8232] [SQL] Add sort_array support
cf6c9ca 
This PR is based on #7581 , just fix the conflict.

Author: Cheng Hao <hao.cheng@intel.com>
Author: Davies Liu <davies@databricks.com>

Closes #7851 from davies/sort_array and squashes the following commits:

a80ef66 [Davies Liu] fix conflict
7cfda65 [Davies Liu] Merge branch 'master' of github.com:apache/spark into sort_array
664c960 [Cheng Hao] update the sort_array by using the ArrayData
276d2d5 [Cheng Hao] add empty line
0edab9c [Cheng Hao] Add asending/descending support for sort_array
80fc0f8 [Cheng Hao] Add type checking
a42b678 [Cheng Hao] Add sort_array support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chenghao-intel @SparkQA @davies