Skip to content

Conversation

@xuanyuanking
Copy link
Member

What changes were proposed in this pull request?

Update the Spark SQL document menu and join strategy hints.

Why are the changes needed?

  • Several new changes in the Spark SQL document didn't change the menu-sql.yaml correspondingly.
  • Update the demo code for join strategy hints.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Document change only.

@xuanyuanking
Copy link
Member Author

cc @cloud-fan @gatorsmile

@SparkQA
Copy link

SparkQA commented Dec 17, 2019

Test build #115422 has finished for PR 26917 at commit 7222fa3.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

{% highlight r %}
src <- sql("SELECT * FROM src")
records <- sql("SELECT * FROM records")
head(join(broadcast(src), records, src$key == records$key))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did we remove the broadcast method?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the broadcast method still available:

spark/R/pkg/R/DataFrame.R

Lines 4129 to 4134 in 18431c7

setMethod("broadcast",
signature(x = "SparkDataFrame"),
function(x) {
sdf <- callJStatic("org.apache.spark.sql.functions", "broadcast", x@sdf)
dataFrame(sdf)
})

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so what's the recommended API to add hint? df.hint(...) or hint(df, ...)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hint(df, ...) is for R, there's no df.hint(...) in R?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah nvm, didn't realize it's R code

@maropu
Copy link
Member

maropu commented Dec 17, 2019

How about adding the docs about the SQL case, too? #25464

@xuanyuanking
Copy link
Member Author

@maropu We have the SQL hint here: https://github.com/apache/spark/pull/26917/files#diff-ecc0a33250015b4a0fb4a5f1de5cc502R167, do you mean add demo for partition by hint?

@maropu
Copy link
Member

maropu commented Dec 17, 2019

do you mean add demo for partition by hint?

Yea, yes. we can accept REPARTITION_BY_RANGE and REPARTITION now.

@xuanyuanking
Copy link
Member Author

Sure, done in accf45c, please take a look.

@SparkQA
Copy link

SparkQA commented Dec 18, 2019

Test build #115516 has finished for PR 26917 at commit accf45c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 2acae97 Dec 27, 2019
@xuanyuanking
Copy link
Member Author

Thanks!

@xuanyuanking xuanyuanking deleted the SPARK-30278 branch December 27, 2019 05:24
@maropu
Copy link
Member

maropu commented Dec 28, 2019

Thanks for the update, @xuanyuanking !

fqaiser94 pushed a commit to fqaiser94/spark that referenced this pull request Mar 30, 2020
### What changes were proposed in this pull request?
Update the Spark SQL document menu and join strategy hints.

### Why are the changes needed?
- Several new changes in the Spark SQL document didn't change the menu-sql.yaml correspondingly.
- Update the demo code for join strategy hints.

### Does this PR introduce any user-facing change?
No.

### How was this patch tested?
Document change only.

Closes apache#26917 from xuanyuanking/SPARK-30278.

Authored-by: Yuanjian Li <xyliyuanjian@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants