Skip to content

Conversation

@bomeng
Copy link
Contributor

@bomeng bomeng commented Apr 21, 2016

What changes were proposed in this pull request?

Currently SET and SET -v commands are similar to Hive SET command except the following difference:

  1. The result is not sorted;
  2. When using SET and SET -v, in addition to the Hive related properties, it will also list all the system properties and environment properties, which is very useful in some cases.

This JIRA is trying to make the current SET command more consistent to Hive output.

How was this patch tested?

Current test cases SQLQuerySuite and HiveQuerySuite should have enough test coverage.

@SparkQA
Copy link

SparkQA commented Apr 21, 2016

Test build #56582 has finished for PR 12583 at commit 01d3e5a.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@rxin
Copy link
Contributor

rxin commented Apr 21, 2016

You might need to rebase this in a bit because i'm refactoring all of these quite heavily for Spark 2.0.

}

test("set / set -v") {
checkExistence(sql("set"), true, "env:", "system:")
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add a test case that checks the value of SPARK_TESTING?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SPARK_TESTING is not in sys.env nor sys.props while running the test cases. Any suggestions to add the test cases for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Found out that current test cases SQLQuerySuite and HiveQuerySuite have intensive test coverage for SET command.

@SparkQA
Copy link

SparkQA commented Apr 21, 2016

Test build #56585 has finished for PR 12583 at commit 790cfda.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 22, 2016

Test build #56603 has finished for PR 12583 at commit 2a077bd.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 22, 2016

Test build #56636 has finished for PR 12583 at commit e3ba716.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 22, 2016

Test build #56651 has finished for PR 12583 at commit c6805e4.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 22, 2016

Test build #56653 has finished for PR 12583 at commit 99ff8a5.

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

@rxin
Copy link
Contributor

rxin commented Apr 22, 2016

@bomeng while you are at this, do you mind implementing https://issues.apache.org/jira/browse/SPARK-14806 together? Should be pretty trivial. Thanks.

@SparkQA
Copy link

SparkQA commented Apr 22, 2016

Test build #56688 has finished for PR 12583 at commit 6c7276b.

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

@dongjoon-hyun
Copy link
Member

Hi, @bomeng .
I hit the same issue. Could you update the PR?

(keyValueOutput, runFunc)

// Queries all properties along with their default values and docs that are defined in the
// SQLConf of the sqlContext.
Copy link
Member

Choose a reason for hiding this comment

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

We should update the comment regarding the system environment properties.

(keyValueOutput, runFunc)

// (In Hive, "SET" returns all changed properties while "SET -v" returns all properties.)
// Queries all key-value pairs that are set in the SQLConf of the sqlContext.
Copy link
Member

Choose a reason for hiding this comment

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

Same.

override def run(sqlContext: SQLContext): Seq[Row] = runFunc(sqlContext)

/**
* get the system environment properties as a sequence
Copy link
Member

Choose a reason for hiding this comment

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

Nit: "get" -> "Gets", "as a sequence" -> "as a sequence of rows".

@dongjoon-hyun
Copy link
Member

Hmm. This PR seems to be stale. Actually, what I wanted was a sorted result of SET and SET -v.

@dongjoon-hyun
Copy link
Member

I made another #16579 for SET for a sorted result.

@HyukjinKwon
Copy link
Member

@dongjoon-hyun, then, do you mind if I ask if this PR is not useful in favour of #16579?

@dongjoon-hyun
Copy link
Member

Hi, @bomeng , do you have something to refresh here?
I think we can close this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants