You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mallman <gi...@git.apache.org> on 2016/11/23 22:21:59 UTC
[GitHub] spark pull request #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
GitHub user mallman opened a pull request:
https://github.com/apache/spark/pull/15998
[SPARK-18572][SQL] Add a method `listPartitionName` to `ExternalCatalog`
(Link to Jira issue: https://issues.apache.org/jira/browse/SPARK-18572)
## What changes were proposed in this pull request?
Currently Spark answers the `SHOW PARTITIONS` command by fetching all of the table's partition metadata from the external catalog and constructing partition names therefrom. The Hive client has a `getPartitionNames` method which is many times faster for this purpose, with the performance improvement scaling with the number of partitions in a table.
To test the performance impact of this PR, I ran the `SHOW PARTITIONS` command on two Hive tables with large numbers of partitions. One table has ~17,800 partitions, and the other has ~95,000 partitions. For the purposes of this PR, I'll call the former table `table1` and the latter table `table2`. I ran 5 trials for each table with before-and-after versions of this PR. The results are as follows:
Spark at bdc8153, `SHOW PARTITIONS table1`, times in seconds:
7.901
3.983
4.018
4.331
4.261
Spark at bdc8153, `SHOW PARTITIONS table2`
(Timed out after 10 minutes with a `SocketTimeoutException`.)
Spark at this PR, `SHOW PARTITIONS table1`, times in seconds:
3.801
0.449
0.395
0.348
0.336
Spark at this PR, `SHOW PARTITIONS table2`, times in seconds:
5.184
1.63
1.474
1.519
1.41
Taking the best times from each trial, we get a 12x performance improvement for a table with ~17,800 partitions and at least a 426x improvement for a table with ~95,000 partitions. More significantly, the latter command doesn't even complete with the current code in master.
This is actually a patch we've been using in-house at VideoAmp since Spark 1.1. It's made all the difference in the practical usability of our largest tables. Even with tables with about 1,000 partitions there's a performance improvement of about 2-3x.
## How was this patch tested?
I added a unit test to `VersionsSuite` which tests that the Hive client's `getPartitionNames` method returns the correct number of partitions.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/VideoAmp/spark-public spark-18572-list_partition_names
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/15998.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 #15998
----
commit f20ef8bd0bbc68396546c60bb816fe9caf02042f
Author: Michael Allman <mi...@videoamp.com>
Date: 2016-11-23T21:50:14Z
[SPARK-18572][SQL] Add a method `listPartitionName` to `ExternalCatalog`
and an implementation in `HiveExternalCatalog` that calls the Hive
`getPartitionNames` method.
----
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
I will work on additional unit test coverage tomorrow.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/69691/
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
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/15998#discussion_r90181774
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
@@ -922,6 +924,25 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
/**
* Returns the partition names from hive metastore for a given table in a database.
*/
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = withClient {
+ val actualPartColNames = getTable(db, table).partitionColumnNames
+ val clientPartitionNames =
+ client.getPartitionNames(db, table, partialSpec.map(lowerCasePartitionSpec))
+ clientPartitionNames.map { partName =>
+ val partSpec = PartitioningUtils.parsePathFragmentAsSeq(partName)
+ partSpec.map { case (partName, partValue) =>
+ escapePathName(actualPartColNames.find(_.equalsIgnoreCase(partName)).get) + "=" +
--- End diff --
can we just build a map with lower cased partition column name as key, and path-escaped actual partition column name as value? then we can do a simple map lookup here, i.e. `map.get(partName.toLowerCase)`
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r90103384
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalog.scala ---
@@ -189,11 +189,28 @@ abstract class ExternalCatalog {
spec: TablePartitionSpec): Option[CatalogTablePartition]
/**
+ * List the names of all partitions that belong to the specified table, assuming it exists.
--- End diff --
Could you specify the return format of partitions (e.g. `/a=v1,b=v2`) and that the values are escape, and that callers can decode it using the functions found in PartitioningUtils?
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/15998
I think we can just fix it in this PR, it's only several lines 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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/15998
What is the expected output? In the ExternalCatalogSuite, it sounds like we do not issue any error.
```
catalog.listPartitionNames("db2", "tbl2", Some(Map("unknown" -> "unknown")))
```
In the `SessionCatalogSuite`, we also should add the test cases like [`listPartitions`](https://github.com/apache/spark/blob/e64a2047eaf02d65dcf98b6e0710e10196aa74b1/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala#L872-L889) and also the negative cases like what I mentioned above.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/15998
LGTM, @rxin shall we backport this? Although it's not a bug, it's a scalability issue.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/15998
Yea this one seems like directly related to the work we did in 2.1 for large table handling.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r91006034
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogSuite.scala ---
@@ -346,6 +346,31 @@ abstract class ExternalCatalogSuite extends SparkFunSuite with BeforeAndAfterEac
assert(new Path(partitionLocation) == defaultPartitionLocation)
}
+ test("list partition names") {
+ val catalog = newBasicCatalog()
+ val newPart = CatalogTablePartition(Map("a" -> "1", "b" -> "%="), storageFormat)
+ catalog.createPartitions("db2", "tbl2", Seq(newPart), ignoreIfExists = false)
+
+ val partitionNames = catalog.listPartitionNames("db2", "tbl2")
+ assert(partitionNames == Seq("a=1/b=%25%3D", "a=1/b=2", "a=3/b=4"))
+ }
+
+ test("list partition names with partial partition spec") {
+ val catalog = newBasicCatalog()
+ val newPart = CatalogTablePartition(Map("a" -> "1", "b" -> "%="), storageFormat)
+ catalog.createPartitions("db2", "tbl2", Seq(newPart), ignoreIfExists = false)
+
+ val partitionNames1 = catalog.listPartitionNames("db2", "tbl2", Some(Map("a" -> "1")))
+ assert(partitionNames1 == Seq("a=1/b=%25%3D", "a=1/b=2"))
--- End diff --
Yeah, I tried Hive 1.2. It actually returns the weird value.
```
hive> create table partTab (col1 int, col2 int) partitioned by (pcol1 String, pcol2 String);
OK
hive> insert into table partTab partition(pcol1='1', pcol2='2') select 3, 4 from dummy;
OK
hive> insert into table partTab partition(pcol1='1', pcol2='%=') select 3, 4 from dummy;
OK
hive> show partitions partTab;
OK
pcol1=1/pcol2=%25%3D
pcol1=1/pcol2=2
hive> show partitions partTab PARTITION(pcol1=1);
OK
pcol1=1/pcol2=2
pcol1=1/pcol2=%25%3D
```
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r90164420
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalog.scala ---
@@ -189,11 +189,28 @@ abstract class ExternalCatalog {
spec: TablePartitionSpec): Option[CatalogTablePartition]
/**
+ * List the names of all partitions that belong to the specified table, assuming it exists.
--- End diff --
`PartitioningUtils` is in SQL core, not catalyst. I decided to use the `escapePathName` and `unescapePathName` methods directly.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
> @mallman do you know which tests fail the partition spec checking? It looks to me that before we call partition related API in SessionCatalog, the partition column names should be normalized already.
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69581/testReport/junit/org.apache.spark.sql.hive/PartitionProviderCompatibilitySuite/SPARK_18659_insert_overwrite_table_with_lowercase___partition_management_true/
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69491 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69491/consoleFull)** for PR 15998 at commit [`b15d0d2`](https://github.com/apache/spark/commit/b15d0d2848cc0bb9a6f706c0f9ccb76ef6e222b8).
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/15998
@mallman do you know which tests fail the partition spec checking? It looks to me that before we call partition related API in `SessionCatalog`, the partition column names should be normalized already.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89839435
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -730,6 +730,23 @@ class SessionCatalog(
}
/**
+ * List the names of all partitions that belong to the specified table, assuming it exists.
+ *
+ * A partial partition spec may optionally be provided to filter the partitions returned.
+ * For instance, if there exist partitions (a='1', b='2'), (a='1', b='3') and (a='2', b='4'),
+ * then a partial spec of (a='1') will return the first two only.
+ */
+ def listPartitionNames(
+ tableName: TableIdentifier,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = {
+ val db = formatDatabaseName(tableName.database.getOrElse(getCurrentDatabase))
+ val table = formatTableName(tableName.table)
+ requireDbExists(db)
+ requireTableExists(TableIdentifier(table, Option(db)))
--- End diff --
I don't know. The only method that calls that method is `dropPartitions`.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/69501/
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/15998
@mallman , https://github.com/apache/spark/commit/f4c48e1d61dc4d1fc581b78dd9e5c948965acad5 might be what @cloud-fan wants.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/69378/
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69652 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69652/consoleFull)** for PR 15998 at commit [`095cdd5`](https://github.com/apache/spark/commit/095cdd51c386e1747cc8645851b0cdffa83a5a70).
* 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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/15998
@mallman ah it's a mistake, check https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceStrategy.scala#L228 , we need to move this to place before https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceStrategy.scala#L198, so that the static partitions are normalized.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89681535
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -730,6 +730,23 @@ class SessionCatalog(
}
/**
+ * List the names of all partitions that belong to the specified table, assuming it exists.
+ *
+ * A partial partition spec may optionally be provided to filter the partitions returned.
+ * For instance, if there exist partitions (a='1', b='2'), (a='1', b='3') and (a='2', b='4'),
+ * then a partial spec of (a='1') will return the first two only.
+ */
+ def listPartitionNames(
+ tableName: TableIdentifier,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = {
+ val db = formatDatabaseName(tableName.database.getOrElse(getCurrentDatabase))
+ val table = formatTableName(tableName.table)
+ requireDbExists(db)
+ requireTableExists(TableIdentifier(table, Option(db)))
--- End diff --
Do we need to call `requirePartialMatchedPartitionSpec` here?
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69089 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69089/consoleFull)** for PR 15998 at commit [`f20ef8b`](https://github.com/apache/spark/commit/f20ef8bd0bbc68396546c60bb816fe9caf02042f).
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69691 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69691/consoleFull)** for PR 15998 at commit [`37fc595`](https://github.com/apache/spark/commit/37fc5955b2a26caa5ef133d19e415172a26330a2).
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69683 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69683/consoleFull)** for PR 15998 at commit [`37fc595`](https://github.com/apache/spark/commit/37fc5955b2a26caa5ef133d19e415172a26330a2).
* 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 issue #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69378 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69378/consoleFull)** for PR 15998 at commit [`48ae2a3`](https://github.com/apache/spark/commit/48ae2a39f8c5365c9f8e087333fc719b901517c2).
* 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 pull request #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89656749
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
@@ -922,6 +923,29 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
/**
* Returns the partition names from hive metastore for a given table in a database.
*/
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = withClient {
+ val actualPartColNames = getTable(db, table).partitionColumnNames
+ val clientPartitionNames =
+ client.getPartitionNames(db, table, partialSpec.map(lowerCasePartitionSpec))
+
+ if (actualPartColNames.exists(partColName => partColName != partColName.toLowerCase)) {
+ clientPartitionNames.map { partName =>
+ val partSpec = PartitioningUtils.parsePathFragmentAsSeq(partName)
--- End diff --
Is the (un)escaping here correct? It would be nice to have a unit test to verify these edge cases.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
The test that failed is definitely related to this PR, however it passes for me locally. I'll investigate...
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
> where is the speed-up come from? Is it because the hive API getPartitionNames is faster than getPartitions? Or is it because we generate the partition string(a=1/b=2/c=3) at hive side and it's faster than Spark?
To answer `getPartitionNames`, Hive does a simple `SELECT` on a table in the metastore. It doesn't not transform the results\u2014the partition names are in a column in that table. That's where the speed up comes from.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
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/15998#discussion_r90756432
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -519,6 +519,26 @@ private[hive] class HiveClientImpl(
client.alterPartitions(table, newParts.map { p => toHivePartition(p, hiveTable) }.asJava)
}
+ /**
+ * Returns the partition names for the given table that match the supplied partition spec.
+ * If no partition spec is specified, all partitions are returned.
+ *
+ * The returned sequence is sorted as strings.
--- End diff --
but according to how we use this API in Spark SQL, we don't need to sort right? maybe we can just do the sorting in test.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/15998
This depends on `spark.sql.caseSensitive`, right? cc @ericl @cloud-fan
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
I've modified the behavior of the partition spec checking methods in `SessionCatalog` to test for case-sensitive analysis.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89411482
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalog.scala ---
@@ -189,6 +189,38 @@ abstract class ExternalCatalog {
spec: TablePartitionSpec): Option[CatalogTablePartition]
/**
+ * List the names of all partitions that belong to the specified table, assuming it exists.
+ *
+ * A partial partition spec may optionally be provided to filter the partitions returned.
+ * For instance, if there exist partitions (a='1', b='2'), (a='1', b='3') and (a='2', b='4'),
+ * then a partial spec of (a='1') will return the first two only.
+ *
+ * We provide a default implementation here which simply delegates to the `listPartitions`
+ * method. For efficiency's sake, overriding this method is recommended for external catalogs
+ * that can list partition names directly.
+ * @param db database name
+ * @param table table name
+ * @param partialSpec partition spec
+ */
+ def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = {
--- End diff --
My worry is that when we create a new implementation of the external catalog, we'd forget to implement this, leading to bad performance.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69501 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69501/consoleFull)** for PR 15998 at commit [`3ad8c9a`](https://github.com/apache/spark/commit/3ad8c9a2e5d613e13eeb4bf95334021e478a450b).
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89679682
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
@@ -922,6 +923,29 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
/**
* Returns the partition names from hive metastore for a given table in a database.
*/
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = withClient {
+ val actualPartColNames = getTable(db, table).partitionColumnNames
+ val clientPartitionNames =
+ client.getPartitionNames(db, table, partialSpec.map(lowerCasePartitionSpec))
+
+ if (actualPartColNames.exists(partColName => partColName != partColName.toLowerCase)) {
+ clientPartitionNames.map { partName =>
+ val partSpec = PartitioningUtils.parsePathFragmentAsSeq(partName)
--- End diff --
I don't think this is correct, and based on what I've found in the Hive source code (and reading the DDL grammar on the Hive wiki) I'm going to push a change with another proposal for partition column name/value handling.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69491 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69491/consoleFull)** for PR 15998 at commit [`b15d0d2`](https://github.com/apache/spark/commit/b15d0d2848cc0bb9a6f706c0f9ccb76ef6e222b8).
* 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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89680470
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
@@ -922,6 +923,29 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
/**
* Returns the partition names from hive metastore for a given table in a database.
*/
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = withClient {
+ val actualPartColNames = getTable(db, table).partitionColumnNames
+ val clientPartitionNames =
+ client.getPartitionNames(db, table, partialSpec.map(lowerCasePartitionSpec))
+
+ if (actualPartColNames.exists(partColName => partColName != partColName.toLowerCase)) {
+ clientPartitionNames.map { partName =>
+ val partSpec = PartitioningUtils.parsePathFragmentAsSeq(partName)
--- End diff --
I've run some tests to compare behavior between Hive and Spark in handling gnarly partition column names, and I found some disparities. We've spent a considerable amount of time wrangling with partition column name handling recently, and I'm not sure what semantics we've decided on. To ensure the behavior I'm seeing is what we're expecting, I want to describe a scenario I ran.
In my test scenario, I created a table named `test` with the stock Hive 2.1.0 distribution. (I simply downloaded it from its download page and initialized an empty Derby schema store.) The exact DDL I used to create this table is as follows:
```create table test(a string) partitioned by (`P``\u0414r t` int);```
When I do a `describe test` with `hive` it shows the column name as ``p`\u0434r t``. It appears to lowercase the P and the cyrillic \u0414 before storing the table schema it in the metastore. I then run
```alter table test add partition(`P``\u0414r t`=0);```
When I run `show partitions test` in `hive` it gives me ``p`\u0434r t=0``. Additionally, when I list the contents of the `test` table's base directory in HDFS, the partition directory entry is
```/user/hive/warehouse/test/p`\u0434r t=0```
If I drop the table, create it with `spark-sql` using the same DDL as before and do a `describe test`, the partition column is given as ``P`\u0414r t``. Spark has preserved the case of the partition column name. If I then do
```alter table test add partition(`P``\u0414r t`=0);```
in `spark-sql` and `show partitions test` I get ``P`\u0414r t=0``. When I list the directory contents in HDFS, I get
```/user/hive/warehouse/test/P`\u0414r t=0```
The upshot is Hive is lowercasing the partition column name and Spark is leaving it unaltered. Is this correct?
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on the issue:
https://github.com/apache/spark/pull/15998
@mallman I'll take a look today
On Tue, Nov 29, 2016, 9:45 AM Michael Allman <no...@github.com>
wrote:
> Hi Guys,
>
> Repeating my comment/query for @ericl <https://github.com/ericl>. I'm
> hoping someone can provide affirmation/refutation to my question before I
> proceed with new unit tests.
>
> I've run some tests to compare behavior between Hive and Spark in handling
> gnarly partition column names, and I found some disparities. We've spent a
> considerable amount of time wrangling with partition column name handling
> recently, and I'm not sure what semantics we've decided on. To ensure the
> behavior I'm seeing is what we're expecting, I want to describe a scenario
> I ran.
>
> In my test scenario, I created a table named test with the stock Hive
> 2.1.0 distribution. (I simply downloaded it from its download page and
> initialized an empty Derby schema store.) The exact DDL I used to create
> this table is as follows:
>
> create table test(a string) partitioned by (`P``\u0414r t` int);
>
> When I do a describe test with hive it shows the column name as p`\u0434r t.
> It appears to lowercase the P and the cyrillic \u0414 before storing the table
> schema it in the metastore. I then run
>
> alter table test add partition(`P``\u0414r t`=0);
>
> When I run show partitions test in hive it gives me p`\u0434r t=0.
> Additionally, when I list the contents of the test table's base directory
> in HDFS, the partition directory entry is
>
> /user/hive/warehouse/test/p`\u0434r t=0
>
> If I drop the table, create it with spark-sql using the same DDL as
> before and do a describe test, the partition column is given as P`\u0414r t.
> Spark has preserved the case of the partition column name. If I then do
>
> alter table test add partition(`P``\u0414r t`=0);
>
> in spark-sql and show partitions test I get P`\u0414r t=0. When I list the
> directory contents in HDFS, I get
>
> /user/hive/warehouse/test/P`\u0414r t=0
>
> The upshot is Hive is lowercasing the partition column name and Spark is
> leaving it unaltered. Is this correct?
>
> \u2014
>
>
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/spark/pull/15998#issuecomment-263643263>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AAA6SiqmPcATOuBJeYtPOJSvMJPCwjRfks5rDGS9gaJpZM4K7Ggx>
> .
>
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
@gatorsmile I enhanced the coverage of `SessionCatalog.listPartitions` and `SessionCatalog.listPartitionNames` to include tests for invalid partial partition specs.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on the issue:
https://github.com/apache/spark/pull/15998
Spark SQL is case insensitive by default. As @gatorsmile mentioned it might be good to check that flag, though I don't know how well case sensitive mode works when enabled.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69691 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69691/consoleFull)** for PR 15998 at commit [`37fc595`](https://github.com/apache/spark/commit/37fc5955b2a26caa5ef133d19e415172a26330a2).
* 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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69174 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69174/consoleFull)** for PR 15998 at commit [`4e03c3e`](https://github.com/apache/spark/commit/4e03c3e46d22e5fe1b1fbc01ea57ef15d2723b9b).
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/15998
retest this please
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89678628
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala ---
@@ -482,6 +482,19 @@ class InMemoryCatalog(
}
}
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = synchronized {
+ val partitionColumnNames = getTable(db, table).partitionColumnNames
+
+ listPartitions(db, table, partialSpec).map { partition =>
+ partitionColumnNames.map { name =>
+ name + "=" + partition.spec(name)
--- End diff --
I looked into how Hive escapes partition names. It does not appear to do any escaping at all. The partition names are in a string column in a table in the metastore database. Hive simply queries that partition name column on this table, filtering by Hive database and table, sorts the results and returns them.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
@ericl @gatorsmile Please see test failure here: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69581/testReport/junit/org.apache.spark.sql.hive/PartitionProviderCompatibilitySuite/SPARK_18659_insert_overwrite_table_with_lowercase___partition_management_true/
Are partial partition specs supposed to be case insensitive? Should I just alter the `SessionCatalog.requirePartialMatchedPartitionSpec` method to do a case-insensitive comparison instead of an exact match?
Thanks.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89679527
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
@@ -922,6 +923,29 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
/**
* Returns the partition names from hive metastore for a given table in a database.
*/
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = withClient {
+ val actualPartColNames = getTable(db, table).partitionColumnNames
+ val clientPartitionNames =
+ client.getPartitionNames(db, table, partialSpec.map(lowerCasePartitionSpec))
+
+ if (actualPartColNames.exists(partColName => partColName != partColName.toLowerCase)) {
+ clientPartitionNames.map { partName =>
+ val partSpec = PartitioningUtils.parsePathFragmentAsSeq(partName)
--- End diff --
Sorry, I just realized that isn't exactly what you were asking about.
I think adding coverage for mixed case partition column names is a good thing, but I'll address path (un)escaping as well.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89679477
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalog.scala ---
@@ -189,6 +189,21 @@ abstract class ExternalCatalog {
spec: TablePartitionSpec): Option[CatalogTablePartition]
/**
+ * List the names of all partitions that belong to the specified table, assuming it exists.
+ *
+ * A partial partition spec may optionally be provided to filter the partitions returned.
+ * For instance, if there exist partitions (a='1', b='2'), (a='1', b='3') and (a='2', b='4'),
+ * then a partial spec of (a='1') will return the first two only.
--- End diff --
Newline added here and in comment for `listPartitions`.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69501 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69501/consoleFull)** for PR 15998 at commit [`3ad8c9a`](https://github.com/apache/spark/commit/3ad8c9a2e5d613e13eeb4bf95334021e478a450b).
* 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 pull request #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r90503688
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClient.scala ---
@@ -155,6 +155,25 @@ private[hive] trait HiveClient {
}
}
+ /**
+ * Returns the partition names for the given table that match the supplied partition spec.
+ * If no partition spec is specified, all partitions are returned.
+ */
+ final def getPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec]): Seq[String] = {
+ getPartitionNames(getTable(db, table), partialSpec)
+ }
+
+ /**
+ * Returns the partition names for the given table that match the supplied partition spec.
+ * If no partition spec is specified, all partitions are returned.
+ */
+ def getPartitionNames(
--- End diff --
Good question. I guess I'm just following the example of the two `getPartitions` methods.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/15998
thanks, merging to master/2.1!
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69581 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69581/consoleFull)** for PR 15998 at commit [`afec06d`](https://github.com/apache/spark/commit/afec06d0f8a8bc592dffd8c4ae66743d42fde551).
* 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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89656487
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala ---
@@ -482,6 +482,19 @@ class InMemoryCatalog(
}
}
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = synchronized {
+ val partitionColumnNames = getTable(db, table).partitionColumnNames
+
+ listPartitions(db, table, partialSpec).map { partition =>
+ partitionColumnNames.map { name =>
+ name + "=" + partition.spec(name)
--- End diff --
Does this need escaping, as provided by escapePathName?
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
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/15998#discussion_r91006319
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogSuite.scala ---
@@ -346,6 +346,31 @@ abstract class ExternalCatalogSuite extends SparkFunSuite with BeforeAndAfterEac
assert(new Path(partitionLocation) == defaultPartitionLocation)
}
+ test("list partition names") {
+ val catalog = newBasicCatalog()
+ val newPart = CatalogTablePartition(Map("a" -> "1", "b" -> "%="), storageFormat)
+ catalog.createPartitions("db2", "tbl2", Seq(newPart), ignoreIfExists = false)
+
+ val partitionNames = catalog.listPartitionNames("db2", "tbl2")
+ assert(partitionNames == Seq("a=1/b=%25%3D", "a=1/b=2", "a=3/b=4"))
+ }
+
+ test("list partition names with partial partition spec") {
+ val catalog = newBasicCatalog()
+ val newPart = CatalogTablePartition(Map("a" -> "1", "b" -> "%="), storageFormat)
+ catalog.createPartitions("db2", "tbl2", Seq(newPart), ignoreIfExists = false)
+
+ val partitionNames1 = catalog.listPartitionNames("db2", "tbl2", Some(Map("a" -> "1")))
+ assert(partitionNames1 == Seq("a=1/b=%25%3D", "a=1/b=2"))
--- End diff --
ok, maybe we should consider diverging from Hive here...
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/15998
Like the other partition ExternalCatalog APIs, could you also add the negative test cases to `ExternalCatalogSuite.scala`?
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69195 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69195/consoleFull)** for PR 15998 at commit [`594c693`](https://github.com/apache/spark/commit/594c6934c4fe4e41a895ea91b3b2cc45f38eaeb6).
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/15998
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
> can you also address this comment? #15998 (comment)
Addressed.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
Hi Guys,
Repeating my comment/query for @ericl. I'm hoping someone can provide affirmation/refutation to my question before I proceed with new unit tests.
I've run some tests to compare behavior between Hive and Spark in handling gnarly partition column names, and I found some disparities. We've spent a considerable amount of time wrangling with partition column name handling recently, and I'm not sure what semantics we've decided on. To ensure the behavior I'm seeing is what we're expecting, I want to describe a scenario I ran.
In my test scenario, I created a table named `test` with the stock Hive 2.1.0 distribution. (I simply downloaded it from its download page and initialized an empty Derby schema store.) The exact DDL I used to create this table is as follows:
```create table test(a string) partitioned by (`P``\u0414r t` int);```
When I do a `describe test` with `hive` it shows the column name as ``p`\u0434r t``. It appears to lowercase the P and the cyrillic \u0414 before storing the table schema it in the metastore. I then run
```alter table test add partition(`P``\u0414r t`=0);```
When I run `show partitions test` in `hive` it gives me ``p`\u0434r t=0``. Additionally, when I list the contents of the `test` table's base directory in HDFS, the partition directory entry is
```/user/hive/warehouse/test/p`\u0434r t=0```
If I drop the table, create it with `spark-sql` using the same DDL as before and do a `describe test`, the partition column is given as ``P`\u0414r t``. Spark has preserved the case of the partition column name. If I then do
```alter table test add partition(`P``\u0414r t`=0);```
in `spark-sql` and `show partitions test` I get ``P`\u0414r t=0``. When I list the directory contents in HDFS, I get
```/user/hive/warehouse/test/P`\u0414r t=0```
The upshot is Hive is lowercasing the partition column name and Spark is leaving it unaltered. Is this correct?
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/69652/
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
LMK if there's anything else you'd like me to address, otherwise\u2014assuming the tests pass\u2014please merge to master. Also, it would be great if we can back port this into 2.1 as well.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/69491/
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89679485
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
@@ -922,6 +923,29 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
/**
* Returns the partition names from hive metastore for a given table in a database.
*/
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = withClient {
+ val actualPartColNames = getTable(db, table).partitionColumnNames
+ val clientPartitionNames =
+ client.getPartitionNames(db, table, partialSpec.map(lowerCasePartitionSpec))
+
+ if (actualPartColNames.exists(partColName => partColName != partColName.toLowerCase)) {
+ clientPartitionNames.map { partName =>
+ val partSpec = PartitioningUtils.parsePathFragmentAsSeq(partName)
--- End diff --
I added coverage for partition column names with uppercase characters to `HiveCommandSuite`.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89656787
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
@@ -922,6 +923,29 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
/**
* Returns the partition names from hive metastore for a given table in a database.
*/
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = withClient {
+ val actualPartColNames = getTable(db, table).partitionColumnNames
+ val clientPartitionNames =
+ client.getPartitionNames(db, table, partialSpec.map(lowerCasePartitionSpec))
+
+ if (actualPartColNames.exists(partColName => partColName != partColName.toLowerCase)) {
+ clientPartitionNames.map { partName =>
+ val partSpec = PartitioningUtils.parsePathFragmentAsSeq(partName)
+ partSpec.map { case (partName, partValue) =>
+ actualPartColNames.find(_.equalsIgnoreCase(partName)).get + "=" + partValue
+ }.mkString("/")
+ }
+ } else {
+ clientPartitionNames
--- End diff --
Consider not having this optimization to avoid two different code paths here.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69683 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69683/consoleFull)** for PR 15998 at commit [`37fc595`](https://github.com/apache/spark/commit/37fc5955b2a26caa5ef133d19e415172a26330a2).
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r90092477
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
@@ -922,6 +923,29 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
/**
* Returns the partition names from hive metastore for a given table in a database.
*/
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = withClient {
+ val actualPartColNames = getTable(db, table).partitionColumnNames
+ val clientPartitionNames =
+ client.getPartitionNames(db, table, partialSpec.map(lowerCasePartitionSpec))
+
+ if (actualPartColNames.exists(partColName => partColName != partColName.toLowerCase)) {
+ clientPartitionNames.map { partName =>
+ val partSpec = PartitioningUtils.parsePathFragmentAsSeq(partName)
--- End diff --
That's correct. As of 2.1 we go through some extremes to preserve the case of partition names by saving extra metadata inside the table storage properties. When the table is resolved in `HiveExternalCatalog`, the lowercase schema is reconciled with the saved original names.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/15998
https://github.com/apache/spark/pull/15998#issuecomment-264761754 found a bug. If this PR will not be merged to Spark 2.1 branch, I think we need to submit a separate PR for resolving the bug.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
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/15998#discussion_r90582154
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogSuite.scala ---
@@ -346,6 +346,31 @@ abstract class ExternalCatalogSuite extends SparkFunSuite with BeforeAndAfterEac
assert(new Path(partitionLocation) == defaultPartitionLocation)
}
+ test("list partition names") {
+ val catalog = newBasicCatalog()
+ val newPart = CatalogTablePartition(Map("a" -> "1", "b" -> "%="), storageFormat)
+ catalog.createPartitions("db2", "tbl2", Seq(newPart), ignoreIfExists = false)
+
+ val partitionNames = catalog.listPartitionNames("db2", "tbl2")
+ assert(partitionNames == Seq("a=1/b=%25%3D", "a=1/b=2", "a=3/b=4"))
--- End diff --
is it the same behavior as before? i.e. `SHOW PARTITIONS` will output URI encoded partition value.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
> Like the other partition ExternalCatalog APIs, could you also add the negative test cases to ExternalCatalogSuite.scala?
I'm sorry, I don't understand what you're asking for. Can you be more specific?
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69652 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69652/consoleFull)** for PR 15998 at commit [`095cdd5`](https://github.com/apache/spark/commit/095cdd51c386e1747cc8645851b0cdffa83a5a70).
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/15998
can you also address this comment? https://github.com/apache/spark/pull/15998#discussion_r90181774
LGTM except a few comments.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
I suspect this is a spurious, unrelated test failure. Can we get a rebuild, please?
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69378 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69378/consoleFull)** for PR 15998 at commit [`48ae2a3`](https://github.com/apache/spark/commit/48ae2a39f8c5365c9f8e087333fc719b901517c2).
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69581 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69581/consoleFull)** for PR 15998 at commit [`afec06d`](https://github.com/apache/spark/commit/afec06d0f8a8bc592dffd8c4ae66743d42fde551).
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
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/15998#discussion_r90181898
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClient.scala ---
@@ -155,6 +155,25 @@ private[hive] trait HiveClient {
}
}
+ /**
+ * Returns the partition names for the given table that match the supplied partition spec.
+ * If no partition spec is specified, all partitions are returned.
+ */
+ final def getPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec]): Seq[String] = {
+ getPartitionNames(getTable(db, table), partialSpec)
+ }
+
+ /**
+ * Returns the partition names for the given table that match the supplied partition spec.
+ * If no partition spec is specified, all partitions are returned.
+ */
+ def getPartitionNames(
--- End diff --
why we create 2 APIs? who will call this method directly?
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/69089/
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
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/15998#discussion_r91001189
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogSuite.scala ---
@@ -346,6 +346,31 @@ abstract class ExternalCatalogSuite extends SparkFunSuite with BeforeAndAfterEac
assert(new Path(partitionLocation) == defaultPartitionLocation)
}
+ test("list partition names") {
+ val catalog = newBasicCatalog()
+ val newPart = CatalogTablePartition(Map("a" -> "1", "b" -> "%="), storageFormat)
+ catalog.createPartitions("db2", "tbl2", Seq(newPart), ignoreIfExists = false)
+
+ val partitionNames = catalog.listPartitionNames("db2", "tbl2")
+ assert(partitionNames == Seq("a=1/b=%25%3D", "a=1/b=2", "a=3/b=4"))
+ }
+
+ test("list partition names with partial partition spec") {
+ val catalog = newBasicCatalog()
+ val newPart = CatalogTablePartition(Map("a" -> "1", "b" -> "%="), storageFormat)
+ catalog.createPartitions("db2", "tbl2", Seq(newPart), ignoreIfExists = false)
+
+ val partitionNames1 = catalog.listPartitionNames("db2", "tbl2", Some(Map("a" -> "1")))
+ assert(partitionNames1 == Seq("a=1/b=%25%3D", "a=1/b=2"))
--- End diff --
cc @gatorsmile @ericl is it same with Hive? It looks weird to me that we return the "weird" value to users.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
> #15998 (comment) found a bug. If this PR will not be merged to Spark 2.1 branch, I think we need to submit a separate PR for resolving the bug.
I would like to get this patch into Spark 2.1 as it's a scalability issue for partitioned tables.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
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 pull request #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r90097773
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala ---
@@ -482,6 +482,19 @@ class InMemoryCatalog(
}
}
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = synchronized {
+ val partitionColumnNames = getTable(db, table).partitionColumnNames
+
+ listPartitions(db, table, partialSpec).map { partition =>
+ partitionColumnNames.map { name =>
+ name + "=" + partition.spec(name)
--- End diff --
Based on manual testing it seems that hive returns escaped values here. So for example, if you had a partition with column `B` and value `============`, then the hive client returns `b=%3D%3D%3D%3D%3D%3D%3D%3D%3D%3D%3D%3D`. So this code should probably call `getPathFragment` to have the same behavior.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
@gatorsmile I've applied your patch and reverted the change I made in the previous commit to workaround that defect. The failed test now passes for me. Let's see what Jenkins says.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69089 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69089/consoleFull)** for PR 15998 at commit [`f20ef8b`](https://github.com/apache/spark/commit/f20ef8bd0bbc68396546c60bb816fe9caf02042f).
* 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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r90788135
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -519,6 +519,26 @@ private[hive] class HiveClientImpl(
client.alterPartitions(table, newParts.map { p => toHivePartition(p, hiveTable) }.asJava)
}
+ /**
+ * Returns the partition names for the given table that match the supplied partition spec.
+ * If no partition spec is specified, all partitions are returned.
+ *
+ * The returned sequence is sorted as strings.
--- End diff --
The main use of this API is to answer `SHOW PARTITIONS` queries. Sorted results are simply easier to read than unsorted results.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r90092813
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
@@ -922,6 +923,29 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
/**
* Returns the partition names from hive metastore for a given table in a database.
*/
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = withClient {
+ val actualPartColNames = getTable(db, table).partitionColumnNames
+ val clientPartitionNames =
+ client.getPartitionNames(db, table, partialSpec.map(lowerCasePartitionSpec))
+
+ if (actualPartColNames.exists(partColName => partColName != partColName.toLowerCase)) {
+ clientPartitionNames.map { partName =>
+ val partSpec = PartitioningUtils.parsePathFragmentAsSeq(partName)
+ partSpec.map { case (partName, partValue) =>
+ actualPartColNames.find(_.equalsIgnoreCase(partName)).get + "=" + partValue
+ }.mkString("/")
+ }
+ } else {
+ clientPartitionNames
--- End diff --
Since this is not in the data path and the times are comparable, I think it would be better to drop the short circuit.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r90164499
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala ---
@@ -482,6 +483,19 @@ class InMemoryCatalog(
}
}
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = synchronized {
+ val partitionColumnNames = getTable(db, table).partitionColumnNames
+
+ listPartitions(db, table, partialSpec).map { partition =>
+ partitionColumnNames.map { name =>
+ escapePathName(name) + "=" + escapePathName(partition.spec(name))
--- End diff --
As mentioned above, `PartitioningUtils` is not available here so I went ahead with `escapePathName` itself.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
@gatorsmile I can't find your commit:
```
[msa@ip-10-0-8-34 spark-master]$ git fetch origin
remote: Counting objects: 114, done.
remote: Compressing objects: 100% (53/53), done.
remote: Total 114 (delta 41), reused 22 (delta 22), pack-reused 15
Receiving objects: 100% (114/114), 51.13 KiB | 0 bytes/s, done.
Resolving deltas: 100% (41/41), completed with 22 local objects.
From github.com:apache/spark
1821cbe..e23c8cf branch-2.1 -> origin/branch-2.1
eb8dd68..01a7d33 master -> origin/master
[msa@ip-10-0-8-34 spark-master]$ git show f4c48e1d61dc4d1fc581b78dd9e5c948965acad5
fatal: bad object f4c48e1d61dc4d1fc581b78dd9e5c948965acad5
```
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
Added a couple of unit tests and rebased.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89909840
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -730,6 +730,23 @@ class SessionCatalog(
}
/**
+ * List the names of all partitions that belong to the specified table, assuming it exists.
+ *
+ * A partial partition spec may optionally be provided to filter the partitions returned.
+ * For instance, if there exist partitions (a='1', b='2'), (a='1', b='3') and (a='2', b='4'),
+ * then a partial spec of (a='1') will return the first two only.
+ */
+ def listPartitionNames(
+ tableName: TableIdentifier,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = {
+ val db = formatDatabaseName(tableName.database.getOrElse(getCurrentDatabase))
+ val table = formatTableName(tableName.table)
+ requireDbExists(db)
+ requireTableExists(TableIdentifier(table, Option(db)))
--- End diff --
Got it. I will add that in my next push.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/69581/
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69174 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69174/consoleFull)** for PR 15998 at commit [`4e03c3e`](https://github.com/apache/spark/commit/4e03c3e46d22e5fe1b1fbc01ea57ef15d2723b9b).
* 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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/15998
where is the speed-up come from? Is it because the hive API `getPartitionNames` is faster than `getPartitions`? Or is it because we generate the partition string(`a=1/b=2/c=3`) at hive side and it's faster than Spark?
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/69174/
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
CC @ericl @cloud-fan
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/15998
I think this PR is for performance improvement, why we update the `HiveCommandSuite` here? Otherwise LGTM
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r90114390
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveCommandSuite.scala ---
@@ -408,14 +411,18 @@ class HiveCommandSuite extends QueryTest with SQLTestUtils with TestHiveSingleto
test("show partitions - datasource") {
withTable("part_datasrc") {
- val df = (1 to 3).map(i => (i, s"val_$i", i * 2)).toDF("a", "b", "c")
+ val df = (1 to 3).map(i => (i, s"val_$i", i * 2)).toDF("A", "b", "c")
df.write
- .partitionBy("a")
+ .partitionBy("A")
.format("parquet")
.mode(SaveMode.Overwrite)
.saveAsTable("part_datasrc")
- assert(sql("SHOW PARTITIONS part_datasrc").count() == 3)
+ checkAnswer(
+ sql("SHOW PARTITIONS part_datasrc"),
+ Row("A=1") ::
+ Row("A=2") ::
--- End diff --
nit: indentation seems a bit funny here, could inline the rows
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r90164431
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -730,6 +730,23 @@ class SessionCatalog(
}
/**
+ * List the names of all partitions that belong to the specified table, assuming it exists.
+ *
+ * A partial partition spec may optionally be provided to filter the partitions returned.
+ * For instance, if there exist partitions (a='1', b='2'), (a='1', b='3') and (a='2', b='4'),
+ * then a partial spec of (a='1') will return the first two only.
+ */
+ def listPartitionNames(
+ tableName: TableIdentifier,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = {
+ val db = formatDatabaseName(tableName.database.getOrElse(getCurrentDatabase))
+ val table = formatTableName(tableName.table)
+ requireDbExists(db)
+ requireTableExists(TableIdentifier(table, Option(db)))
--- End diff --
Done
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on the issue:
https://github.com/apache/spark/pull/15998
* looks good once InMemoryCatalog is fixed
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89656509
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalog.scala ---
@@ -189,6 +189,21 @@ abstract class ExternalCatalog {
spec: TablePartitionSpec): Option[CatalogTablePartition]
/**
+ * List the names of all partitions that belong to the specified table, assuming it exists.
+ *
+ * A partial partition spec may optionally be provided to filter the partitions returned.
+ * For instance, if there exist partitions (a='1', b='2'), (a='1', b='3') and (a='2', b='4'),
+ * then a partial spec of (a='1') will return the first two only.
--- End diff --
nit: newline here
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
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/15998#discussion_r90582264
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -519,6 +519,26 @@ private[hive] class HiveClientImpl(
client.alterPartitions(table, newParts.map { p => toHivePartition(p, hiveTable) }.asJava)
}
+ /**
+ * Returns the partition names for the given table that match the supplied partition spec.
+ * If no partition spec is specified, all partitions are returned.
+ *
+ * The returned sequence is sorted as strings.
--- End diff --
why do we need to sort?
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
@cloud-fan I'm not familiar enough with that code to be comfortable making that change. Can you submit a PR against `VideoAmp:spark-18572-list_partition_names` with the necessary changes to this file?
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/69683/
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r90681188
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -519,6 +519,26 @@ private[hive] class HiveClientImpl(
client.alterPartitions(table, newParts.map { p => toHivePartition(p, hiveTable) }.asJava)
}
+ /**
+ * Returns the partition names for the given table that match the supplied partition spec.
+ * If no partition spec is specified, all partitions are returned.
+ *
+ * The returned sequence is sorted as strings.
--- End diff --
According to the Hive wiki, these values are supposed to be sorted. However, a test build failed because the output of this method without explicit sorting was not sorted. I don't know why. I'm including ".sorted" to explicitly ensure sorting.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:
https://github.com/apache/spark/pull/15998
@cloud-fan That's unfortunate if it's going to block this PR. How do we proceed?
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15998
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/69195/
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89906448
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
@@ -730,6 +730,23 @@ class SessionCatalog(
}
/**
+ * List the names of all partitions that belong to the specified table, assuming it exists.
+ *
+ * A partial partition spec may optionally be provided to filter the partitions returned.
+ * For instance, if there exist partitions (a='1', b='2'), (a='1', b='3') and (a='2', b='4'),
+ * then a partial spec of (a='1') will return the first two only.
+ */
+ def listPartitionNames(
+ tableName: TableIdentifier,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = {
+ val db = formatDatabaseName(tableName.database.getOrElse(getCurrentDatabase))
+ val table = formatTableName(tableName.table)
+ requireDbExists(db)
+ requireTableExists(TableIdentifier(table, Option(db)))
--- End diff --
Without `requirePartialMatchedPartitionSpec`, do we detect these negative cases?
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89678233
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala ---
@@ -482,6 +482,19 @@ class InMemoryCatalog(
}
}
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = synchronized {
+ val partitionColumnNames = getTable(db, table).partitionColumnNames
+
+ listPartitions(db, table, partialSpec).map { partition =>
+ partitionColumnNames.map { name =>
+ name + "=" + partition.spec(name)
--- End diff --
I think there should be column name and value escaping as these are column names and values. What's confusing to me is that the previous code uses a method called `escapePathName` which suggests something related to a filesystem path. Indeed the source code includes a comment stating that the aforementioned method comes from a Hive class called `FileUtils`.
I'll look into how Hive escapes partition names and values in its `getPartitionNames` method and report back.
---
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 #15998: [SPARK-18572][SQL] Add a method `listPartitionNames` to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15998
**[Test build #69195 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/69195/consoleFull)** for PR 15998 at commit [`594c693`](https://github.com/apache/spark/commit/594c6934c4fe4e41a895ea91b3b2cc45f38eaeb6).
* 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 pull request #15998: [SPARK-18572][SQL] Add a method `listPartitionNam...
Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:
https://github.com/apache/spark/pull/15998#discussion_r89678259
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
@@ -922,6 +923,29 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
/**
* Returns the partition names from hive metastore for a given table in a database.
*/
+ override def listPartitionNames(
+ db: String,
+ table: String,
+ partialSpec: Option[TablePartitionSpec] = None): Seq[String] = withClient {
+ val actualPartColNames = getTable(db, table).partitionColumnNames
+ val clientPartitionNames =
+ client.getPartitionNames(db, table, partialSpec.map(lowerCasePartitionSpec))
+
+ if (actualPartColNames.exists(partColName => partColName != partColName.toLowerCase)) {
+ clientPartitionNames.map { partName =>
+ val partSpec = PartitioningUtils.parsePathFragmentAsSeq(partName)
+ partSpec.map { case (partName, partValue) =>
+ actualPartColNames.find(_.equalsIgnoreCase(partName)).get + "=" + partValue
+ }.mkString("/")
+ }
+ } else {
+ clientPartitionNames
--- End diff --
I'll try a build with and without this code branch and see if there's a noticeable performance difference.
---
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