You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by rxin <gi...@git.apache.org> on 2017/02/14 12:21:54 UTC
[GitHub] spark pull request #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
GitHub user rxin opened a pull request:
https://github.com/apache/spark/pull/16925
[SPARK-16475][SQL] Broadcast Hint for SQL Queries
## What changes were proposed in this pull request?
This PR aims to achieve the following two goals in Spark SQL.
1. Generic Hint Syntax
The generic hints are parsed and transformed into concrete hints by SubstituteHints of Analyzer. The unknown hints are removed, too. For example, Hint("MAPJOIN") is transformed into BroadcastJoin and other hints are removed currently.
```
SELECT /*+ MAPJOIN(t) */ * FROM t
SELECT /*+ STREAMTABLE(a,b,c) */ * FROM t
SELECT /*+ INDEX(t emp_job_ix) */ * FROM t
```
Unlink Hive, NEWMAPJOIN(t) is allowed for accepting new Spark Hints.
2. Broadcast Hints
The followings are recognized. Technically, broadcast hints are matched UnresolvedRelation to support Hive MetastoreRelation. The style of database_name.table_name is not allowed in this PR.
```
SELECT /*+ MAPJOIN(t) */ * FROM t JOIN u ON t.id = u.id
SELECT /*+ BROADCAST(u) */ * FROM t JOIN u ON t.id = u.id
SELECT /*+ BROADCASTJOIN(u) */ * FROM t JOIN u ON t.id = u.id
```
Examples
```
scala> spark.range(1000000000).createOrReplaceTempView("t")
scala> spark.range(1000000000).createOrReplaceTempView("u")
scala> sql("SELECT * FROM t JOIN u ON t.id = u.id").explain
== Physical Plan ==
*SortMergeJoin [id#0L], [id#4L], Inner
:- *Sort [id#0L ASC], false, 0
: +- Exchange hashpartitioning(id#0L, 200)
: +- *Range (0, 1000000000, splits=8)
+- *Sort [id#4L ASC], false, 0
+- ReusedExchange [id#4L], Exchange hashpartitioning(id#0L, 200)
scala> sql("SELECT /*+ MAPJOIN(t) */ * FROM t JOIN u ON t.id = u.id").explain
== Physical Plan ==
*BroadcastHashJoin [id#0L], [id#4L], Inner, BuildLeft
:- BroadcastExchange HashedRelationBroadcastMode(List(input[0, bigint, false]))
: +- *Range (0, 1000000000, splits=8)
+- *Range (0, 1000000000, splits=8)
scala> sql("SELECT /*+ MAPJOIN(u) */ * FROM t JOIN u ON t.id = u.id").explain
== Physical Plan ==
*BroadcastHashJoin [id#0L], [id#4L], Inner, BuildRight
:- *Range (0, 1000000000, splits=8)
+- BroadcastExchange HashedRelationBroadcastMode(List(input[0, bigint, false]))
+- *Range (0, 1000000000, splits=8)
scala> sql("CREATE TABLE hive_t(id INT)")
res5: org.apache.spark.sql.DataFrame = []
scala> sql("CREATE TABLE hive_u(id INT)")
res6: org.apache.spark.sql.DataFrame = []
scala> sql("SELECT /*+ MAPJOIN(hive_u) */ * FROM hive_t JOIN hive_u ON hive_t.id = hive_u.id").explain
== Physical Plan ==
*BroadcastHashJoin [id#28], [id#29], Inner, BuildRight
:- *Filter isnotnull(id#28)
: +- HiveTableScan [id#28], MetastoreRelation default, hive_t
+- BroadcastExchange HashedRelationBroadcastMode(List(cast(input[0, int, false] as bigint)))
+- *Filter isnotnull(id#29)
+- HiveTableScan [id#29], MetastoreRelation default, hive_u
scala> sql("SELECT * FROM hive_t JOIN hive_u ON hive_t.id = hive_u.id").explain
== Physical Plan ==
*SortMergeJoin [id#36], [id#37], Inner
:- *Sort [id#36 ASC], false, 0
: +- Exchange hashpartitioning(id#36, 200)
: +- *Filter isnotnull(id#36)
: +- HiveTableScan [id#36], MetastoreRelation default, hive_t
+- *Sort [id#37 ASC], false, 0
+- Exchange hashpartitioning(id#37, 200)
+- *Filter isnotnull(id#37)
+- HiveTableScan [id#37], MetastoreRelation default, hive_u
```
This patch is based on the work done in https://github.com/apache/spark/pull/14426.
## How was this patch tested?
Added a new unit test suite for the broadcast hint rule (SubstituteHintsSuite) and new test cases for parser change (in PlanParserSuite). Also added end-to-end test suites.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rxin/spark SPARK-16475-broadcast-hint
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/16925.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 #16925
----
commit 539782d93af14ed27c6f3a0fc659b13c4f92da41
Author: Dongjoon Hyun <do...@apache.org>
Date: 2016-07-11T08:04:51Z
[SPARK-16475][SQL] Broadcast Hint for SQL Queries
commit 318bc031985d178cb0ae7612cbee4d62636916b3
Author: Reynold Xin <rx...@databricks.com>
Date: 2017-02-14T12:16:55Z
Merge pull request #14426 from dongjoon-hyun/SPARK-16475-HINT
[SPARK-16475][SQL] Broadcast Hint for SQL Queries
----
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
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/16925#discussion_r101123120
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala ---
@@ -380,7 +380,10 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with Logging {
}
// Window
- withDistinct.optionalMap(windows)(withWindows)
+ val withWindow = withDistinct.optionalMap(windows)(withWindows)
+
+ // Hint
+ withWindow.optionalMap(ctx.hint)(withHints)
--- End diff --
nit: we already `import ctx._` at the beginning, to be consistent we can just write `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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72872/
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101286756
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -374,6 +374,16 @@ querySpecification
windows?)
;
+hint
+ : '/*+' hintStatement '*/'
+ ;
+
+hintStatement
+ : hintName=identifier
+ | hintName=identifier '(' parameters+=identifier parameters+=identifier ')'
--- End diff --
This rule doesn't support like `SELECT /*+ INDEX(a b c) */ * FROM t`. To support it, it should be:
| hintName=identifier '(' parameters+=identifier parameters+=identifier* ')'
Is it intentional?
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101129453
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/SubstituteHintsSuite.scala ---
@@ -0,0 +1,123 @@
+/*
+ * 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.catalyst.analysis
+
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.catalyst.dsl.expressions._
+import org.apache.spark.sql.catalyst.dsl.plans._
+import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
+import org.apache.spark.sql.catalyst.plans.Inner
+import org.apache.spark.sql.catalyst.plans.logical._
+
+class SubstituteHintsSuite extends AnalysisTest {
+ import org.apache.spark.sql.catalyst.analysis.TestRelations._
+
+ test("invalid hints should be ignored") {
+ checkAnalysis(
+ Hint("some_random_hint_that_does_not_exist", Seq("TaBlE"), table("TaBlE")),
+ testRelation,
+ caseSensitive = false)
+ }
+
+ test("case-sensitive or insensitive parameters") {
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("TaBlE"), table("TaBlE")),
+ BroadcastHint(testRelation),
+ caseSensitive = false)
+
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("table"), table("TaBlE")),
+ BroadcastHint(testRelation),
+ caseSensitive = false)
+
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("TaBlE"), table("TaBlE")),
+ BroadcastHint(testRelation),
+ caseSensitive = true)
+
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("table"), table("TaBlE")),
+ testRelation,
+ caseSensitive = true)
+ }
+
+ test("multiple broadcast hint aliases") {
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("table", "table2"), table("table").join(table("table2"))),
+ Join(BroadcastHint(testRelation), BroadcastHint(testRelation2), Inner, None),
+ caseSensitive = false)
+ }
+
+ test("do not traverse past existing broadcast hints") {
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("table"), BroadcastHint(table("table").where('a > 1))),
+ BroadcastHint(testRelation.where('a > 1)).analyze,
+ caseSensitive = false)
+ }
+
+ test("should work for subqueries") {
+ val relation = UnresolvedRelation(TableIdentifier("table"), Some("tableAlias"))
--- End diff --
They do different things.
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16925
**[Test build #72872 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72872/testReport)** for PR 16925 at commit [`c702e3e`](https://github.com/apache/spark/commit/c702e3ee8ee41ede216eab1209fe1e9374a2c301).
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
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/16925#discussion_r101125744
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/SubstituteHints.scala ---
@@ -0,0 +1,103 @@
+/*
+ * 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.catalyst.analysis
+
+import org.apache.spark.sql.catalyst.CatalystConf
+import org.apache.spark.sql.catalyst.plans.logical._
+import org.apache.spark.sql.catalyst.rules.Rule
+import org.apache.spark.sql.catalyst.trees.CurrentOrigin
+
+
+/**
+ * Collection of rules related to hints. The only hint currently available is broadcast join hint.
+ *
+ * Note that this is separatedly into two rules because in the future we might introduce new hint
+ * rules that have different ordering requirements from broadcast.
+ */
+object SubstituteHints {
+
+ /**
+ * Substitute Hints.
+ *
+ * The only hint currently available is broadcast join hint.
+ *
+ * For broadcast hint, we accept "BROADCAST", "BROADCASTJOIN", and "MAPJOIN", and a sequence of
+ * relation aliases can be specified in the hint. A broadcast hint plan node will be inserted
+ * on top of any relation (that is not aliased differently), subquery, or common table expression
+ * that match the specified name.
+ *
+ * The hint resolution works by recursively traversing down the query plan to find a relation or
+ * subquery that matches one of the specified broadcast aliases. The traversal does not go past
+ * beyond any existing broadcast hints, subquery aliases.
+ *
+ * This rule must happen before common table expressions.
+ */
+ class SubstituteBroadcastHints(conf: CatalystConf) extends Rule[LogicalPlan] {
+ private val BROADCAST_HINT_NAMES = Set("BROADCAST", "BROADCASTJOIN", "MAPJOIN")
+
+ def resolver: Resolver = conf.resolver
+
+ private def applyBroadcastHint(plan: LogicalPlan, toBroadcast: Set[String]): LogicalPlan = {
+ // Whether to continue recursing down the tree
+ var recurse = true
+
+ val newNode = CurrentOrigin.withOrigin(plan.origin) {
+ plan match {
+ case r: UnresolvedRelation =>
+ val alias = r.alias.getOrElse(r.tableIdentifier.table)
+ if (toBroadcast.exists(resolver(_, alias))) BroadcastHint(plan) else plan
+ case r: SubqueryAlias =>
+ if (toBroadcast.exists(resolver(_, r.alias))) {
+ BroadcastHint(plan)
+ } else {
+ // Don't recurse down subquery aliases if there are no match.
+ recurse = false
+ plan
+ }
+ case _: BroadcastHint =>
+ // Found a broadcast hint; don't change the plan but also don't recurse down.
+ recurse = false
+ plan
+ case _ =>
+ plan
+ }
+ }
+
+ if ((plan fastEquals newNode) && recurse) {
+ newNode.mapChildren(child => applyBroadcastHint(child, toBroadcast))
--- End diff --
in the case of self-join, we may broadcast both side, is it expected?
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101129594
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/SubstituteHints.scala ---
@@ -0,0 +1,103 @@
+/*
+ * 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.catalyst.analysis
+
+import org.apache.spark.sql.catalyst.CatalystConf
+import org.apache.spark.sql.catalyst.plans.logical._
+import org.apache.spark.sql.catalyst.rules.Rule
+import org.apache.spark.sql.catalyst.trees.CurrentOrigin
+
+
+/**
+ * Collection of rules related to hints. The only hint currently available is broadcast join hint.
+ *
+ * Note that this is separatedly into two rules because in the future we might introduce new hint
+ * rules that have different ordering requirements from broadcast.
+ */
+object SubstituteHints {
+
+ /**
+ * Substitute Hints.
+ *
+ * The only hint currently available is broadcast join hint.
+ *
+ * For broadcast hint, we accept "BROADCAST", "BROADCASTJOIN", and "MAPJOIN", and a sequence of
+ * relation aliases can be specified in the hint. A broadcast hint plan node will be inserted
+ * on top of any relation (that is not aliased differently), subquery, or common table expression
+ * that match the specified name.
+ *
+ * The hint resolution works by recursively traversing down the query plan to find a relation or
+ * subquery that matches one of the specified broadcast aliases. The traversal does not go past
+ * beyond any existing broadcast hints, subquery aliases.
+ *
+ * This rule must happen before common table expressions.
+ */
+ class SubstituteBroadcastHints(conf: CatalystConf) extends Rule[LogicalPlan] {
+ private val BROADCAST_HINT_NAMES = Set("BROADCAST", "BROADCASTJOIN", "MAPJOIN")
+
+ def resolver: Resolver = conf.resolver
+
+ private def applyBroadcastHint(plan: LogicalPlan, toBroadcast: Set[String]): LogicalPlan = {
+ // Whether to continue recursing down the tree
+ var recurse = true
+
+ val newNode = CurrentOrigin.withOrigin(plan.origin) {
+ plan match {
+ case r: UnresolvedRelation =>
+ val alias = r.alias.getOrElse(r.tableIdentifier.table)
+ if (toBroadcast.exists(resolver(_, alias))) BroadcastHint(plan) else plan
+ case r: SubqueryAlias =>
+ if (toBroadcast.exists(resolver(_, r.alias))) {
+ BroadcastHint(plan)
+ } else {
+ // Don't recurse down subquery aliases if there are no match.
+ recurse = false
+ plan
+ }
+ case _: BroadcastHint =>
+ // Found a broadcast hint; don't change the plan but also don't recurse down.
+ recurse = false
+ plan
+ case _ =>
+ plan
+ }
+ }
+
+ if ((plan fastEquals newNode) && recurse) {
+ newNode.mapChildren(child => applyBroadcastHint(child, toBroadcast))
--- End diff --
That's fine. Both being broadcastable doesn't mean we broadcast 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 pull request #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
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/16925#discussion_r101127851
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala ---
@@ -493,4 +493,46 @@ class PlanParserSuite extends PlanTest {
assertEqual("select a, b from db.c where x !> 1",
table("db", "c").where('x <= 1).select('a, 'b))
}
+
+ test("select hint syntax") {
+ // Hive compatibility: Missing parameter raises ParseException.
+ 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("no viable alternative at input"))
+
+ comparePlans(
+ parsePlan("SELECT /*+ HINT */ * FROM t"),
+ Hint("HINT", Seq.empty, table("t").select(star())))
+
+ comparePlans(
+ parsePlan("SELECT /*+ BROADCASTJOIN(u) */ * FROM t"),
+ Hint("BROADCASTJOIN", Seq("u"), table("t").select(star())))
+
+ comparePlans(
+ parsePlan("SELECT /*+ MAPJOIN(u) */ * FROM t"),
+ Hint("MAPJOIN", Seq("u"), table("t").select(star())))
+
+ comparePlans(
+ parsePlan("SELECT /*+ STREAMTABLE(a,b,c) */ * FROM t"),
+ Hint("STREAMTABLE", Seq("a", "b", "c"), table("t").select(star())))
+
+ comparePlans(
+ parsePlan("SELECT /*+ INDEX(t emp_job_ix) */ * FROM t"),
--- End diff --
what if users writing `/*+ BROADCASTJOIN(a b) */`? shall we treat it as `/*+ BROADCASTJOIN(a, b) */`?
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Merged build triggered.
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16925
**[Test build #72888 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72888/testReport)** for PR 16925 at commit [`51a73d5`](https://github.com/apache/spark/commit/51a73d510a5faf3bbed8bb76ffe5590c68a67e2c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` class SubstituteBroadcastHints(conf: CatalystConf) extends Rule[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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101022875
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala ---
@@ -363,6 +363,29 @@ case class BroadcastHint(child: LogicalPlan) extends UnaryNode {
}
/**
+ * A general hint for the child. This node will be eliminated post analysis.
+ * A pair of (name, parameters).
+ */
+case class Hint(name: String, parameters: Seq[String], child: LogicalPlan) extends UnaryNode {
+ override lazy val resolved: Boolean = false
+ override def output: Seq[Attribute] = child.output
+}
+
+/**
+ * Options for writing new data into a table.
+ *
+ * @param enabled whether to overwrite existing data in the table.
+ * @param specificPartition only data in the specified partition will be overwritten.
+ */
+case class OverwriteOptions(
--- End diff --
What is this?
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/16925
Except the doc you mentioned, it looks great to me, @rxin !
It's great to have this in Spark SQL finally.
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16925
**[Test build #72893 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72893/testReport)** for PR 16925 at commit [`0d42978`](https://github.com/apache/spark/commit/0d429782b38cf19c0d4d6a5102102e39d872fee7).
* 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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16925
**[Test build #72874 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72874/testReport)** for PR 16925 at commit [`a095df3`](https://github.com/apache/spark/commit/a095df370aedbfd9a14e7761a9da18365cfd2bbf).
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries - WIP
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/16925
Actually I'm going to completely rewrite this. I don't think the current implementation makes sense.
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/16925
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101289645
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -374,6 +374,16 @@ querySpecification
windows?)
;
+hint
+ : '/*+' hintStatement '*/'
+ ;
+
+hintStatement
+ : hintName=identifier
+ | hintName=identifier '(' parameters+=identifier parameters+=identifier ')'
--- End diff --
btw this was written by @dongjoon-hyun. I didn't change 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 issue #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Merged build started.
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Merged build triggered.
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries - WIP
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16925
**[Test build #72879 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72879/testReport)** for PR 16925 at commit [`617f8a2`](https://github.com/apache/spark/commit/617f8a2b9d6069472729609c262ff4e4e2dde5f7).
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Merged build started.
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16925
**[Test build #72893 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72893/testReport)** for PR 16925 at commit [`0d42978`](https://github.com/apache/spark/commit/0d429782b38cf19c0d4d6a5102102e39d872fee7).
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
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/16925#discussion_r101124672
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -374,6 +374,16 @@ querySpecification
windows?)
;
+hint
+ : '/*+' hintStatement '*/'
+ ;
+
+hintStatement
+ : hintName=identifier
+ | hintName=identifier '(' parameters+=identifier parameters+=identifier ')'
--- End diff --
why do we need this?
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101137229
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/SubstituteHintsSuite.scala ---
@@ -0,0 +1,123 @@
+/*
+ * 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.catalyst.analysis
+
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.catalyst.dsl.expressions._
+import org.apache.spark.sql.catalyst.dsl.plans._
+import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
+import org.apache.spark.sql.catalyst.plans.Inner
+import org.apache.spark.sql.catalyst.plans.logical._
+
+class SubstituteHintsSuite extends AnalysisTest {
+ import org.apache.spark.sql.catalyst.analysis.TestRelations._
+
+ test("invalid hints should be ignored") {
+ checkAnalysis(
+ Hint("some_random_hint_that_does_not_exist", Seq("TaBlE"), table("TaBlE")),
+ testRelation,
+ caseSensitive = false)
+ }
+
+ test("case-sensitive or insensitive parameters") {
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("TaBlE"), table("TaBlE")),
+ BroadcastHint(testRelation),
+ caseSensitive = false)
+
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("table"), table("TaBlE")),
+ BroadcastHint(testRelation),
+ caseSensitive = false)
+
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("TaBlE"), table("TaBlE")),
+ BroadcastHint(testRelation),
+ caseSensitive = true)
+
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("table"), table("TaBlE")),
+ testRelation,
+ caseSensitive = true)
+ }
+
+ test("multiple broadcast hint aliases") {
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("table", "table2"), table("table").join(table("table2"))),
+ Join(BroadcastHint(testRelation), BroadcastHint(testRelation2), Inner, None),
+ caseSensitive = false)
+ }
+
+ test("do not traverse past existing broadcast hints") {
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("table"), BroadcastHint(table("table").where('a > 1))),
+ BroadcastHint(testRelation.where('a > 1)).analyze,
+ caseSensitive = false)
+ }
+
+ test("should work for subqueries") {
+ val relation = UnresolvedRelation(TableIdentifier("table"), Some("tableAlias"))
--- End diff --
actually they do the same thing. there's a pattern match in as. let me change 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 issue #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries - WIP
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Merged build started.
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries - WIP
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/16925
@rxin . May I update that better or do you wnat to finish here?
Anything is okay for me. Anyway, thank you for everything!
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101289635
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala ---
@@ -493,4 +493,46 @@ class PlanParserSuite extends PlanTest {
assertEqual("select a, b from db.c where x !> 1",
table("db", "c").where('x <= 1).select('a, 'b))
}
+
+ test("select hint syntax") {
+ // Hive compatibility: Missing parameter raises ParseException.
+ 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("no viable alternative at input"))
+
+ comparePlans(
+ parsePlan("SELECT /*+ HINT */ * FROM t"),
+ Hint("HINT", Seq.empty, table("t").select(star())))
+
+ comparePlans(
+ parsePlan("SELECT /*+ BROADCASTJOIN(u) */ * FROM t"),
+ Hint("BROADCASTJOIN", Seq("u"), table("t").select(star())))
+
+ comparePlans(
+ parsePlan("SELECT /*+ MAPJOIN(u) */ * FROM t"),
+ Hint("MAPJOIN", Seq("u"), table("t").select(star())))
+
+ comparePlans(
+ parsePlan("SELECT /*+ STREAMTABLE(a,b,c) */ * FROM t"),
+ Hint("STREAMTABLE", Seq("a", "b", "c"), table("t").select(star())))
+
+ comparePlans(
+ parsePlan("SELECT /*+ INDEX(t emp_job_ix) */ * FROM t"),
--- End diff --
@rxin Looks like we already support space as delimiter?
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101122924
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -1002,8 +1012,12 @@ SIMPLE_COMMENT
: '--' ~[\r\n]* '\r'? '\n'? -> channel(HIDDEN)
;
+BRACKETED_EMPTY_COMMENT
+ : '/**/' -> channel(HIDDEN)
+ ;
+
BRACKETED_COMMENT
- : '/*' .*? '*/' -> channel(HIDDEN)
+ : '/*' ~[+] .*? '*/' -> channel(HIDDEN)
--- End diff --
`/*+` is used `hint` rule in line 377.
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
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/16925#discussion_r101126846
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/SubstituteHintsSuite.scala ---
@@ -0,0 +1,123 @@
+/*
+ * 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.catalyst.analysis
+
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.catalyst.dsl.expressions._
+import org.apache.spark.sql.catalyst.dsl.plans._
+import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
+import org.apache.spark.sql.catalyst.plans.Inner
+import org.apache.spark.sql.catalyst.plans.logical._
+
+class SubstituteHintsSuite extends AnalysisTest {
+ import org.apache.spark.sql.catalyst.analysis.TestRelations._
+
+ test("invalid hints should be ignored") {
+ checkAnalysis(
+ Hint("some_random_hint_that_does_not_exist", Seq("TaBlE"), table("TaBlE")),
+ testRelation,
+ caseSensitive = false)
+ }
+
+ test("case-sensitive or insensitive parameters") {
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("TaBlE"), table("TaBlE")),
+ BroadcastHint(testRelation),
+ caseSensitive = false)
+
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("table"), table("TaBlE")),
+ BroadcastHint(testRelation),
+ caseSensitive = false)
+
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("TaBlE"), table("TaBlE")),
+ BroadcastHint(testRelation),
+ caseSensitive = true)
+
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("table"), table("TaBlE")),
+ testRelation,
+ caseSensitive = true)
+ }
+
+ test("multiple broadcast hint aliases") {
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("table", "table2"), table("table").join(table("table2"))),
+ Join(BroadcastHint(testRelation), BroadcastHint(testRelation2), Inner, None),
+ caseSensitive = false)
+ }
+
+ test("do not traverse past existing broadcast hints") {
+ checkAnalysis(
+ Hint("MAPJOIN", Seq("table"), BroadcastHint(table("table").where('a > 1))),
+ BroadcastHint(testRelation.where('a > 1)).analyze,
+ caseSensitive = false)
+ }
+
+ test("should work for subqueries") {
+ val relation = UnresolvedRelation(TableIdentifier("table"), Some("tableAlias"))
--- End diff --
nit: `table("table").as("tableAlias")`
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/16925
A late LGTM. : )
Normally, we do not encourage users to define hints inside a view. Users can add a BROADCAST hint when they define a persistent view. That will be stored in the catalog. The hint will take an effect, as long as this view is not cached.
Users can also use a BROADCAST hint for the operators (e.g., INTERSECT) that will be rewritten to a Join in optimizer.
```Scala
sql("CREATE VIEW v AS SELECT /*+ BROADCAST(u) */ key FROM u")
sql("SELECT /*+ BROADCAST(t) */ * FROM t INTERSECT SELECT * FROM v")
```
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16925
**[Test build #72879 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72879/testReport)** for PR 16925 at commit [`617f8a2`](https://github.com/apache/spark/commit/617f8a2b9d6069472729609c262ff4e4e2dde5f7).
* 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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72870/
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16925
**[Test build #72872 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72872/testReport)** for PR 16925 at commit [`c702e3e`](https://github.com/apache/spark/commit/c702e3ee8ee41ede216eab1209fe1e9374a2c301).
* 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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
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/16925#discussion_r101124529
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/SubstituteHints.scala ---
@@ -0,0 +1,103 @@
+/*
+ * 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.catalyst.analysis
+
+import org.apache.spark.sql.catalyst.CatalystConf
+import org.apache.spark.sql.catalyst.plans.logical._
+import org.apache.spark.sql.catalyst.rules.Rule
+import org.apache.spark.sql.catalyst.trees.CurrentOrigin
+
+
+/**
+ * Collection of rules related to hints. The only hint currently available is broadcast join hint.
+ *
+ * Note that this is separatedly into two rules because in the future we might introduce new hint
+ * rules that have different ordering requirements from broadcast.
+ */
+object SubstituteHints {
+
+ /**
+ * Substitute Hints.
+ *
+ * The only hint currently available is broadcast join hint.
+ *
+ * For broadcast hint, we accept "BROADCAST", "BROADCASTJOIN", and "MAPJOIN", and a sequence of
+ * relation aliases can be specified in the hint. A broadcast hint plan node will be inserted
+ * on top of any relation (that is not aliased differently), subquery, or common table expression
+ * that match the specified name.
+ *
+ * The hint resolution works by recursively traversing down the query plan to find a relation or
+ * subquery that matches one of the specified broadcast aliases. The traversal does not go past
+ * beyond any existing broadcast hints, subquery aliases.
+ *
+ * This rule must happen before common table expressions.
+ */
+ class SubstituteBroadcastHints(conf: CatalystConf) extends Rule[LogicalPlan] {
+ private val BROADCAST_HINT_NAMES = Set("BROADCAST", "BROADCASTJOIN", "MAPJOIN")
+
+ def resolver: Resolver = conf.resolver
+
+ private def applyBroadcastHint(plan: LogicalPlan, toBroadcast: Set[String]): LogicalPlan = {
+ // Whether to continue recursing down the tree
+ var recurse = true
+
+ val newNode = CurrentOrigin.withOrigin(plan.origin) {
+ plan match {
+ case r: UnresolvedRelation =>
+ val alias = r.alias.getOrElse(r.tableIdentifier.table)
+ if (toBroadcast.exists(resolver(_, alias))) BroadcastHint(plan) else plan
+ case r: SubqueryAlias =>
+ if (toBroadcast.exists(resolver(_, r.alias))) {
+ BroadcastHint(plan)
+ } else {
+ // Don't recurse down subquery aliases if there are no match.
+ recurse = false
+ plan
+ }
+ case _: BroadcastHint =>
+ // Found a broadcast hint; don't change the plan but also don't recurse down.
+ recurse = false
+ plan
+ case _ =>
+ plan
+ }
+ }
+
+ if ((plan fastEquals newNode) && recurse) {
+ newNode.mapChildren(child => applyBroadcastHint(child, toBroadcast))
+ } else {
+ newNode
+ }
+ }
+
+ def apply(plan: LogicalPlan): LogicalPlan = plan transformUp {
+ case h: Hint if BROADCAST_HINT_NAMES.contains(h.name.toUpperCase) =>
+ applyBroadcastHint(h.child, h.parameters.toSet)
+ }
+ }
+
+ /**
+ * Removes all the hints. This must be executed after all the other hint rules are executed.
--- End diff --
shall we mention that, this can happen when users specify invalid hints and we will ignore 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 issue #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72874/
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/16925
cc @dongjoon-hyun, @cloud-fan, @gatorsmile and @hvanhovell This should be ready for review. Note that the semantics is different from the earlier versions.
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16925
**[Test build #72874 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72874/testReport)** for PR 16925 at commit [`a095df3`](https://github.com/apache/spark/commit/a095df370aedbfd9a14e7761a9da18365cfd2bbf).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class SubstituteHints(conf: CatalystConf) extends Rule[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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101129634
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/SubstituteHints.scala ---
@@ -0,0 +1,103 @@
+/*
+ * 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.catalyst.analysis
+
+import org.apache.spark.sql.catalyst.CatalystConf
+import org.apache.spark.sql.catalyst.plans.logical._
+import org.apache.spark.sql.catalyst.rules.Rule
+import org.apache.spark.sql.catalyst.trees.CurrentOrigin
+
+
+/**
+ * Collection of rules related to hints. The only hint currently available is broadcast join hint.
+ *
+ * Note that this is separatedly into two rules because in the future we might introduce new hint
+ * rules that have different ordering requirements from broadcast.
+ */
+object SubstituteHints {
+
+ /**
+ * Substitute Hints.
+ *
+ * The only hint currently available is broadcast join hint.
+ *
+ * For broadcast hint, we accept "BROADCAST", "BROADCASTJOIN", and "MAPJOIN", and a sequence of
+ * relation aliases can be specified in the hint. A broadcast hint plan node will be inserted
+ * on top of any relation (that is not aliased differently), subquery, or common table expression
+ * that match the specified name.
+ *
+ * The hint resolution works by recursively traversing down the query plan to find a relation or
+ * subquery that matches one of the specified broadcast aliases. The traversal does not go past
+ * beyond any existing broadcast hints, subquery aliases.
+ *
+ * This rule must happen before common table expressions.
+ */
+ class SubstituteBroadcastHints(conf: CatalystConf) extends Rule[LogicalPlan] {
+ private val BROADCAST_HINT_NAMES = Set("BROADCAST", "BROADCASTJOIN", "MAPJOIN")
+
+ def resolver: Resolver = conf.resolver
+
+ private def applyBroadcastHint(plan: LogicalPlan, toBroadcast: Set[String]): LogicalPlan = {
+ // Whether to continue recursing down the tree
+ var recurse = true
+
+ val newNode = CurrentOrigin.withOrigin(plan.origin) {
+ plan match {
+ case r: UnresolvedRelation =>
+ val alias = r.alias.getOrElse(r.tableIdentifier.table)
+ if (toBroadcast.exists(resolver(_, alias))) BroadcastHint(plan) else plan
+ case r: SubqueryAlias =>
+ if (toBroadcast.exists(resolver(_, r.alias))) {
+ BroadcastHint(plan)
+ } else {
+ // Don't recurse down subquery aliases if there are no match.
+ recurse = false
+ plan
+ }
+ case _: BroadcastHint =>
+ // Found a broadcast hint; don't change the plan but also don't recurse down.
+ recurse = false
+ plan
+ case _ =>
+ plan
+ }
+ }
+
+ if ((plan fastEquals newNode) && recurse) {
+ newNode.mapChildren(child => applyBroadcastHint(child, toBroadcast))
+ } else {
+ newNode
+ }
+ }
+
+ def apply(plan: LogicalPlan): LogicalPlan = plan transformUp {
+ case h: Hint if BROADCAST_HINT_NAMES.contains(h.name.toUpperCase) =>
+ applyBroadcastHint(h.child, h.parameters.toSet)
+ }
+ }
+
+ /**
+ * Removes all the hints. This must be executed after all the other hint rules are executed.
--- End diff --
Yea let me add that.
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101288304
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -374,6 +374,16 @@ querySpecification
windows?)
;
+hint
+ : '/*+' hintStatement '*/'
+ ;
+
+hintStatement
+ : hintName=identifier
+ | hintName=identifier '(' parameters+=identifier parameters+=identifier ')'
--- End diff --
What does it support?
```
INDEX(a, b, c)
```
?
I think ti's a bit weird to support space as the delimiter.
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/16925
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72888/
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101289059
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -374,6 +374,16 @@ querySpecification
windows?)
;
+hint
+ : '/*+' hintStatement '*/'
+ ;
+
+hintStatement
+ : hintName=identifier
+ | hintName=identifier '(' parameters+=identifier parameters+=identifier ')'
--- End diff --
But we support `INDEX(a b)`?
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101129830
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -374,6 +374,16 @@ querySpecification
windows?)
;
+hint
+ : '/*+' hintStatement '*/'
+ ;
+
+hintStatement
+ : hintName=identifier
+ | hintName=identifier '(' parameters+=identifier parameters+=identifier ')'
--- End diff --
The purpose of this is to allow `SqlBase.g4` accept more general rule for the future.
So, possibly, it's for reducing the change in `SqlBase.g4` in order to add new hint rules.
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/16925
the latest commit hasn't finished running tests yet ... but probably fine given the small change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Merged build triggered.
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/16925
merging to master!
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16925
**[Test build #72870 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72870/testReport)** for PR 16925 at commit [`318bc03`](https://github.com/apache/spark/commit/318bc031985d178cb0ae7612cbee4d62636916b3).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class Hint(name: String, parameters: Seq[String], child: LogicalPlan) extends UnaryNode `
* `case class OverwriteOptions(`
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Merged build triggered.
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101122783
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -1002,8 +1012,12 @@ SIMPLE_COMMENT
: '--' ~[\r\n]* '\r'? '\n'? -> channel(HIDDEN)
;
+BRACKETED_EMPTY_COMMENT
+ : '/**/' -> channel(HIDDEN)
+ ;
+
BRACKETED_COMMENT
- : '/*' .*? '*/' -> channel(HIDDEN)
+ : '/*' ~[+] .*? '*/' -> channel(HIDDEN)
--- End diff --
It means `except +`.
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101289574
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -374,6 +374,16 @@ querySpecification
windows?)
;
+hint
+ : '/*+' hintStatement '*/'
+ ;
+
+hintStatement
+ : hintName=identifier
+ | hintName=identifier '(' parameters+=identifier parameters+=identifier ')'
--- End diff --
do we? we should disallow it. want to submit a pr?
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Merged build started.
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72879/
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16925
**[Test build #72888 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72888/testReport)** for PR 16925 at commit [`51a73d5`](https://github.com/apache/spark/commit/51a73d510a5faf3bbed8bb76ffe5590c68a67e2c).
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Merged build started.
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries - WIP
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Merged build triggered.
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Merged build started.
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72893/
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101289922
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -374,6 +374,16 @@ querySpecification
windows?)
;
+hint
+ : '/*+' hintStatement '*/'
+ ;
+
+hintStatement
+ : hintName=identifier
+ | hintName=identifier '(' parameters+=identifier parameters+=identifier ')'
--- End diff --
ok. let me prepare a tiny one.
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16925
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 pull request #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101088496
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/SubstituteHints.scala ---
@@ -0,0 +1,85 @@
+/*
+ * 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.catalyst.analysis
+
+import org.apache.spark.sql.catalyst.CatalystConf
+import org.apache.spark.sql.catalyst.plans.logical._
+import org.apache.spark.sql.catalyst.rules.Rule
+import org.apache.spark.sql.catalyst.trees.CurrentOrigin
+
+
+/**
+ * Substitute Hints.
+ * - BROADCAST/BROADCASTJOIN/MAPJOIN match the closest table with the given name parameters.
+ *
+ * In the case of broadcast hint, we find the frontier of
--- End diff --
this block needs to be updated.
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
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/16925#discussion_r101122146
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -1002,8 +1012,12 @@ SIMPLE_COMMENT
: '--' ~[\r\n]* '\r'? '\n'? -> channel(HIDDEN)
;
+BRACKETED_EMPTY_COMMENT
+ : '/**/' -> channel(HIDDEN)
+ ;
+
BRACKETED_COMMENT
- : '/*' .*? '*/' -> channel(HIDDEN)
+ : '/*' ~[+] .*? '*/' -> channel(HIDDEN)
--- End diff --
what does `~[+]` mean?
---
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 #16925: [SPARK-16475][SQL] Broadcast Hint for SQL Queries
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16925
**[Test build #72870 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72870/testReport)** for PR 16925 at commit [`318bc03`](https://github.com/apache/spark/commit/318bc031985d178cb0ae7612cbee4d62636916b3).
---
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 #16925: [SPARK-16475][SQL] Broadcast hint for SQL Queries
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/16925#discussion_r101129302
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala ---
@@ -493,4 +493,46 @@ class PlanParserSuite extends PlanTest {
assertEqual("select a, b from db.c where x !> 1",
table("db", "c").where('x <= 1).select('a, 'b))
}
+
+ test("select hint syntax") {
+ // Hive compatibility: Missing parameter raises ParseException.
+ 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("no viable alternative at input"))
+
+ comparePlans(
+ parsePlan("SELECT /*+ HINT */ * FROM t"),
+ Hint("HINT", Seq.empty, table("t").select(star())))
+
+ comparePlans(
+ parsePlan("SELECT /*+ BROADCASTJOIN(u) */ * FROM t"),
+ Hint("BROADCASTJOIN", Seq("u"), table("t").select(star())))
+
+ comparePlans(
+ parsePlan("SELECT /*+ MAPJOIN(u) */ * FROM t"),
+ Hint("MAPJOIN", Seq("u"), table("t").select(star())))
+
+ comparePlans(
+ parsePlan("SELECT /*+ STREAMTABLE(a,b,c) */ * FROM t"),
+ Hint("STREAMTABLE", Seq("a", "b", "c"), table("t").select(star())))
+
+ comparePlans(
+ parsePlan("SELECT /*+ INDEX(t emp_job_ix) */ * FROM t"),
--- End diff --
Yes. Both are considered equally now.
---
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