You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by bogdanrdc <gi...@git.apache.org> on 2017/05/24 11:17:52 UTC
[GitHub] spark pull request #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
GitHub user bogdanrdc opened a pull request:
https://github.com/apache/spark/pull/18086
[SPARK-20854][SQL] Extend hint syntax to support expressions
## What changes were proposed in this pull request?
SQL hint syntax:
* support expressions such as strings, numbers, etc. instead of only identifiers as it is currently.
* support multiple hints, which was missing compared to the DataFrame syntax.
DataFrame API:
* support any parameters in DataFrame.hint instead of just strings
## How was this patch tested?
Existing tests. New tests in PlanParserSuite. New suite DataFrameHintSuite.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bogdanrdc/spark SPARK-20854
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18086.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 #18086
----
commit 03a4281751e02acd2b97ceff6cf8e1621e83eb93
Author: Bogdan Raducanu <bo...@databricks.com>
Date: 2017-04-20T10:59:49Z
fix + test
commit 72cf1d117890abe45aa30c6b91a7e2c527fc4969
Author: Bogdan Raducanu <bo...@databricks.com>
Date: 2017-04-20T11:01:40Z
reverted mistake commit
commit 2c96a8d65059db3b808e05241b870ccd17937095
Author: Bogdan Raducanu <bo...@databricks.com>
Date: 2017-05-12T15:24:57Z
erge remote-tracking branch 'upstream/master'
commit fa11b0b97b38bb98b599a8edf1d43e01b067a926
Author: Bogdan Raducanu <bo...@databricks.com>
Date: 2017-05-23T11:28:34Z
Merge remote-tracking branch 'upstream/master'
commit 21ad3aa4468b58aa4e552e2922e1bceda61097f7
Author: Bogdan Raducanu <bo...@databricks.com>
Date: 2017-05-23T11:28:57Z
Merge remote-tracking branch 'upstream/master'
commit 84c0746d3d71c5a7f7e99c427ab68b72871817a3
Author: Bogdan Raducanu <bo...@databricks.com>
Date: 2017-05-24T09:14:30Z
new syntax + tests
commit dff75c854a972bc2a61eb752971faa208f9ccb84
Author: Bogdan Raducanu <bo...@databricks.com>
Date: 2017-05-24T09:25:28Z
merged
commit 5439468f85c01dc2c5ae36a4c1a0fceb210d9852
Author: Bogdan Raducanu <bo...@databricks.com>
Date: 2017-05-24T11:11:57Z
multiple hints syntaxes + more tests
----
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by bogdanrdc <gi...@git.apache.org>.
Github user bogdanrdc commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r119561805
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala ---
@@ -407,7 +407,7 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
val withWindow = withDistinct.optionalMap(windows)(withWindows)
// Hint
- withWindow.optionalMap(hint)(withHints)
+ hints.asScala.foldRight(withWindow)(withHints)
--- End diff --
so that `select /*+ hint1() /* /*+ hint2() */`produces `Hint1(Hint2(plan))` and not `Hint2(Hint1(plan))`. `withHints` adds a Hint on top so the last one folded is the top most.
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r118832143
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -371,7 +371,7 @@ querySpecification
(RECORDREADER recordReader=STRING)?
fromClause?
(WHERE where=booleanExpression)?)
- | ((kind=SELECT hint? setQuantifier? namedExpressionSeq fromClause?
+ | ((kind=SELECT (hints+=hint)* setQuantifier? namedExpressionSeq fromClause?
--- End diff --
In Hive and Oracle, multiple hints are put in the 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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r118832415
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -381,12 +381,12 @@ querySpecification
;
hint
- : '/*+' hintStatement '*/'
+ : '/*+' hintStatements+=hintStatement (hintStatements+=hintStatement)* '*/'
--- End diff --
In the same block `/*+ */`, multiple hints are separated by commas in Hive. However, in Oracle, it is separated by spaces.
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18086
Merged build finished. Test FAILed.
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18086
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by wzhfy <gi...@git.apache.org>.
Github user wzhfy commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r119755764
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala ---
@@ -527,47 +527,117 @@ class PlanParserSuite extends PlanTest {
val m = intercept[ParseException] {
parsePlan("SELECT /*+ HINT() */ * FROM t")
}.getMessage
- assert(m.contains("no viable alternative at input"))
-
- // Hive compatibility: No database.
- val m2 = intercept[ParseException] {
- parsePlan("SELECT /*+ MAPJOIN(default.t) */ * from default.t")
- }.getMessage
- assert(m2.contains("mismatched input '.' expecting {')', ','}"))
+ assert(m.contains("mismatched input"))
// Disallow space as the delimiter.
val m3 = intercept[ParseException] {
parsePlan("SELECT /*+ INDEX(a b c) */ * from default.t")
}.getMessage
- assert(m3.contains("mismatched input 'b' expecting {')', ','}"))
+ assert(m3.contains("mismatched input 'b' expecting"))
comparePlans(
parsePlan("SELECT /*+ HINT */ * FROM t"),
UnresolvedHint("HINT", Seq.empty, table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ BROADCASTJOIN(u) */ * FROM t"),
- UnresolvedHint("BROADCASTJOIN", Seq("u"), table("t").select(star())))
+ UnresolvedHint("BROADCASTJOIN", Seq($"u"), table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ MAPJOIN(u) */ * FROM t"),
- UnresolvedHint("MAPJOIN", Seq("u"), table("t").select(star())))
+ UnresolvedHint("MAPJOIN", Seq($"u"), table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ STREAMTABLE(a,b,c) */ * FROM t"),
- UnresolvedHint("STREAMTABLE", Seq("a", "b", "c"), table("t").select(star())))
+ UnresolvedHint("STREAMTABLE", Seq($"a", $"b", $"c"), table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ INDEX(t, emp_job_ix) */ * FROM t"),
- UnresolvedHint("INDEX", Seq("t", "emp_job_ix"), table("t").select(star())))
+ UnresolvedHint("INDEX", Seq($"t", $"emp_job_ix"), table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ MAPJOIN(`default.t`) */ * from `default.t`"),
- UnresolvedHint("MAPJOIN", Seq("default.t"), table("default.t").select(star())))
+ UnresolvedHint("MAPJOIN", Seq(UnresolvedAttribute.quoted("default.t")),
+ table("default.t").select(star())))
comparePlans(
parsePlan("SELECT /*+ MAPJOIN(t) */ a from t where true group by a order by a"),
- UnresolvedHint("MAPJOIN", Seq("t"),
+ UnresolvedHint("MAPJOIN", Seq($"t"),
table("t").where(Literal(true)).groupBy('a)('a)).orderBy('a.asc))
}
+
+ test("SPARK-20854: select hint syntax with expressions") {
+ comparePlans(
+ parsePlan("SELECT /*+ HINT1(a, array(1, 2, 3)) */ * from t"),
+ UnresolvedHint("HINT1", Seq($"a",
+ UnresolvedFunction("array", Literal(1) :: Literal(2) :: Literal(3) :: Nil, false)),
+ table("t").select(star())
+ )
+ )
+
+ comparePlans(
+ parsePlan("SELECT /*+ HINT1(a, array(1, 2, 3)) */ * from t"),
--- End diff --
Is this test case redundant?
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
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/18086#discussion_r118467587
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala ---
@@ -533,13 +533,16 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
}
/**
- * Add a [[UnresolvedHint]] to a logical plan.
+ * Add a [[UnresolvedHint]]s to a logical plan.
*/
private def withHints(
ctx: HintContext,
query: LogicalPlan): LogicalPlan = withOrigin(ctx) {
- val stmt = ctx.hintStatement
- UnresolvedHint(stmt.hintName.getText, stmt.parameters.asScala.map(_.getText), query)
+ var plan = query
--- End diff --
using `foldLeft` instead of having a `var`?
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18086
**[Test build #77424 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77424/testReport)** for PR 18086 at commit [`6e40301`](https://github.com/apache/spark/commit/6e4030160fa12934dccacea59b06033fdc8ced80).
* This patch **fails Spark unit 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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
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/18086#discussion_r119267472
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameHintSuite.scala ---
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.spark.sql.catalyst.analysis.EliminateBarriers
+import org.apache.spark.sql.catalyst.plans.PlanTest
+import org.apache.spark.sql.catalyst.plans.logical._
+import org.apache.spark.sql.test.SharedSQLContext
+
+class DataFrameHintSuite extends PlanTest with SharedSQLContext {
+ import testImplicits._
+ lazy val df = spark.range(10)
+
+ private def check(df: Dataset[_], expected: LogicalPlan) = {
+ comparePlans(
+ EliminateBarriers(df.queryExecution.logical),
--- End diff --
you should match `df.logicalPlan`, which doesn't have barrier node.
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18086
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18086
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r118832527
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameHintSuite.scala ---
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
--- End diff --
Normally, we move such a test suite to `org.apache.spark.sql.catalyst`. We just need to add `hint` into `org.apache.spark.sql.catalyst.dsl`.
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18086
**[Test build #77531 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77531/testReport)** for PR 18086 at commit [`8daa05e`](https://github.com/apache/spark/commit/8daa05e1ec595c54f8aac7c041d5c6571a759aaf).
* 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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18086
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by bogdanrdc <gi...@git.apache.org>.
Github user bogdanrdc commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r119085351
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/hints.scala ---
@@ -25,7 +25,7 @@ import org.apache.spark.sql.internal.SQLConf
* should be removed This node will be eliminated post analysis.
* A pair of (name, parameters).
*/
-case class UnresolvedHint(name: String, parameters: Seq[String], child: LogicalPlan)
+case class UnresolvedHint(name: String, parameters: Seq[Any], child: LogicalPlan)
--- End diff --
I think that could be something extra. The DF API should accept scala expressions too: function calls (df.hint("hint", getInterestingValues()))
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by bogdanrdc <gi...@git.apache.org>.
Github user bogdanrdc commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r118939619
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/hints.scala ---
@@ -25,7 +25,7 @@ import org.apache.spark.sql.internal.SQLConf
* should be removed This node will be eliminated post analysis.
* A pair of (name, parameters).
*/
-case class UnresolvedHint(name: String, parameters: Seq[String], child: LogicalPlan)
+case class UnresolvedHint(name: String, parameters: Seq[Any], child: LogicalPlan)
--- End diff --
One useful hint parameter is a list of columns.
Something like `df.hint("hint", $"table", Seq($"col1", $"col2", $"col3"))`
In this case UnresolvedHint could be called like this:
```UnresolvedHint(name = String, parameters = Seq(Expression, Seq[Expression]), child)```
But if `UnresolvedHint.parameters` is `Seq[Expression]` then it's not possible to have this kind of hint.
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18086
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77536/
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18086
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77421/
Test FAILed.
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/18086
thanks, merging to master/2.2!
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by bogdanrdc <gi...@git.apache.org>.
Github user bogdanrdc commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r119087261
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -371,7 +371,7 @@ querySpecification
(RECORDREADER recordReader=STRING)?
fromClause?
(WHERE where=booleanExpression)?)
- | ((kind=SELECT hint? setQuantifier? namedExpressionSeq fromClause?
+ | ((kind=SELECT (hints+=hint)* setQuantifier? namedExpressionSeq fromClause?
--- End diff --
This patch supports both.
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18086
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77636/
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18086
**[Test build #77300 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77300/testReport)** for PR 18086 at commit [`5439468`](https://github.com/apache/spark/commit/5439468f85c01dc2c5ae36a4c1a0fceb210d9852).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class UnresolvedHint(name: String, parameters: Seq[Any], child: LogicalPlan)`
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r119169530
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -371,7 +371,7 @@ querySpecification
(RECORDREADER recordReader=STRING)?
fromClause?
(WHERE where=booleanExpression)?)
- | ((kind=SELECT hint? setQuantifier? namedExpressionSeq fromClause?
+ | ((kind=SELECT (hints+=hint)* setQuantifier? namedExpressionSeq fromClause?
--- End diff --
It does not hurt anything if we support more hint styles, as long as they are user-friendly.
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
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/18086#discussion_r119268299
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameHintSuite.scala ---
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.spark.sql.catalyst.analysis.EliminateBarriers
+import org.apache.spark.sql.catalyst.plans.PlanTest
+import org.apache.spark.sql.catalyst.plans.logical._
+import org.apache.spark.sql.test.SharedSQLContext
+
+class DataFrameHintSuite extends PlanTest with SharedSQLContext {
+ import testImplicits._
+ lazy val df = spark.range(10)
+
+ private def check(df: Dataset[_], expected: LogicalPlan) = {
+ comparePlans(
+ EliminateBarriers(df.queryExecution.logical),
--- End diff --
that PR has been reverted, can you rebase?
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18086
**[Test build #77421 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77421/testReport)** for PR 18086 at commit [`d386cdf`](https://github.com/apache/spark/commit/d386cdf1853b90741762a8a73e82b997e5c3ee7d).
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18086
**[Test build #77636 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77636/testReport)** for PR 18086 at commit [`7776ae6`](https://github.com/apache/spark/commit/7776ae66781961724bbc10a10162bf21d5330d12).
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
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/18086#discussion_r118467522
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala ---
@@ -533,13 +533,16 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
}
/**
- * Add a [[UnresolvedHint]] to a logical plan.
+ * Add a [[UnresolvedHint]]s to a logical plan.
--- End diff --
remove `a`
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18086
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77424/
Test FAILed.
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/18086
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r118832533
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala ---
@@ -518,47 +518,97 @@ class PlanParserSuite extends PlanTest {
val m = intercept[ParseException] {
parsePlan("SELECT /*+ HINT() */ * FROM t")
}.getMessage
- assert(m.contains("no viable alternative at input"))
-
- // Hive compatibility: No database.
- val m2 = intercept[ParseException] {
- parsePlan("SELECT /*+ MAPJOIN(default.t) */ * from default.t")
- }.getMessage
- assert(m2.contains("mismatched input '.' expecting {')', ','}"))
+ assert(m.contains("mismatched input"))
// Disallow space as the delimiter.
val m3 = intercept[ParseException] {
parsePlan("SELECT /*+ INDEX(a b c) */ * from default.t")
}.getMessage
- assert(m3.contains("mismatched input 'b' expecting {')', ','}"))
+ assert(m3.contains("mismatched input 'b' expecting"))
comparePlans(
parsePlan("SELECT /*+ HINT */ * FROM t"),
UnresolvedHint("HINT", Seq.empty, table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ BROADCASTJOIN(u) */ * FROM t"),
- UnresolvedHint("BROADCASTJOIN", Seq("u"), table("t").select(star())))
+ UnresolvedHint("BROADCASTJOIN", Seq($"u"), table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ MAPJOIN(u) */ * FROM t"),
- UnresolvedHint("MAPJOIN", Seq("u"), table("t").select(star())))
+ UnresolvedHint("MAPJOIN", Seq($"u"), table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ STREAMTABLE(a,b,c) */ * FROM t"),
- UnresolvedHint("STREAMTABLE", Seq("a", "b", "c"), table("t").select(star())))
+ UnresolvedHint("STREAMTABLE", Seq($"a", $"b", $"c"), table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ INDEX(t, emp_job_ix) */ * FROM t"),
- UnresolvedHint("INDEX", Seq("t", "emp_job_ix"), table("t").select(star())))
+ UnresolvedHint("INDEX", Seq($"t", $"emp_job_ix"), table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ MAPJOIN(`default.t`) */ * from `default.t`"),
- UnresolvedHint("MAPJOIN", Seq("default.t"), table("default.t").select(star())))
+ UnresolvedHint("MAPJOIN", Seq(UnresolvedAttribute.quoted("default.t")),
+ table("default.t").select(star())))
comparePlans(
parsePlan("SELECT /*+ MAPJOIN(t) */ a from t where true group by a order by a"),
- UnresolvedHint("MAPJOIN", Seq("t"),
+ UnresolvedHint("MAPJOIN", Seq($"t"),
table("t").where(Literal(true)).groupBy('a)('a)).orderBy('a.asc))
}
+
+ test("SPARK-20854: select hint syntax with expressions") {
+ comparePlans(
+ parsePlan("SELECT /*+ HINT1(a, array(1, 2, 3)) */ * from t"),
+ UnresolvedHint("HINT1", Seq($"a",
+ UnresolvedFunction("array", Literal(1) :: Literal(2) :: Literal(3) :: Nil, false)),
+ table("t").select(star())
+ )
+ )
+
+ comparePlans(
+ parsePlan("SELECT /*+ HINT1(a, array(1, 2, 3)) */ * from t"),
+ UnresolvedHint("HINT1", Seq($"a",
+ UnresolvedFunction("array", Literal(1) :: Literal(2) :: Literal(3) :: Nil, false)),
+ table("t").select(star())
+ )
+ )
+
+ comparePlans(
+ parsePlan("SELECT /*+ HINT1(a, 5, 'a', b) */ * from t"),
+ UnresolvedHint("HINT1", Seq($"a", Literal(5), Literal("a"), $"b"),
+ table("t").select(star())
+ )
+ )
+
+ comparePlans(
+ parsePlan("SELECT /*+ HINT1('a', (b, c), (1, 2)) */ * from t"),
+ UnresolvedHint("HINT1",
+ Seq(Literal("a"),
+ CreateStruct($"b" :: $"c" :: Nil),
+ CreateStruct(Literal(1) :: Literal(2) :: Nil)),
+ table("t").select(star())
+ )
+ )
+ }
+
+ test("SPARK-20854: multiple hints") {
+ comparePlans(
+ parsePlan("SELECT /*+ HINT1(a, 1) hint2(b, 2) */ * from t"),
+ UnresolvedHint("hint2", Seq($"b", Literal(2)),
+ UnresolvedHint("HINT1", Seq($"a", Literal(1)),
+ table("t").select(star())
--- End diff --
Nit: Indent
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18086
**[Test build #77536 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77536/testReport)** for PR 18086 at commit [`09635a9`](https://github.com/apache/spark/commit/09635a9b1350596961f2bb3f824f9730c3c348d2).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class DataFrameSuite extends QueryTest with SharedSQLContext `
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/18086
cc @gatorsmile @cloud-fan @hvanhovell
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18086
**[Test build #77636 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77636/testReport)** for PR 18086 at commit [`7776ae6`](https://github.com/apache/spark/commit/7776ae66781961724bbc10a10162bf21d5330d12).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class Uuid() extends LeafExpression `
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r118832456
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/hints.scala ---
@@ -25,7 +25,7 @@ import org.apache.spark.sql.internal.SQLConf
* should be removed This node will be eliminated post analysis.
* A pair of (name, parameters).
--- End diff --
This needs an update.
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18086
**[Test build #77536 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77536/testReport)** for PR 18086 at commit [`09635a9`](https://github.com/apache/spark/commit/09635a9b1350596961f2bb3f824f9730c3c348d2).
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18086
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77300/
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
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/18086#discussion_r119266605
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/DSLHintSuite.scala ---
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.spark.sql.catalyst.analysis.AnalysisTest
+import org.apache.spark.sql.catalyst.dsl.expressions._
+import org.apache.spark.sql.catalyst.dsl.plans._
+import org.apache.spark.sql.catalyst.expressions._
+import org.apache.spark.sql.catalyst.plans.logical._
+
+class DSLHintSuite extends AnalysisTest {
+ lazy val a = 'a.int
+ lazy val b = 'b.string
+ lazy val c = 'c.string
+ lazy val r1 = LocalRelation(a, b, c)
+
+ test("various hint parameters") {
+ comparePlans(
+ r1.hint("hint1"),
+ UnresolvedHint("hint1", Seq(),
+ r1
+ )
+ )
+
+ comparePlans(
+ r1.hint("hint1", 1, "a"),
+ UnresolvedHint("hint1", Seq(1, "a"), r1)
+ )
+
+ comparePlans(
+ r1.hint("hint1", 1, $"a"),
+ UnresolvedHint("hint1", Seq(1, $"a"),
+ r1
+ )
--- End diff --
ditto
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
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/18086#discussion_r119266598
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/DSLHintSuite.scala ---
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.spark.sql.catalyst.analysis.AnalysisTest
+import org.apache.spark.sql.catalyst.dsl.expressions._
+import org.apache.spark.sql.catalyst.dsl.plans._
+import org.apache.spark.sql.catalyst.expressions._
+import org.apache.spark.sql.catalyst.plans.logical._
+
+class DSLHintSuite extends AnalysisTest {
+ lazy val a = 'a.int
+ lazy val b = 'b.string
+ lazy val c = 'c.string
+ lazy val r1 = LocalRelation(a, b, c)
+
+ test("various hint parameters") {
+ comparePlans(
+ r1.hint("hint1"),
+ UnresolvedHint("hint1", Seq(),
+ r1
+ )
--- End diff --
nit: can we collapse it to the previous line?
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
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/18086#discussion_r118701338
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/hints.scala ---
@@ -25,7 +25,7 @@ import org.apache.spark.sql.internal.SQLConf
* should be removed This node will be eliminated post analysis.
* A pair of (name, parameters).
*/
-case class UnresolvedHint(name: String, parameters: Seq[String], child: LogicalPlan)
+case class UnresolvedHint(name: String, parameters: Seq[Any], child: LogicalPlan)
--- End diff --
we can keep `Any` in the API(`df.hint(xxx)`), but use `Expression` in `UnresolvedHint`, what do 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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by bogdanrdc <gi...@git.apache.org>.
Github user bogdanrdc commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r119087361
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameHintSuite.scala ---
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
--- End diff --
I added a new test for dsl. I also want a test that calls df.hint
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18086
**[Test build #77424 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77424/testReport)** for PR 18086 at commit [`6e40301`](https://github.com/apache/spark/commit/6e4030160fa12934dccacea59b06033fdc8ced80).
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18086
**[Test build #77531 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77531/testReport)** for PR 18086 at commit [`8daa05e`](https://github.com/apache/spark/commit/8daa05e1ec595c54f8aac7c041d5c6571a759aaf).
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by bogdanrdc <gi...@git.apache.org>.
Github user bogdanrdc commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r118660663
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala ---
@@ -533,13 +533,16 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
}
/**
- * Add a [[UnresolvedHint]] to a logical plan.
+ * Add a [[UnresolvedHint]]s to a logical plan.
*/
private def withHints(
ctx: HintContext,
query: LogicalPlan): LogicalPlan = withOrigin(ctx) {
- val stmt = ctx.hintStatement
- UnresolvedHint(stmt.hintName.getText, stmt.parameters.asScala.map(_.getText), query)
+ var plan = query
--- End diff --
I used foldRight somewhere too. Why is it a bad idea?
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
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/18086#discussion_r119267579
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala ---
@@ -407,7 +407,7 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
val withWindow = withDistinct.optionalMap(windows)(withWindows)
// Hint
- withWindow.optionalMap(hint)(withHints)
+ hints.asScala.foldRight(withWindow)(withHints)
--- End diff --
why we construct the hint from right to left?
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by bogdanrdc <gi...@git.apache.org>.
Github user bogdanrdc commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r118661387
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/hints.scala ---
@@ -25,7 +25,7 @@ import org.apache.spark.sql.internal.SQLConf
* should be removed This node will be eliminated post analysis.
* A pair of (name, parameters).
*/
-case class UnresolvedHint(name: String, parameters: Seq[String], child: LogicalPlan)
+case class UnresolvedHint(name: String, parameters: Seq[Any], child: LogicalPlan)
--- End diff --
If we use Expression then either:
* Dataset.hint parameters should be Expression too, in which case you can't do `df.hint("hint", 1, 2, "c")` you'd have to do `df.hint("hint", Literal(1), Literal(2), Literal("c"))` or a shortcut if there is
* Dataset.hint accepts Any but then has to convert Any to Expressions. One problem here is that Seq(1,2,3) can't be converted to Literal. So you have to use `df.hint("hint", Array(1,2,3))`
The disadvantage of have Any in UnresolvedHint is that to resolve the hint you have to check both for String and Literal(String) but the API is easier to use.
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
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/18086#discussion_r119143953
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -371,7 +371,7 @@ querySpecification
(RECORDREADER recordReader=STRING)?
fromClause?
(WHERE where=booleanExpression)?)
- | ((kind=SELECT hint? setQuantifier? namedExpressionSeq fromClause?
+ | ((kind=SELECT (hints+=hint)* setQuantifier? namedExpressionSeq fromClause?
--- End diff --
@gatorsmile does hive support multiple `/*+ */`?
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/18086
why rename `DataFrameSuite`?
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r118473083
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala ---
@@ -533,13 +533,16 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
}
/**
- * Add a [[UnresolvedHint]] to a logical plan.
+ * Add a [[UnresolvedHint]]s to a logical plan.
*/
private def withHints(
ctx: HintContext,
query: LogicalPlan): LogicalPlan = withOrigin(ctx) {
- val stmt = ctx.hintStatement
- UnresolvedHint(stmt.hintName.getText, stmt.parameters.asScala.map(_.getText), query)
+ var plan = query
--- End diff --
Honestly I think foldLeft is almost always a bad idea ...
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r119271262
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala ---
@@ -533,13 +533,16 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
}
/**
- * Add a [[UnresolvedHint]] to a logical plan.
+ * Add a [[UnresolvedHint]]s to a logical plan.
*/
private def withHints(
ctx: HintContext,
query: LogicalPlan): LogicalPlan = withOrigin(ctx) {
- val stmt = ctx.hintStatement
- UnresolvedHint(stmt.hintName.getText, stmt.parameters.asScala.map(_.getText), query)
+ var plan = query
--- End diff --
i always find a loop simpler to reason about ...
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by bogdanrdc <gi...@git.apache.org>.
Github user bogdanrdc commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r119087296
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -381,12 +381,12 @@ querySpecification
;
hint
- : '/*+' hintStatement '*/'
+ : '/*+' hintStatements+=hintStatement (hintStatements+=hintStatement)* '*/'
--- End diff --
I added support for optional comma
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
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/18086#discussion_r119266645
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/DSLHintSuite.scala ---
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql
+
+import org.apache.spark.sql.catalyst.analysis.AnalysisTest
+import org.apache.spark.sql.catalyst.dsl.expressions._
+import org.apache.spark.sql.catalyst.dsl.plans._
+import org.apache.spark.sql.catalyst.expressions._
+import org.apache.spark.sql.catalyst.plans.logical._
+
+class DSLHintSuite extends AnalysisTest {
+ lazy val a = 'a.int
+ lazy val b = 'b.string
+ lazy val c = 'c.string
+ lazy val r1 = LocalRelation(a, b, c)
+
+ test("various hint parameters") {
+ comparePlans(
+ r1.hint("hint1"),
+ UnresolvedHint("hint1", Seq(),
+ r1
+ )
+ )
+
+ comparePlans(
+ r1.hint("hint1", 1, "a"),
+ UnresolvedHint("hint1", Seq(1, "a"), r1)
+ )
+
+ comparePlans(
+ r1.hint("hint1", 1, $"a"),
+ UnresolvedHint("hint1", Seq(1, $"a"),
+ r1
+ )
+ )
+
+ comparePlans(
+ r1.hint("hint1", Seq(1, 2, 3), Seq($"a", $"b", $"c")),
+ UnresolvedHint("hint1", Seq(Seq(1, 2, 3), Seq($"a", $"b", $"c")),
+ r1
+ )
--- End diff --
ditto
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r118832568
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/hints.scala ---
@@ -25,7 +25,7 @@ import org.apache.spark.sql.internal.SQLConf
* should be removed This node will be eliminated post analysis.
* A pair of (name, parameters).
*/
-case class UnresolvedHint(name: String, parameters: Seq[String], child: LogicalPlan)
+case class UnresolvedHint(name: String, parameters: Seq[Any], child: LogicalPlan)
--- End diff --
To support multiple parameters in `hint`, does it make sense to do it like `df.hint("hint", "1, 2, c")`? We can use our Parser to parse this parameter string.
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/18086
LGTM pending Jenkins
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r119168868
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -371,7 +371,7 @@ querySpecification
(RECORDREADER recordReader=STRING)?
fromClause?
(WHERE where=booleanExpression)?)
- | ((kind=SELECT hint? setQuantifier? namedExpressionSeq fromClause?
+ | ((kind=SELECT (hints+=hint)* setQuantifier? namedExpressionSeq fromClause?
--- End diff --
Yes, but using the comma as the separators.
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
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/18086#discussion_r119762455
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala ---
@@ -527,47 +527,117 @@ class PlanParserSuite extends PlanTest {
val m = intercept[ParseException] {
parsePlan("SELECT /*+ HINT() */ * FROM t")
}.getMessage
- assert(m.contains("no viable alternative at input"))
-
- // Hive compatibility: No database.
- val m2 = intercept[ParseException] {
- parsePlan("SELECT /*+ MAPJOIN(default.t) */ * from default.t")
- }.getMessage
- assert(m2.contains("mismatched input '.' expecting {')', ','}"))
+ assert(m.contains("mismatched input"))
// Disallow space as the delimiter.
val m3 = intercept[ParseException] {
parsePlan("SELECT /*+ INDEX(a b c) */ * from default.t")
}.getMessage
- assert(m3.contains("mismatched input 'b' expecting {')', ','}"))
+ assert(m3.contains("mismatched input 'b' expecting"))
comparePlans(
parsePlan("SELECT /*+ HINT */ * FROM t"),
UnresolvedHint("HINT", Seq.empty, table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ BROADCASTJOIN(u) */ * FROM t"),
- UnresolvedHint("BROADCASTJOIN", Seq("u"), table("t").select(star())))
+ UnresolvedHint("BROADCASTJOIN", Seq($"u"), table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ MAPJOIN(u) */ * FROM t"),
- UnresolvedHint("MAPJOIN", Seq("u"), table("t").select(star())))
+ UnresolvedHint("MAPJOIN", Seq($"u"), table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ STREAMTABLE(a,b,c) */ * FROM t"),
- UnresolvedHint("STREAMTABLE", Seq("a", "b", "c"), table("t").select(star())))
+ UnresolvedHint("STREAMTABLE", Seq($"a", $"b", $"c"), table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ INDEX(t, emp_job_ix) */ * FROM t"),
- UnresolvedHint("INDEX", Seq("t", "emp_job_ix"), table("t").select(star())))
+ UnresolvedHint("INDEX", Seq($"t", $"emp_job_ix"), table("t").select(star())))
comparePlans(
parsePlan("SELECT /*+ MAPJOIN(`default.t`) */ * from `default.t`"),
- UnresolvedHint("MAPJOIN", Seq("default.t"), table("default.t").select(star())))
+ UnresolvedHint("MAPJOIN", Seq(UnresolvedAttribute.quoted("default.t")),
+ table("default.t").select(star())))
comparePlans(
parsePlan("SELECT /*+ MAPJOIN(t) */ a from t where true group by a order by a"),
- UnresolvedHint("MAPJOIN", Seq("t"),
+ UnresolvedHint("MAPJOIN", Seq($"t"),
table("t").where(Literal(true)).groupBy('a)('a)).orderBy('a.asc))
}
+
+ test("SPARK-20854: select hint syntax with expressions") {
+ comparePlans(
+ parsePlan("SELECT /*+ HINT1(a, array(1, 2, 3)) */ * from t"),
+ UnresolvedHint("HINT1", Seq($"a",
+ UnresolvedFunction("array", Literal(1) :: Literal(2) :: Literal(3) :: Nil, false)),
+ table("t").select(star())
+ )
+ )
+
+ comparePlans(
+ parsePlan("SELECT /*+ HINT1(a, array(1, 2, 3)) */ * from t"),
--- End diff --
yea, @bogdanrdc can you send a follow-up PR to clean it up?
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18086
Merged build finished. Test FAILed.
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18086
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77531/
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18086#discussion_r118832466
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveHints.scala ---
@@ -91,7 +92,12 @@ object ResolveHints {
ResolvedHint(h.child, HintInfo(isBroadcastable = Option(true)))
} else {
// Otherwise, find within the subtree query plans that should be broadcasted.
- applyBroadcastHint(h.child, h.parameters.toSet)
+ applyBroadcastHint(h.child, h.parameters.map {
+ case tableName: String => tableName
+ case tableId: UnresolvedAttribute => tableId.name
+ case unsupported => throw new AnalysisException("Broadcast hint parameter should be " +
+ s" identifier or string but was $unsupported (${unsupported.getClass}")
--- End diff --
Nit: `s" identifier or string` -> `s"an identifier or string `
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
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/18086#discussion_r119266987
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala ---
@@ -525,47 +525,117 @@ class PlanParserSuite extends PlanTest {
val m = intercept[ParseException] {
parsePlan("SELECT /*+ HINT() */ * FROM t")
--- End diff --
seems this is not consistent with dataframe API, which doesn't require at least one parameter.
---
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 #18086: [SPARK-20854][SQL] Extend hint syntax to support ...
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/18086#discussion_r118467770
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/hints.scala ---
@@ -25,7 +25,7 @@ import org.apache.spark.sql.internal.SQLConf
* should be removed This node will be eliminated post analysis.
* A pair of (name, parameters).
*/
-case class UnresolvedHint(name: String, parameters: Seq[String], child: LogicalPlan)
+case class UnresolvedHint(name: String, parameters: Seq[Any], child: LogicalPlan)
--- End diff --
shall we use `Expression` as type?
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18086
**[Test build #77300 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77300/testReport)** for PR 18086 at commit [`5439468`](https://github.com/apache/spark/commit/5439468f85c01dc2c5ae36a4c1a0fceb210d9852).
---
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 issue #18086: [SPARK-20854][SQL] Extend hint syntax to support express...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18086
**[Test build #77421 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77421/testReport)** for PR 18086 at commit [`d386cdf`](https://github.com/apache/spark/commit/d386cdf1853b90741762a8a73e82b997e5c3ee7d).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class HasMinSupport(Params):`
* `class HasNumPartitions(Params):`
* `class HasMinConfidence(Params):`
* `case class AnalysisBarrier(child: LogicalPlan) extends LeafNode `
* `case class ResolvedHint(child: LogicalPlan, hints: HintInfo = HintInfo())`
* `case class HintInfo(`
---
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