You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by dilipbiswal <gi...@git.apache.org> on 2015/12/30 04:57:31 UTC
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
GitHub user dilipbiswal opened a pull request:
https://github.com/apache/spark/pull/10520
[SPARK-12558][SQL][WIP] AnalysisException when multiple functions applied in GROUP BY clause
@cloud-fan Can you please take a look ?
In this case, we are failing during check analysis while validating the aggregation expression. I have added a semanticEquals for HiveGenericUDF to fix this. Please let me know if this is the right way to address this issue.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dilipbiswal/spark spark-12558
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/10520.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #10520
----
commit e7a50b91fbdc52fabf85a043f437ff8c7ef2623d
Author: Dilip Biswal <db...@us.ibm.com>
Date: 2015-12-30T01:49:51Z
[SPARK-12558] AnalysisException when multiple functions applied in GROUP BY clause
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r49364948
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveShim.scala ---
@@ -123,6 +126,24 @@ private[hive] object HiveShim {
// for Serialization
def this() = this(null)
+ override def hashCode(): Int = {
+ if (instance != null && instance.isInstanceOf[GenericUDFMacro]) {
--- End diff --
@yhuai Ah.. thanks a lot. I understand now that its not possible for users to extend the macro.I will make the change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-171138126
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/49275/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-167932923
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-170068731
@yhuai Hello Yin, did you have any further insights on this ? Is it safe to override equal to ignore the instance while comparing to see if two functions are same ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-171177410
Great. Thank you!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-169178261
@yhuai Yes Yin.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-169177927
so, two `HiveFunctionWrapper`s equality check failed because they have different `instance`s?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by nongli <gi...@git.apache.org>.
Github user nongli commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r48587483
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala ---
@@ -233,6 +233,18 @@ private[hive] case class HiveGenericUDF(funcWrapper: HiveFunctionWrapper, childr
udfType != null && udfType.deterministic()
}
+ override def semanticEquals(other: Expression): Boolean = {
+ val eqClass = other.isInstanceOf[HiveGenericUDF] &&
+ funcWrapper.functionClassName ==
+ other.asInstanceOf[HiveGenericUDF].funcWrapper.functionClassName
+
+ val isEqual = eqClass && children.zip(other.asInstanceOf[HiveGenericUDF].children).forall {
--- End diff --
I think you need to check children.length == other.children.length.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-171116529
**[Test build #49275 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/49275/consoleFull)** for PR 10520 at commit [`41f6f02`](https://github.com/apache/spark/commit/41f6f026036cface22d21c60acc18531de0d47ba).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-169231145
@dilipbiswal Sorry. I do not think I understand the cause. We only set the `instance` at https://github.com/apache/spark/blob/master/sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala#L78 (for `GenericUDFMacro`)? In most of the case, it will be null. Looks like the equality check should not be the problem?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-171176382
LGTM. Merging to master and branch 1.6.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r49289367
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveShim.scala ---
@@ -123,6 +126,24 @@ private[hive] object HiveShim {
// for Serialization
def this() = this(null)
+ override def hashCode(): Int = {
+ if (instance != null && instance.isInstanceOf[GenericUDFMacro]) {
--- End diff --
Actually, should we check the class name?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-171113109
test this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-170878877
**[Test build #2371 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2371/consoleFull)** for PR 10520 at commit [`b1e2d13`](https://github.com/apache/spark/commit/b1e2d13b323d204a15be50b3afdb1d964120ee8a).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r48588602
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala ---
@@ -233,6 +233,18 @@ private[hive] case class HiveGenericUDF(funcWrapper: HiveFunctionWrapper, childr
udfType != null && udfType.deterministic()
}
+ override def semanticEquals(other: Expression): Boolean = {
--- End diff --
@nongli Thanks a lot for your feedback. In this case , we have two instances of HiveGenericUDF one from the grouping expression and other from aggregation expression. While doing a semantic equality between them , we unwrap the case class and fall through to the last case statement which does equality between two HiveFunctionWrapper(s) and is failing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r49219243
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveShim.scala ---
@@ -125,6 +125,12 @@ private[hive] object HiveShim {
// for Serialization
def this() = this(null)
+ override def equals(other: Any): Boolean = other match {
+ case a: HiveFunctionWrapper =>
+ functionClassName == a.functionClassName
--- End diff --
@dilipbiswal I feel it is general fine to compare the class name (it is what we do in 1.5. In 1.5, instance is not a part of the constructor). We added instance when we added the support of hive's GenericUDFMacro. I guess we can add a check to see if `GenericUDFMacro` is the class name. If so, we also check instance. Otherwise, we just use the class name. Also, I guess you need to override hashCode as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r49228847
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveShim.scala ---
@@ -125,6 +125,12 @@ private[hive] object HiveShim {
// for Serialization
def this() = this(null)
+ override def equals(other: Any): Boolean = other match {
+ case a: HiveFunctionWrapper =>
+ functionClassName == a.functionClassName
--- End diff --
@yhuai Thanks Yin. Will make the changes per your suggestion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r48590980
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala ---
@@ -233,6 +233,18 @@ private[hive] case class HiveGenericUDF(funcWrapper: HiveFunctionWrapper, childr
udfType != null && udfType.deterministic()
}
+ override def semanticEquals(other: Expression): Boolean = {
--- End diff --
@nongli @cloud-fan We could also override equals in HiveFunctionWrapper to just compare the functionClassName. Would this be a better option ? Please let me know.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r49289420
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveShim.scala ---
@@ -123,6 +126,24 @@ private[hive] object HiveShim {
// for Serialization
def this() = this(null)
+ override def hashCode(): Int = {
+ if (instance != null && instance.isInstanceOf[GenericUDFMacro]) {
--- End diff --
I mean when we check if this function wrapper is for a GenericUDFMacro, should we check the class name?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r48646831
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala ---
@@ -233,6 +233,18 @@ private[hive] case class HiveGenericUDF(funcWrapper: HiveFunctionWrapper, childr
udfType != null && udfType.deterministic()
}
+ override def semanticEquals(other: Expression): Boolean = {
--- End diff --
I prefer to override `HiveFunctionWrapper.equals`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-169170856
Can you explain more why the semanticEquals at https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala#L152 fails?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r49289326
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveShim.scala ---
@@ -123,6 +126,24 @@ private[hive] object HiveShim {
// for Serialization
def this() = this(null)
+ override def hashCode(): Int = {
+ if (instance != null && instance.isInstanceOf[GenericUDFMacro]) {
+ Objects.hashCode(functionClassName, instance)
+ } else {
+ functionClassName.hashCode()
+ }
--- End diff --
Seems we need to correct the format?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-169477260
@yhuai So the instance is authored through createFunction() while computing returnInspector.
https://github.com/apache/spark/blob/master/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveShim.scala#L198-#L201
So we think we need to cache the instance in this case as its not a simpleUDF. Let me know your thoughts.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r49359695
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveShim.scala ---
@@ -123,6 +126,24 @@ private[hive] object HiveShim {
// for Serialization
def this() = this(null)
+ override def hashCode(): Int = {
+ if (instance != null && instance.isInstanceOf[GenericUDFMacro]) {
--- End diff --
Hive's macro is kind of special. I do not think users can use a class that extends GenericUDFMacro (You can take a look at hive's function registry. Macro registration is handled by registerMacro.).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r48588613
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala ---
@@ -233,6 +233,18 @@ private[hive] case class HiveGenericUDF(funcWrapper: HiveFunctionWrapper, childr
udfType != null && udfType.deterministic()
}
+ override def semanticEquals(other: Expression): Boolean = {
+ val eqClass = other.isInstanceOf[HiveGenericUDF] &&
+ funcWrapper.functionClassName ==
+ other.asInstanceOf[HiveGenericUDF].funcWrapper.functionClassName
+
+ val isEqual = eqClass && children.zip(other.asInstanceOf[HiveGenericUDF].children).forall {
--- End diff --
@nongli Sure. Will make the change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/10520
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r49288690
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveShim.scala ---
@@ -125,6 +125,12 @@ private[hive] object HiveShim {
// for Serialization
def this() = this(null)
+ override def equals(other: Any): Boolean = other match {
+ case a: HiveFunctionWrapper =>
+ functionClassName == a.functionClassName
--- End diff --
@yhuai Have addressed your comments. Can you please take a look to see if it looks ok ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-169451087
@yhuai Hi Yin, i remember that in this reproduction, the instances were actually having values i.e they were not null. I will double check on this and get back.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by nongli <gi...@git.apache.org>.
Github user nongli commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r48587428
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala ---
@@ -233,6 +233,18 @@ private[hive] case class HiveGenericUDF(funcWrapper: HiveFunctionWrapper, childr
udfType != null && udfType.deterministic()
}
+ override def semanticEquals(other: Expression): Boolean = {
--- End diff --
Can you comment why the version in Expression doesn't work? What about it is too strict?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-169450366
@yhuai Hi Yin, let me double check. As far as i remember, we come to the last case in expression.scala:semanticEqual with two HiveFunctionWrapper and was failing in equal. With a overriden equal it worked. Let me debug this further and get back...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-171137414
**[Test build #49275 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/49275/consoleFull)** for PR 10520 at commit [`41f6f02`](https://github.com/apache/spark/commit/41f6f026036cface22d21c60acc18531de0d47ba).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r49553587
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala ---
@@ -350,6 +350,13 @@ class HiveUDFSuite extends QueryTest with TestHiveSingleton with SQLTestUtils {
sqlContext.dropTempTable("testUDF")
}
+ test("Hive UDF in group by") {
+ Seq(Tuple1(1451400761)).toDF("test_date").registerTempTable("tab1")
+ val count = sql("select date(cast(test_date as timestamp))" +
+ " from tab1 group by date(cast(test_date as timestamp))").count()
--- End diff --
Actually it will be good to use withTempTable, which will automatically drop the temp table. Also, it will be more robust if we create a temp function based on hive's `org.apache.hadoop.hive.ql.udf.generic.GenericUDFToDate`. So, even if we later have a native function called date, we can still test the hive udf. We can do the test change in a follow-up pr (the pr can re-use the jira number).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-171176359
Actually, let me change the title while merging it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-171176251
Can you remove the `WIP` from the title?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-171177077
@yhuai Thank you very much !!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-171138123
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-170879414
**[Test build #2371 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2371/consoleFull)** for PR 10520 at commit [`b1e2d13`](https://github.com/apache/spark/commit/b1e2d13b323d204a15be50b3afdb1d964120ee8a).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-171177209
@yhuai I will make the change in a follow up PR re-using the same JIRA.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r48817507
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveShim.scala ---
@@ -125,6 +125,12 @@ private[hive] object HiveShim {
// for Serialization
def this() = this(null)
+ override def equals(other: Any): Boolean = other match {
+ case a: HiveFunctionWrapper =>
+ functionClassName == a.functionClassName
--- End diff --
cc @yhuai , Does same function name means exactly same function in hive?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-171112636
@yhuai Hi Yin, Could you please look at the changes when you get a chance. It turns out that we can't compare the classname and instance for equality for GenericUDFMacro as the instances are different. I am instead comparing the underlying function that is held by GenericUDFMacro. Please let me know what you think.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r49553700
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala ---
@@ -350,6 +350,13 @@ class HiveUDFSuite extends QueryTest with TestHiveSingleton with SQLTestUtils {
sqlContext.dropTempTable("testUDF")
}
+ test("Hive UDF in group by") {
+ Seq(Tuple1(1451400761)).toDF("test_date").registerTempTable("tab1")
+ val count = sql("select date(cast(test_date as timestamp))" +
+ " from tab1 group by date(cast(test_date as timestamp))").count()
--- End diff --
I just realize it after I merge the PR. Let's have a pr to improve the test. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-169175945
@yhuai Hi Yin, Here is what i had while answering the question from Nong Li. Please let me know if you have more questions that i need to find the answers for.
In this case , we have two instances of HiveGenericUDF one from the grouping expression and other from aggregation expression. While doing a semantic equality between them , we unwrap the case class and fall through to the last case statement which does equality between two HiveFunctionWrapper(s) and is failing.
Let me know what you think.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on a diff in the pull request:
https://github.com/apache/spark/pull/10520#discussion_r49289994
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveShim.scala ---
@@ -123,6 +126,24 @@ private[hive] object HiveShim {
// for Serialization
def this() = this(null)
+ override def hashCode(): Int = {
+ if (instance != null && instance.isInstanceOf[GenericUDFMacro]) {
--- End diff --
@yhuai Thanks !! Could users have a class that extends GenericUDFMacro ? Wouldn't we want to handle that case as well ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12558][SQL][WIP] AnalysisException when...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:
https://github.com/apache/spark/pull/10520#issuecomment-168917085
@cloud-fan Hi Wenchen, thanks for your comments. I went through the processing. I think its ok to just override equals and compare the udf class names here. The number of arguments, their semantic equality and deterministic property is handled in the semanticEquals method in expression.scala. Please let me know what you think.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org