You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2016/05/07 07:04:06 UTC
[GitHub] spark pull request: [SPARK-15199] Disallow Dropping Build-in Funct...
GitHub user gatorsmile opened a pull request:
https://github.com/apache/spark/pull/12975
[SPARK-15199] Disallow Dropping Build-in Functions
#### What changes were proposed in this pull request?
As Hive and the major RDBMS behave, the built-in functions are not allowed to drop. In the current implementation, users can drop the built-in functions. However, after dropping the built-in functions, users are unable to add them back.
#### How was this patch tested?
Added a test case.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gatorsmile/spark dropBuildInFunction
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12975.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 #12975
----
commit 0c4b1a4b4be1e4f752313b60d484fa4cab8a1b35
Author: gatorsmile <ga...@gmail.com>
Date: 2016-05-07T06:57:17Z
initial fix.
----
---
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-15199] [SQL] Disallow Dropping Build-in...
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/12975#discussion_r62449710
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala ---
@@ -157,6 +157,9 @@ case class DropFunction(
throw new AnalysisException(s"Specifying a database in DROP TEMPORARY FUNCTION " +
s"is not allowed: '${databaseName.get}'")
}
+ if (FunctionRegistry.builtin.functionExists(functionName)) {
--- End diff --
what if users overwrite a built-in function and drop 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-15199] [SQL] Disallow Dropping Build-in...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12975
---
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-15199] [SQL] Disallow Dropping Build-in...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12975#issuecomment-217647786
**[Test build #58070 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58070/consoleFull)** for PR 12975 at commit [`d6e3e50`](https://github.com/apache/spark/commit/d6e3e50ada66f4081b606b766385299d54263399).
* 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-15199] [SQL] Disallow Dropping Build-in...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12975#discussion_r62454620
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala ---
@@ -157,6 +157,9 @@ case class DropFunction(
throw new AnalysisException(s"Specifying a database in DROP TEMPORARY FUNCTION " +
s"is not allowed: '${databaseName.get}'")
}
+ if (FunctionRegistry.builtin.functionExists(functionName)) {
--- End diff --
No problem. Just let me know if anything I can help. : )
---
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-15199] [SQL] Disallow Dropping Build-in...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/12975#issuecomment-217936896
LGTM merging into master 2.0. I confirmed with Yin offline.
---
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-15199] [SQL] Disallow Dropping Build-in...
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/12975#discussion_r62450713
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala ---
@@ -157,6 +157,9 @@ case class DropFunction(
throw new AnalysisException(s"Specifying a database in DROP TEMPORARY FUNCTION " +
s"is not allowed: '${databaseName.get}'")
}
+ if (FunctionRegistry.builtin.functionExists(functionName)) {
--- End diff --
Does hive allow users to overwrite built-in function?
---
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-15199] [SQL] Disallow Dropping Build-in...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12975#issuecomment-217647832
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58070/
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-15199] [SQL] Disallow Dropping Build-in...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12975#discussion_r62451866
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala ---
@@ -157,6 +157,9 @@ case class DropFunction(
throw new AnalysisException(s"Specifying a database in DROP TEMPORARY FUNCTION " +
s"is not allowed: '${databaseName.get}'")
}
+ if (FunctionRegistry.builtin.functionExists(functionName)) {
--- End diff --
Yeah, you can overwrite it by creating a temporary function, but you are unable to drop it.
For example,
```
hive> drop function lower;
FAILED: SemanticException [Error 10301]: Cannot drop native function lower
hive> CREATE TEMPORARY FUNCTION lower AS 'org.apache.hadoop.hive.ql.udf.generic.GenericUDFUpper';
OK
Time taken: 0.005 seconds
hive> select lower('a');
OK
A
Time taken: 0.068 seconds, Fetched: 1 row(s)
hive> drop temporary function lower;
OK
Time taken: 0.01 seconds
hive> select lower('a');
OK
a
Time taken: 0.057 seconds, Fetched: 1 row(s)
hive> drop function lower;
FAILED: SemanticException [Error 10301]: Cannot drop native function lower
```
In this example, I overwrite the built-in function `lower` by creating the same name temporary function by the implementation of `upper`. Obviously, when I called `lower`, the actual logic is `upper`. That means, we can overwrite the built-in functions. After I dropping the temporary function, the built-in function becomes active again.
---
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-15199] [SQL] Disallow Dropping Build-in...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12975#issuecomment-217612557
**[Test build #58060 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58060/consoleFull)** for PR 12975 at commit [`0c4b1a4`](https://github.com/apache/spark/commit/0c4b1a4b4be1e4f752313b60d484fa4cab8a1b35).
---
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-15199] [SQL] Disallow Dropping Build-in...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12975#issuecomment-217642015
**[Test build #58070 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58070/consoleFull)** for PR 12975 at commit [`d6e3e50`](https://github.com/apache/spark/commit/d6e3e50ada66f4081b606b766385299d54263399).
---
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-15199] [SQL] Disallow Dropping Build-in...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12975#discussion_r62449958
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala ---
@@ -157,6 +157,9 @@ case class DropFunction(
throw new AnalysisException(s"Specifying a database in DROP TEMPORARY FUNCTION " +
s"is not allowed: '${databaseName.get}'")
}
+ if (FunctionRegistry.builtin.functionExists(functionName)) {
--- End diff --
I also think what you said is valid. If so, I think we also need to provide users a way to recover the built-in function, if they dropped it for any purpose.
This PR is just to make our behavior consistent with Hive and the mainstream RDBMS. Normally, we do not allow users to drop the built-in functions. I am fine if we allow users to drop the built-in functions.
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-15199] [SQL] Disallow Dropping Build-in...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12975#issuecomment-217619800
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-15199] [SQL] Disallow Dropping Build-in...
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/12975#discussion_r62454308
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala ---
@@ -157,6 +157,9 @@ case class DropFunction(
throw new AnalysisException(s"Specifying a database in DROP TEMPORARY FUNCTION " +
s"is not allowed: '${databaseName.get}'")
}
+ if (FunctionRegistry.builtin.functionExists(functionName)) {
--- End diff --
Thanks for the investigation! I guess we won't support this feature in 2.0, but need @yhuai to confirm.
---
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-15199] [SQL] Disallow Dropping Build-in...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12975#issuecomment-217647831
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-15199] [SQL] Disallow Dropping Build-in...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12975#issuecomment-217619469
**[Test build #58060 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58060/consoleFull)** for PR 12975 at commit [`0c4b1a4`](https://github.com/apache/spark/commit/0c4b1a4b4be1e4f752313b60d484fa4cab8a1b35).
* 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-15199] [SQL] Disallow Dropping Build-in...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12975#issuecomment-217619806
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58060/
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-15199] [SQL] Disallow Dropping Build-in...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12975#discussion_r62451961
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala ---
@@ -157,6 +157,9 @@ case class DropFunction(
throw new AnalysisException(s"Specifying a database in DROP TEMPORARY FUNCTION " +
s"is not allowed: '${databaseName.get}'")
}
+ if (FunctionRegistry.builtin.functionExists(functionName)) {
--- End diff --
Overwriting a built-in function is missing in the current implementation of the current `FunctionRegistry`. Should we support it in this release?
---
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-15199] [SQL] Disallow Dropping Build-in...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/12975#issuecomment-217737711
cc @yhuai @andrewor14 @cloud-fan
---
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