[SPARK-16475][SQL] broadcast hint for SQL queries - disallow space as the delimiter #16941
Conversation
| assert(m2.contains("mismatched input '.' expecting {')', ','}")) | ||
|
|
||
| // Disallow space as the delimiter. | ||
| val m3 = intercept[ParseException] { |
There was a problem hiding this comment.
NIT: You can use the PlanParserSuite.intercept method. That saves some typing.
|
LGTM pending jenkins |
|
|
||
| comparePlans( | ||
| parsePlan("SELECT /*+ INDEX(t emp_job_ix) */ * FROM t"), | ||
| parsePlan("SELECT /*+ INDEX(t, emp_job_ix) */ * FROM t"), |
There was a problem hiding this comment.
we can remove this test now, as it's duplicated if we use , as delimiter
There was a problem hiding this comment.
Hi, @viirya and @cloud-fan @hvanhovell @rxin @gatorsmile .
Just for explanation, it was originally designed to support other syntax like the followings. Except query block parts (which we don't support), this case means tableSpec and indexSpec.
https://docs.oracle.com/cd/B12037_01/server.101/b10752/hintsref.htm#21629
At that time, there was a request to provide more general syntax to prevent future changes on SqlBase.g4 layer.
Anyway, I have no objection on the current approach since I also read the comments between @rxin and @viirya on the previous @rxin 's commit.
There was a problem hiding this comment.
Yea we can generalize it later.
There was a problem hiding this comment.
@dongjoon-hyun Thanks for explanation. Looks like the syntax you referred only supports space as the delimiter?
|
Test build #72940 has finished for PR 16941 at commit
|
|
Merging in master. |
… the delimiter ## What changes were proposed in this pull request? A follow-up to disallow space as the delimiter in broadcast hint. ## How was this patch tested? Jenkins test. Please review http://spark.apache.org/contributing.html before opening a pull request. Author: Liang-Chi Hsieh <[email protected]> Closes apache#16941 from viirya/disallow-space-delimiter.
6 participants
What changes were proposed in this pull request?
A follow-up to disallow space as the delimiter in broadcast hint.
How was this patch tested?
Jenkins test.
Please review http://spark.apache.org/contributing.html before opening a pull request.