You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/10/31 22:34:11 UTC
[GitHub] [iceberg] kbendick opened a new pull request #3428: 0.12.1 Cherry-picks: Part 2
kbendick opened a new pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428
This includes more cherry-picks for the 0.12.1 release.
It is a continuation of https://github.com/apache/iceberg/pull/3402, for cherry-picks that are not 100% clean when pulling.
I will mark the things I needed to change.
As mentioned in the last PR, the following two (earlier) PRs did not cherry-pick cleanly:
- https://github.com/apache/iceberg/pull/3195
- https://github.com/apache/iceberg/pull/3199
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #3428: 0.12.1 Cherry-Picks: Part 3
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428#issuecomment-956536502
Looks great! Thanks, @kbendick. I merged this.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick edited a comment on pull request #3428: 0.12.1 Cherry-Picks: Part 3
Posted by GitBox <gi...@apache.org>.
kbendick edited a comment on pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428#issuecomment-956467327
This is ready for review @rdblue, but should come after https://github.com/apache/iceberg/pull/3429:
We don't necessarily need #3264 (Hive: Ensure tableLevelMutex is unlocked when uncommitted metadata de...), but I grabbed it as the test suite was taking quite some time to run. It also seems like a correctness fix so argubaly we should grab it.
It's also in both this PR and the `Part 2` PR #3429, but wanted to leave the green tests before I rebased and removed it in case there's other changes needed. If that causes issue for rebase and merge, I can remove it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #3428: 0.12.1 Cherry-Picks: Part 3
Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #3428: 0.12.1 Cherry-picks: Part 2
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428#discussion_r739887702
##########
File path: mr/src/main/java/org/apache/iceberg/mr/Catalogs.java
##########
@@ -202,7 +202,15 @@ public static boolean dropTable(Configuration conf, Properties props) {
*/
public static boolean hiveCatalog(Configuration conf, Properties props) {
String catalogName = props.getProperty(InputFormatConfig.CATALOG_NAME);
- return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(getCatalogType(conf, catalogName));
+ String catalogType = getCatalogType(conf, catalogName);
+ if (catalogType != null) {
+ return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(catalogType);
+ }
+ catalogType = getCatalogType(conf, ICEBERG_DEFAULT_CATALOG_NAME);
+ if (catalogType != null) {
+ return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(catalogType);
+ }
+ return getCatalogProperties(conf, catalogName, catalogType).get(CatalogProperties.CATALOG_IMPL) == null;
Review comment:
This whole block didn't cherry-pick cleanly, but the new diff was exactly the same as from the originating PR: https://github.com/apache/iceberg/pull/3338
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #3428: 0.12.1 Cherry-picks: Part 2
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428#discussion_r739885921
##########
File path: build.gradle
##########
@@ -1104,6 +1104,8 @@ project(":iceberg-spark3-extensions") {
testCompile project(path: ':iceberg-spark', configuration: 'testArtifacts')
testCompile project(path: ':iceberg-spark3', configuration: 'testArtifacts')
+ testCompile "org.apache.avro:avro"
Review comment:
In the original PR https://github.com/apache/iceberg/pull/3273, this was in `spark/v3.0/build.gradle`. But we're not including the structural refactor in the point release.
Additionally, this was `testImplementation`. Given that the above dependencies were also changed to `testImplementation`, I chose to be consistent and use `testCompile`.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #3428: 0.12.1 Cherry-picks: Part 2
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428#discussion_r739887702
##########
File path: mr/src/main/java/org/apache/iceberg/mr/Catalogs.java
##########
@@ -202,7 +202,15 @@ public static boolean dropTable(Configuration conf, Properties props) {
*/
public static boolean hiveCatalog(Configuration conf, Properties props) {
String catalogName = props.getProperty(InputFormatConfig.CATALOG_NAME);
- return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(getCatalogType(conf, catalogName));
+ String catalogType = getCatalogType(conf, catalogName);
+ if (catalogType != null) {
+ return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(catalogType);
+ }
+ catalogType = getCatalogType(conf, ICEBERG_DEFAULT_CATALOG_NAME);
+ if (catalogType != null) {
+ return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(catalogType);
+ }
+ return getCatalogProperties(conf, catalogName, catalogType).get(CatalogProperties.CATALOG_IMPL) == null;
Review comment:
This whole block didn't cherry-pick cleanly, because we didn't grab this PR: [Core: Throw an exception if both catalog type and catalog-impl are set #3162](https://github.com/apache/iceberg/pull/3162)
This is the PR from which these changes came from: [Hive: Fix Catalogs.hiveCatalog method for default catalogs #3338](https://github.com/apache/iceberg/pull/3338).
I don't think we should grab #3162 as it could break existing user's presently valid configs, but very much open to discussion on that.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #3428: 0.12.1 Cherry-picks: Part 2
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428#discussion_r739887702
##########
File path: mr/src/main/java/org/apache/iceberg/mr/Catalogs.java
##########
@@ -202,7 +202,15 @@ public static boolean dropTable(Configuration conf, Properties props) {
*/
public static boolean hiveCatalog(Configuration conf, Properties props) {
String catalogName = props.getProperty(InputFormatConfig.CATALOG_NAME);
- return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(getCatalogType(conf, catalogName));
+ String catalogType = getCatalogType(conf, catalogName);
+ if (catalogType != null) {
+ return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(catalogType);
+ }
+ catalogType = getCatalogType(conf, ICEBERG_DEFAULT_CATALOG_NAME);
+ if (catalogType != null) {
+ return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(catalogType);
+ }
+ return getCatalogProperties(conf, catalogName, catalogType).get(CatalogProperties.CATALOG_IMPL) == null;
Review comment:
This whole block didn't cherry-pick cleanly, because we didn't grab this PR: [Core: Throw an exception if both catalog type and catalog-impl are set #3162](https://github.com/apache/iceberg/pull/3162)
This is the PR from which these changes came from: [Hive: Fix Catalogs.hiveCatalog method for default catalogs #3338](https://github.com/apache/iceberg/pull/3338).
I don't think we should grab #3162 as it could break existing user's presently valid configs, but open to discussion on that.
I don't think we should include
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #3428: 0.12.1 Cherry-picks: Part 2
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428#issuecomment-956467327
This is ready for review @rdblue, but should come after https://github.com/apache/iceberg/pull/3429:
We don't necessarily need #3264 (Hive: Ensure tableLevelMutex is unlocked when uncommitted metadata de...), but I grabbed it as the test suite was taking quite some time to run.
It's also in both this PR and the initial PR, but wanted to leave the green tests before I rebased and removed it in case there's other changes needed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #3428: 0.12.1 Cherry-Picks: Part 3
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428#discussion_r740482694
##########
File path: mr/src/main/java/org/apache/iceberg/mr/Catalogs.java
##########
@@ -202,7 +202,15 @@ public static boolean dropTable(Configuration conf, Properties props) {
*/
public static boolean hiveCatalog(Configuration conf, Properties props) {
String catalogName = props.getProperty(InputFormatConfig.CATALOG_NAME);
- return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(getCatalogType(conf, catalogName));
+ String catalogType = getCatalogType(conf, catalogName);
+ if (catalogType != null) {
+ return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(catalogType);
+ }
+ catalogType = getCatalogType(conf, ICEBERG_DEFAULT_CATALOG_NAME);
+ if (catalogType != null) {
+ return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(catalogType);
+ }
+ return getCatalogProperties(conf, catalogName, catalogType).get(CatalogProperties.CATALOG_IMPL) == null;
Review comment:
Agreed. Let's not port #3162.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #3428: 0.12.1 Cherry-Picks: Part 3
Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #3428: 0.12.1 Cherry-Picks: Part 3
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428#issuecomment-956536502
Looks great! Thanks, @kbendick. I merged this.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #3428: 0.12.1 Cherry-picks: Part 2
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428#discussion_r739887702
##########
File path: mr/src/main/java/org/apache/iceberg/mr/Catalogs.java
##########
@@ -202,7 +202,15 @@ public static boolean dropTable(Configuration conf, Properties props) {
*/
public static boolean hiveCatalog(Configuration conf, Properties props) {
String catalogName = props.getProperty(InputFormatConfig.CATALOG_NAME);
- return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(getCatalogType(conf, catalogName));
+ String catalogType = getCatalogType(conf, catalogName);
+ if (catalogType != null) {
+ return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(catalogType);
+ }
+ catalogType = getCatalogType(conf, ICEBERG_DEFAULT_CATALOG_NAME);
+ if (catalogType != null) {
+ return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(catalogType);
+ }
+ return getCatalogProperties(conf, catalogName, catalogType).get(CatalogProperties.CATALOG_IMPL) == null;
Review comment:
This whole block didn't cherry-pick cleanly, because we didn't grab this PR: [Core: Throw an exception if both catalog type and catalog-impl are set](https://github.com/apache/iceberg/pull/3162)
This is the PR from which these changes came from. It's the only PR in that file that isn't being grabbed: https://github.com/apache/iceberg/pull/3338
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #3428: 0.12.1 Cherry-picks: Part 2
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428#issuecomment-956467327
This is ready for review @rdblue, but should come after https://github.com/apache/iceberg/pull/3429:
We don't necessarily need #3264 (Hive: Ensure tableLevelMutex is unlocked when uncommitted metadata de...), but I grabbed it as the test suite was taking quite some time to run.
It's also in both this PR and the initial PR, but wanted to leave the green tests before I rebased and removed it in case there's other changes needed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick edited a comment on pull request #3428: 0.12.1 Cherry-Picks: Part 3
Posted by GitBox <gi...@apache.org>.
kbendick edited a comment on pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428#issuecomment-956467327
This is ready for review @rdblue, but should come after https://github.com/apache/iceberg/pull/3429:
We don't necessarily need #3264 (Hive: Ensure tableLevelMutex is unlocked when uncommitted metadata de...), but I grabbed it as the test suite was taking quite some time to run. It also seems like a correctness fix so argubaly we should grab it.
It's also in both this PR and the `Part 2` PR #3429, but wanted to leave the green tests before I rebased and removed it in case there's other changes needed. If that causes issue for rebase and merge, I can remove it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #3428: 0.12.1 Cherry-Picks: Part 3
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3428:
URL: https://github.com/apache/iceberg/pull/3428#discussion_r740482694
##########
File path: mr/src/main/java/org/apache/iceberg/mr/Catalogs.java
##########
@@ -202,7 +202,15 @@ public static boolean dropTable(Configuration conf, Properties props) {
*/
public static boolean hiveCatalog(Configuration conf, Properties props) {
String catalogName = props.getProperty(InputFormatConfig.CATALOG_NAME);
- return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(getCatalogType(conf, catalogName));
+ String catalogType = getCatalogType(conf, catalogName);
+ if (catalogType != null) {
+ return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(catalogType);
+ }
+ catalogType = getCatalogType(conf, ICEBERG_DEFAULT_CATALOG_NAME);
+ if (catalogType != null) {
+ return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(catalogType);
+ }
+ return getCatalogProperties(conf, catalogName, catalogType).get(CatalogProperties.CATALOG_IMPL) == null;
Review comment:
Agreed. Let's not port #3162.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org