Skip to content
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.

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

[SPARK-18134][SQL] Comparable MapTypes [POC] #15970

Closed
wants to merge 10 commits into from

Conversation

hvanhovell
Copy link
Contributor

What changes were proposed in this pull request?

This is a small POC to see if we can make MapType orderable, and thus usable in aggregates and joins. The key idea in this PR is that there is a difference between an unordered and an ordered map (an ordered map is can be compared), and that ordered is a property of MapType.

A map can be converted from an unordered map to an ordered map by injecting a SortMap expression. The analyzer will inject SortMap expressions whenever we use a map in a binary comparison and when we use it in an aggregate. Note that the SortMap expression is far from optimized, it should however perform reasonable.

How was this patch tested?

No tests yet. This will probably fail tests.

@hvanhovell
Copy link
Contributor Author

cc @cloud-fan we discussed something like this earlier today

@SparkQA
Copy link

SparkQA commented Nov 22, 2016

Test build #68968 has finished for PR 15970 at commit edec2d8.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class SortMap(child: Expression) extends UnaryExpression with ExpectsInputTypes

@SparkQA
Copy link

SparkQA commented Nov 22, 2016

Test build #69003 has finished for PR 15970 at commit b16ddb8.

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

# Conflicts:
#	sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
#	sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisErrorSuite.scala
@SparkQA
Copy link

SparkQA commented Nov 22, 2016

Test build #69023 has finished for PR 15970 at commit d570c84.

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

@SparkQA
Copy link

SparkQA commented Nov 24, 2016

Test build #69136 has finished for PR 15970 at commit e4cc4b0.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class ColumnStat(
  • case class UncacheTableCommand(

@SparkQA
Copy link

SparkQA commented Nov 25, 2016

Test build #69141 has finished for PR 15970 at commit 7d847b8.

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

@albertferras
Copy link

Any update on this?

@gatorsmile
Copy link
Member

cc @hvanhovell

@janewangfb
Copy link
Contributor

Is there any updates on this PR? This feature is quite useful to us.

@jinxing64
Copy link

@hvanhovell Are you still working on this? I think this is feature is useful :)

@maropu
Copy link
Member

maropu commented Jul 23, 2018

@hvanhovell We still need to keep this pr open? Either way, we need rework based on this pr. If so, can you close this for now?

@SparkQA
Copy link

SparkQA commented Oct 22, 2018

Test build #97829 has started for PR 15970 at commit 7d847b8.

@SparkQA
Copy link

SparkQA commented Oct 22, 2018

Test build #97847 has started for PR 15970 at commit 7d847b8.

@SparkQA
Copy link

SparkQA commented Oct 22, 2018

Test build #97859 has started for PR 15970 at commit 7d847b8.

@AmplabJenkins
Copy link

Build finished. Test FAILed.

@github-actions
Copy link

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants