You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/07/15 00:26:13 UTC
[GitHub] [spark] sunchao opened a new pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
sunchao opened a new pull request #33350:
URL: https://github.com/apache/spark/pull/33350
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'core/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
Move both `PruneFileSourcePartitionsSuite` and `PrunePartitionSuiteBase` to the package `org.apache.spark.sql.execution.datasources`. Did a few refactoring to enable this.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
Currently both `PruneFileSourcePartitionsSuite` and `PrunePartitionSuiteBase` are in package `org.apache.spark.sql.hive.execution` which doesn't look correct as these tests are not specific to Hive. Therefore, it's better to move them into `org.apache.spark.sql.execution.datasources`, the same place where the rule `PruneFileSourcePartitions` is at.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No, it's just test refactoring.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Using existing tests:
```
build/sbt "sql/testOnly *PruneFileSourcePartitionsSuite"
```
and
```
build/sbt "hive/testOnly *PruneHiveTablePartitionsSuite"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r671725543
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
Review comment:
If you are not sure, why don't we keep the AS-IS one, @sunchao ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882820913
**[Test build #141276 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141276/testReport)** for PR 33350 at commit [`1316644`](https://github.com/apache/spark/commit/1316644a2a95587a65e62eabd745372aac5994df).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882778485
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882778485
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887200130
seems they are failing in 3.2 branch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887663399
Thanks @LuciferYang ! yes I found that we could either use `local[1]` or `coalesce(1)` to fix the first test case. For the second, it relies on `HiveCatalogMetrics.METRIC_PARTITIONS_FETCHED` which is no longer available since it switched to use `InMemoryCatalog`. I need to find a new way to write the test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882782991
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141273/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r671463471
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
Review comment:
Thanks @dongjoon-hyun . I found it now. However I'm not sure whether this matters for the test though: what it does is just 1) register table metadata in the catalog, 2) create a `LogicalRelation` wrapping a `HadoopFsRelation` which has the data and partition schema from the step 1), and 3) feed it into the rule `PruneFileSourcePartitions` and see if the `LogicalRelation`'s `expectedOutputAttributes` is properly set. Seems this is irrelevant to what SerDe it is using?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r670933413
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
Review comment:
As you know, `saveAsTable` is different from `STORED AS` parquet. The original test coverage seems to be coupled with `convertMetastoreParquet`, but this one looks different. Are we losing the existing test coverage?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882782949
**[Test build #141273 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141273/testReport)** for PR 33350 at commit [`33c38a4`](https://github.com/apache/spark/commit/33c38a449735cbfd78405476f381eeac0f2627b9).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase with SharedSparkSession `
* `abstract class PrunePartitionSuiteBase extends StatisticsCollectionTestBase `
* `class PruneHiveTablePartitionsSuite extends PrunePartitionSuiteBase with TestHiveSingleton `
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r673330623
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -45,35 +46,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
Review comment:
Yes, since we're moving `PruneFileSourcePartitionsSuite` out of the `hive` package, we need to remove the Hive dependency here too.
As commented in the [other thread](https://github.com/apache/spark/pull/33350#discussion_r671463471), to me it's OK to switch to use data source table here. I also digged the history of the change, and it seems at the time when this test was added (in #15569), data source table doesn't use HMS to store table metadata by default (it was added #15515 later), but instead was using `ListingFileCatalog` (?). Maybe it was for testing purpose that we created a Hive table here but then constructed a `LogicalRelation` to feed into the `PruneFileSourcePartitions` rule?
Let me know if you see concern here @cloud-fan , since you are the main author of this test and the related code :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882782991
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141273/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-886963363
**[Test build #141651 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141651/testReport)** for PR 33350 at commit [`d6fa7a5`](https://github.com/apache/spark/commit/d6fa7a504944212337150147be86540730ff4346).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r671024091
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
Review comment:
FYI, `CREATE TABLE ... USING PARQUET` (spark syntax) and `CREATE TABLE ... STORED AS PARQUET` (hive syntax) generates different tables in Apache Spark.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on pull request #33350: [SPARK-36136][SQL][TEST] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-881093037
Thanks @dongjoon-hyun . I've changed the title to "Refactor PruneFileSourcePartitionsSuite etc to a different package" - let me know if this works for you :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-880338848
**[Test build #141039 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141039/testReport)** for PR 33350 at commit [`f782ce7`](https://github.com/apache/spark/commit/f782ce72e149e362d87dba46d7740d0cd3dbf036).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase with SharedSparkSession `
* `abstract class PrunePartitionSuiteBase extends StatisticsCollectionTestBase `
* `class PruneHiveTablePartitionsSuite extends PrunePartitionSuiteBase with TestHiveSingleton `
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887211632
We can revert it if it needs taking some time to investigate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-880298986
**[Test build #141039 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141039/testReport)** for PR 33350 at commit [`f782ce7`](https://github.com/apache/spark/commit/f782ce72e149e362d87dba46d7740d0cd3dbf036).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r670799877
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
+ val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
+ val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
+
+ val dataSchema = StructType(tableMeta.schema.filterNot { f =>
+ tableMeta.partitionColumnNames.contains(f.name)
+ })
+ val relation = HadoopFsRelation(
+ location = catalogFileIndex,
+ partitionSchema = tableMeta.partitionSchema,
+ dataSchema = dataSchema,
+ bucketSpec = None,
+ fileFormat = new ParquetFileFormat(),
+ options = Map.empty)(sparkSession = spark)
+
+ val logicalRelation = LogicalRelation(relation, tableMeta)
+ val query = Project(Seq(Symbol("id"), Symbol("p")),
Review comment:
Anyway, if the title and scope becomes broaden, I'm okay for `id`, too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-886961591
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46167/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-880315195
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45554/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r672046543
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
Do we know why it uses external table before? Is it related to the test coverage here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r671018264
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
Review comment:
Hmm what is `convertMetastoreParquet`? I couldn't find it anywhere.
Regarding the test, I think it is still covered (I've debugged the test and made sure it is still going through the related code paths in `PruneFileSourcePartitions`). Much has changed since 2016 though: the test (added in #15569) was originally designed to make sure that `LogicalRelation.expectedOutputAttributes` was correctly populated in the class. The `expectedOutputAttributes`, however, was later replaced by directly passing `output` in `LogicalRelation` (in #17552), which I think further prevented the issue from happening.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-886884805
**[Test build #141651 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141651/testReport)** for PR 33350 at commit [`d6fa7a5`](https://github.com/apache/spark/commit/d6fa7a504944212337150147be86540730ff4346).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-880341814
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-880452384
BTW, I agree with the idea.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882782991
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-883031312
**[Test build #141293 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141293/testReport)** for PR 33350 at commit [`7473aea`](https://github.com/apache/spark/commit/7473aea9aa91586366206b7a01ed3b6e11f7236a).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r671023406
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
Review comment:
I mean `spark.sql.hive.convertMetastoreParquet`. Here is the document.
- https://spark.apache.org/docs/latest/sql-data-sources-parquet.html#hive-metastore-parquet-table-conversion
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-886988096
Thanks. Merging to master/3.2. Although this is not bug fix, but it is only for test and I think it is better to keep consistency between master/3.2 so it is easier to backport changes.
Feel free to revert it from 3.2 if you prefer to have it only in master. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r671725543
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
Review comment:
If you are not sure, why don't we keep the AS-IS existing one, @sunchao ? Refactoring is good but always needs verification.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-883071887
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45807/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882921683
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45796/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887204038
Sorry. Let me check the failed tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882886574
**[Test build #141282 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141282/testReport)** for PR 33350 at commit [`c98975c`](https://github.com/apache/spark/commit/c98975c1a3424caa0a5b254a97ba4b0e8b09ce73).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882871500
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45790/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882825098
**[Test build #141276 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141276/testReport)** for PR 33350 at commit [`1316644`](https://github.com/apache/spark/commit/1316644a2a95587a65e62eabd745372aac5994df).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r672877467
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -45,35 +46,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
Review comment:
so it's not only moving the package, but also changes some tests to not use hive tables but use data source tables instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882886574
**[Test build #141282 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141282/testReport)** for PR 33350 at commit [`c98975c`](https://github.com/apache/spark/commit/c98975c1a3424caa0a5b254a97ba4b0e8b09ce73).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882778485
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882778485
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882699915
> Do we still have the test coverage for partition pruning with hive tables?
@cloud-fan you mean `PruneHiveTablePartitionsSuite`? yes it is untouched by this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r672877467
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -45,35 +46,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
Review comment:
so it's not only moving the package, but also changes some tests to not use hive tables but use data source tables instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-886884805
**[Test build #141651 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141651/testReport)** for PR 33350 at commit [`d6fa7a5`](https://github.com/apache/spark/commit/d6fa7a504944212337150147be86540730ff4346).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882884998
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45790/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882787888
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45787/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r671024763
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
Review comment:
For Hive tables generated by `STORED AS` syntax, Spark converts them to data source tables on the fly because `spark.sql.hive.convertMetastoreParquet` is `true` by default. It's the same for ORC. For ORC, we have `spark.sql.hive.convertMetastoreOrc`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r672555784
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
I'm not sure too, IMO the `EXTERNAL` keyword doesn't matter here. I've run the test with and without it and the outcome is the same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r672877467
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -45,35 +46,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
Review comment:
so it's not only moving the package, but also changes some tests to not use hive tables but use data source tables instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887221527
Created #33533 to revert it first. @cloud-fan @sunchao @dongjoon-hyun
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] venkata91 commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
venkata91 commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887171471
@viirya @sunchao It seems like the refactor caused couple of tests in `PruneFileSourcePartitionsSuite` tests to fail. Seems like the refactor from Hive tables to datasource is causing issues.
I think this test `SPARK-35985 push filters for empty read schema - returns all the files under the partition with DSV2 therefore failing `
`SPARK-36128: spark.sql.hive.metastorePartitionPruning should work for file data sources` - this test checks for `HiveCatalogMetrics.METRIC_PARTITIONS_FETCHED.getCount` since the tests are now moved, this is becoming 0. Please take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r672542298
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
Yea, I saw the comment. It looks making sense and that's also what I read from the test. Just wondering why it uses external table originally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882271140
Could you resolve the conflicts, @sunchao ?
Also, cc @cloud-fan , @maropu , @viirya .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887291000
It seems that `SPARK-36128: spark.sql.hive.metastorePartitionPruning should work for file data sources` should not be placed in sql/core module.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882884998
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45790/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-886964287
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141651/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang edited a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
LuciferYang edited a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887269393
@sunchao
I found that the old case use `spark.master local[1]`, but `SharedSparkSession` create `TestSparkSession` with `local[2]` as default , so we should override `createSparkSession` method in new `PruneFileSourcePartitionsSuite` file to create `TestSparkSession` with `local[1]` to pass `SPARK-35985 push filters for empty read schema`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-880327754
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45554/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887005723
Thanks @viirya ! Yes I agree we should backport and keep master & 3.2 consistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-883087045
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45807/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-880887504
Thanks @dongjoon-hyun for reviewing. On the title, do you have any suggestion? this PR does move these files from one package to another package so I'm thinking at least it expressed that part OK. How about something like "Refactor PruneFileSourcePartitionsSuite and move it to the correct package"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882787907
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45787/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-886877665
Thanks @cloud-fan ! just rebased.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887242347
We can revert it first. The test failures are related. Not sure why they weren't detected by the CI previously though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r672555784
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
I'm not sure too, IMO the `EXTERNAL` keyword doesn't matter here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-883050392
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45807/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887261436
For `SPARK-35985 push filters for empty read schema`
The old case writes one result file for each partition, but the new case writes two result files for each partition, I guess some test configurations have changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r671024763
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
Review comment:
For Hive tables generated by `STORED AS` syntax, Spark converts it to data source tables on the fly because `spark.sql.hive.convertMetastoreParquet` is `true` by default. It's the same for ORC. For ORC, we have `spark.sql.hive.convertMetastoreOrc`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-883031312
**[Test build #141293 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141293/testReport)** for PR 33350 at commit [`7473aea`](https://github.com/apache/spark/commit/7473aea9aa91586366206b7a01ed3b6e11f7236a).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887203356
hmm, they are also failing in master: https://github.com/apache/spark/pull/33498
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882890255
**[Test build #141282 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141282/testReport)** for PR 33350 at commit [`c98975c`](https://github.com/apache/spark/commit/c98975c1a3424caa0a5b254a97ba4b0e8b09ce73).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882938480
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45796/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-886929186
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46167/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r672542298
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
Yea, I saw the comment. It looks making sense and that's also what I read from the test. Just wondering why it uses external table originally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-883106788
**[Test build #141293 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141293/testReport)** for PR 33350 at commit [`7473aea`](https://github.com/apache/spark/commit/7473aea9aa91586366206b7a01ed3b6e11f7236a).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-883137739
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141293/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #33350: [SPARK-36136][SQL][TEST] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r670886900
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
+ val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
+ val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
+
+ val dataSchema = StructType(tableMeta.schema.filterNot { f =>
+ tableMeta.partitionColumnNames.contains(f.name)
+ })
+ val relation = HadoopFsRelation(
+ location = catalogFileIndex,
+ partitionSchema = tableMeta.partitionSchema,
+ dataSchema = dataSchema,
+ bucketSpec = None,
+ fileFormat = new ParquetFileFormat(),
+ options = Map.empty)(sparkSession = spark)
+
+ val logicalRelation = LogicalRelation(relation, tableMeta)
+ val query = Project(Seq(Symbol("id"), Symbol("p")),
Review comment:
Thanks! I'll keep as it is then :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882890293
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141282/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r670190452
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
+ val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
+ val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
+
+ val dataSchema = StructType(tableMeta.schema.filterNot { f =>
+ tableMeta.partitionColumnNames.contains(f.name)
+ })
+ val relation = HadoopFsRelation(
+ location = catalogFileIndex,
+ partitionSchema = tableMeta.partitionSchema,
+ dataSchema = dataSchema,
+ bucketSpec = None,
+ fileFormat = new ParquetFileFormat(),
+ options = Map.empty)(sparkSession = spark)
+
+ val logicalRelation = LogicalRelation(relation, tableMeta)
+ val query = Project(Seq(Symbol("id"), Symbol("p")),
Review comment:
Since this claims a simple moving classes, shall we preserve `i` instead of introducing new column name, `id`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r670676507
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
+ val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
+ val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
+
+ val dataSchema = StructType(tableMeta.schema.filterNot { f =>
+ tableMeta.partitionColumnNames.contains(f.name)
+ })
+ val relation = HadoopFsRelation(
+ location = catalogFileIndex,
+ partitionSchema = tableMeta.partitionSchema,
+ dataSchema = dataSchema,
+ bucketSpec = None,
+ fileFormat = new ParquetFileFormat(),
+ options = Map.empty)(sparkSession = spark)
+
+ val logicalRelation = LogicalRelation(relation, tableMeta)
+ val query = Project(Seq(Symbol("id"), Symbol("p")),
Review comment:
Thanks. I'm not sure whether it's worth doing so because we changed how the test table is created by using the DataFrame API `spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")`, which creates `id` column by default. The `id` here is also consistent with the rest of the tests in this file as well as other tests which use the same API to create tables.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya closed pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
viirya closed pull request #33350:
URL: https://github.com/apache/spark/pull/33350
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882907556
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45796/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887212407
Is it flaky or do other merged PRs cause the result different?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887243353
@sunchao I also meet this problem
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-886961591
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46167/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887178322
Hm? This passed Jenkins and GA was also passed. Where do you see the failed tests?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r672465107
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
I explained this in [the other thread](https://github.com/apache/spark/pull/33350/files/f782ce72e149e362d87dba46d7740d0cd3dbf036#r671463471) and I don't think this is related to the test coverage here. Let me know if you think otherwise @viirya @cloud-fan .
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
> Do we still have the test coverage for partition pruning with hive tables?
@cloud-fan you mean `PruneHiveTablePartitionsSuite`? yes it is untouched by this PR.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
I'm not sure too, IMO the `EXTERNAL` keyword doesn't matter here.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
I'm not sure too, IMO the `EXTERNAL` keyword doesn't matter here. I've run the test with and without it and the outcome is the same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r670799061
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
+ val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
+ val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
+
+ val dataSchema = StructType(tableMeta.schema.filterNot { f =>
+ tableMeta.partitionColumnNames.contains(f.name)
+ })
+ val relation = HadoopFsRelation(
+ location = catalogFileIndex,
+ partitionSchema = tableMeta.partitionSchema,
+ dataSchema = dataSchema,
+ bucketSpec = None,
+ fileFormat = new ParquetFileFormat(),
+ options = Map.empty)(sparkSession = spark)
+
+ val logicalRelation = LogicalRelation(relation, tableMeta)
+ val query = Project(Seq(Symbol("id"), Symbol("p")),
Review comment:
I meant we should recover it from `id` to `i` together~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r672465793
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
> Do we still have the test coverage for partition pruning with hive tables?
@cloud-fan you mean `PruneHiveTablePartitionsSuite`? yes it is untouched by this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882778485
**[Test build #141273 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141273/testReport)** for PR 33350 at commit [`33c38a4`](https://github.com/apache/spark/commit/33c38a449735cbfd78405476f381eeac0f2627b9).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-880341816
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r670933413
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
Review comment:
As you know, `saveAsTable` is different from `STORED AS` parquet. The original test coverage seems to be coupled with `convertMetastoreParquet`, but this one looks different. Are we losing the existing test coverage?
```
scala> spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("t1")
scala> sql("DESCRIBE TABLE EXTENDED t1").show()
...
| Provider| parquet| |
...
scala> sql("CREATE TABLE t2(a int) STORED AS parquet").show()
scala> sql("DESCRIBE TABLE EXTENDED t2").show()
...
| Provider| hive| |
...
```
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
Review comment:
As you know, `saveAsTable` is different from `STORED AS` parquet. The original test coverage seems to be coupled with `convertMetastoreParquet`, but this one looks different. Are we losing the existing test coverage?
```scala
scala> spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("t1")
scala> sql("DESCRIBE TABLE EXTENDED t1").show()
...
| Provider| parquet| |
...
scala> sql("CREATE TABLE t2(a int) STORED AS parquet").show()
scala> sql("DESCRIBE TABLE EXTENDED t2").show()
...
| Provider| hive| |
...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r670935067
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
-
- val tableMeta = spark.sharedState.externalCatalog.getTable("default", "test")
- val catalogFileIndex = new CatalogFileIndex(spark, tableMeta, 0)
-
- val dataSchema = StructType(tableMeta.schema.filterNot { f =>
- tableMeta.partitionColumnNames.contains(f.name)
- })
- val relation = HadoopFsRelation(
- location = catalogFileIndex,
- partitionSchema = tableMeta.partitionSchema,
- dataSchema = dataSchema,
- bucketSpec = None,
- fileFormat = new ParquetFileFormat(),
- options = Map.empty)(sparkSession = spark)
-
- val logicalRelation = LogicalRelation(relation, tableMeta)
- val query = Project(Seq(Symbol("i"), Symbol("p")),
- Filter(Symbol("p") === 1, logicalRelation)).analyze
-
- val optimized = Optimize.execute(query)
- assert(optimized.missingInput.isEmpty)
- }
+ spark.range(10).selectExpr("id", "id % 3 as p").write.partitionBy("p").saveAsTable("test")
Review comment:
This specific test coverage should remain at `hive` module.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882782991
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-880298986
**[Test build #141039 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141039/testReport)** for PR 33350 at commit [`f782ce7`](https://github.com/apache/spark/commit/f782ce72e149e362d87dba46d7740d0cd3dbf036).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r672542298
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
Yea, I saw the comment. It looks making sense and that's also what I read from the test. Just wondering why it uses external table originally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882825138
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141276/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-886964287
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141651/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882825138
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141276/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882787907
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45787/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882890293
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141282/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #33350: [SPARK-36136][SQL][TEST] Move PruneFileSourcePartitionsSuite to org.apache.spark.sql.execution.datasources
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-881007707
Ya, `Refactoring ...` sounds more accurate. However, `the correct package` should not be there. The previous one also can be considered correct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-886951698
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46167/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r672465107
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
I explained this in [the other thread](https://github.com/apache/spark/pull/33350/files/f782ce72e149e362d87dba46d7740d0cd3dbf036#r671463471) and I don't think this is related to the test coverage here. Let me know if you think otherwise @viirya @cloud-fan .
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
> Do we still have the test coverage for partition pruning with hive tables?
@cloud-fan you mean `PruneHiveTablePartitionsSuite`? yes it is untouched by this PR.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
I'm not sure too, IMO the `EXTERNAL` keyword doesn't matter here.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
I'm not sure too, IMO the `EXTERNAL` keyword doesn't matter here. I've run the test with and without it and the outcome is the same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882820913
**[Test build #141276 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141276/testReport)** for PR 33350 at commit [`1316644`](https://github.com/apache/spark/commit/1316644a2a95587a65e62eabd745372aac5994df).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-887269393
@sunchao
I found that the old case use `spark.master local[1]`, but `SharedSparkSession` create `TestSparkSession` with `local[2]` as default , so we should override `createSparkSession` method to create `TestSparkSession` with `local[1]` to pass `SPARK-35985 push filters for empty read schema`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882782991
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882850515
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45790/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #33350:
URL: https://github.com/apache/spark/pull/33350#discussion_r672465107
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/PruneFileSourcePartitionsSuite.scala
##########
@@ -42,35 +43,27 @@ class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase {
test("PruneFileSourcePartitions should not change the output of LogicalRelation") {
withTable("test") {
- withTempDir { dir =>
- sql(
- s"""
- |CREATE EXTERNAL TABLE test(i int)
- |PARTITIONED BY (p int)
- |STORED AS parquet
- |LOCATION '${dir.toURI}'""".stripMargin)
Review comment:
I explained this in [the other thread](https://github.com/apache/spark/pull/33350/files/f782ce72e149e362d87dba46d7740d0cd3dbf036#r671463471) and I don't think this is related to the test coverage here. Let me know if you think otherwise @viirya @cloud-fan .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-883137739
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141293/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882782991
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882778485
**[Test build #141273 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141273/testReport)** for PR 33350 at commit [`33c38a4`](https://github.com/apache/spark/commit/33c38a449735cbfd78405476f381eeac0f2627b9).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882699915
> Do we still have the test coverage for partition pruning with hive tables?
@cloud-fan you mean `PruneHiveTablePartitionsSuite`? yes it is untouched by this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882308393
Do we still have the test coverage for partition pruning with hive tables?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-883087045
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45807/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882938480
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45796/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on pull request #33350: [SPARK-36136][SQL][TESTS] Refactor PruneFileSourcePartitionsSuite etc to a different package
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #33350:
URL: https://github.com/apache/spark/pull/33350#issuecomment-882699915
> Do we still have the test coverage for partition pruning with hive tables?
@cloud-fan you mean `PruneHiveTablePartitionsSuite`? yes it is untouched by this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org