Skip to content

[SPARK-15901] [SQL] [TEST] Verification of CONVERT_METASTORE_ORC and CONVERT_METASTORE_PARQUET#13622

Closed
gatorsmile wants to merge 2 commits intoapache:masterfrom
gatorsmile:addTestCase4parquetOrcConversion
Closed

[SPARK-15901] [SQL] [TEST] Verification of CONVERT_METASTORE_ORC and CONVERT_METASTORE_PARQUET#13622
gatorsmile wants to merge 2 commits intoapache:masterfrom
gatorsmile:addTestCase4parquetOrcConversion

Conversation

@gatorsmile
Copy link
Member

What changes were proposed in this pull request?

So far, we do not have test cases for verifying whether the external parameters HiveUtils .CONVERT_METASTORE_ORC and HiveUtils.CONVERT_METASTORE_PARQUET properly works when users use non-default values. This PR is to add such test cases for avoiding potential regression.

How was this patch tested?

N/A

@SparkQA
Copy link

SparkQA commented Jun 12, 2016

Test build #60358 has finished for PR 13622 at commit 7a2b9b6.

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

@SparkQA
Copy link

SparkQA commented Jun 12, 2016

Test build #60369 has finished for PR 13622 at commit b81fde9.

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

}
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

It is a bit costy to write a end to end test like this, how about adding an Unit test using SimpleAnalyzer?

Copy link
Member Author

@gatorsmile gatorsmile Jun 14, 2016

Choose a reason for hiding this comment

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

Thank you for your review! @clockfly

I like your suggestion, but the conf CONVERT_METASTORE_ORC and CONVERT_METASTORE_PARQUET are not available in CatalystConf and SimpleCatalystConf. I am hesitant to change the source codes for improving test case coverage.

Actually, I am not sure if we should keep these rules in the Analyzer in the future release. These two rules are not used for resolution. Instead, they are for performance enhancement. Ideally, I think they are Optimizer rules.

cc @cloud-fan @liancheng

Copy link
Contributor

Choose a reason for hiding this comment

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

Having Parquet/ORC conversions in the optimizer is also reasonable, but doesn't seem to bring much benefits. Furthermore, it also requires us to refactor the optimizer to support Hive specific rules.

@liancheng
Copy link
Contributor

LGTM, merging to master and branch-2.0. Thanks!

asfgit pushed a commit that referenced this pull request Jun 15, 2016
…NVERT_METASTORE_PARQUET

#### What changes were proposed in this pull request?
So far, we do not have test cases for verifying whether the external parameters `HiveUtils .CONVERT_METASTORE_ORC` and `HiveUtils.CONVERT_METASTORE_PARQUET` properly works when users use non-default values. This PR is to add such test cases for avoiding potential regression.

#### How was this patch tested?
N/A

Author: gatorsmile <gatorsmile@gmail.com>

Closes #13622 from gatorsmile/addTestCase4parquetOrcConversion.

(cherry picked from commit 0992573)
Signed-off-by: Cheng Lian <lian@databricks.com>
@asfgit asfgit closed this in 0992573 Jun 15, 2016
nezihyigitbasi pushed a commit to nezihyigitbasi/spark that referenced this pull request Jun 16, 2016
…NVERT_METASTORE_PARQUET

So far, we do not have test cases for verifying whether the external parameters `HiveUtils .CONVERT_METASTORE_ORC` and `HiveUtils.CONVERT_METASTORE_PARQUET` properly works when users use non-default values. This PR is to add such test cases for avoiding potential regression.

N/A

Author: gatorsmile <gatorsmile@gmail.com>

Closes apache#13622 from gatorsmile/addTestCase4parquetOrcConversion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants