Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-25431][SQL][EXAMPLES] Fix function examples and unify the format of the example results. #22421

Closed
wants to merge 1 commit into from

Conversation

ueshin
Copy link
Member

@ueshin ueshin commented Sep 14, 2018

What changes were proposed in this pull request?

There are some mistakes in examples of newly added functions. Also the format of the example results are not unified. We should fix and unify them.

How was this patch tested?

Manually executed the examples.

@SparkQA
Copy link

SparkQA commented Sep 14, 2018

Test build #96069 has finished for PR 22421 at commit baed43a.

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

Copy link
Member

@gatorsmile gatorsmile left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks! Merged to master/2.4

@asfgit asfgit closed this in 9c25d7f Sep 14, 2018
asfgit pushed a commit that referenced this pull request Sep 14, 2018
…at of the example results.

## What changes were proposed in this pull request?

There are some mistakes in examples of newly added functions. Also the format of the example results are not unified. We should fix and unify them.

## How was this patch tested?

Manually executed the examples.

Closes #22421 from ueshin/issues/SPARK-25431/fix_examples.

Authored-by: Takuya UESHIN <ueshin@databricks.com>
Signed-off-by: gatorsmile <gatorsmile@gmail.com>
(cherry picked from commit 9c25d7f)
Signed-off-by: gatorsmile <gatorsmile@gmail.com>
@@ -131,7 +131,7 @@ case class Size(child: Expression) extends UnaryExpression with ExpectsInputType
examples = """
Examples:
> SELECT _FUNC_(map(1, 'a', 2, 'b'));
[1,2]
[1, 2]
Copy link
Member

Choose a reason for hiding this comment

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

?

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Sep 14, 2018

@ueshin and @gatorsmile .

Sorry, but could you explain where did you get the new result? IIRC, historically, the example illustrates the result from spark-sql like Hive description. The new change doesn't consistent to the master result.

spark-sql> describe function extended map_concat;
Function: map_concat
Class: org.apache.spark.sql.catalyst.expressions.MapConcat
Usage: map_concat(map, ...) - Returns the union of all the given maps
Extended Usage:
    Examples:
      > SELECT map_concat(map(1, 'a', 2, 'b'), map(2, 'c', 3, 'd'));
       [1 -> "a", 2 -> "b", 2 -> "c", 3 -> "d"]

    Since: 2.4.0

spark-sql> SELECT map_concat(map(1, 'a', 2, 'b'), map(2, 'c', 3, 'd'));
{1:"a",2:"c",3:"d"}
Time taken: 0.034 seconds, Fetched 1 row(s)

@dongjoon-hyun
Copy link
Member

If you don't mind, please revert this.
cc @cloud-fan

@@ -718,7 +718,7 @@ case class MapConcat(children: Seq[Expression]) extends ComplexTypeMergingExpres
examples = """
Examples:
> SELECT _FUNC_(array(struct(1, 'a'), struct(2, 'b')));
{1:"a",2:"b"}
[1 -> "a", 2 -> "b"]
Copy link
Member

Choose a reason for hiding this comment

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

The previous one is correct.

spark-sql> SELECT map_from_entries(array(struct(1, 'a'), struct(2, 'b')));
{1:"a",2:"b"}

@ueshin
Copy link
Member Author

ueshin commented Sep 15, 2018

@dongjoon-hyun I picked the string from sql(...).show(truncate = false) and modified a little. I'm sorry but I didn't notice that we use the spark-sql to pick the result. Let me send a follow-up pr.

@dongjoon-hyun
Copy link
Member

@ueshin . Could you revert the current one and send a new one please?

@ueshin
Copy link
Member Author

ueshin commented Sep 15, 2018

@dongjoon-hyun Sure, reverted. Thanks!

@dongjoon-hyun
Copy link
Member

Thank you, @ueshin .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants