-
Notifications
You must be signed in to change notification settings - Fork 2.9k
0.12.1 Cherry-Picks: Part 3 #3428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
0.12.1 Cherry-Picks: Part 3 #3428
Conversation
| testCompile project(path: ':iceberg-spark', configuration: 'testArtifacts') | ||
| testCompile project(path: ':iceberg-spark3', configuration: 'testArtifacts') | ||
|
|
||
| testCompile "org.apache.avro:avro" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the original PR #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 should improve performance by allocating the correct size directly rather than reallocating later.
| if (catalogType != null) { | ||
| return CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE.equalsIgnoreCase(catalogType); | ||
| } | ||
| return getCatalogProperties(conf, catalogName, catalogType).get(CatalogProperties.CATALOG_IMPL) == null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
This is the PR from which these changes came from: Hive: Fix Catalogs.hiveCatalog method for default catalogs #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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Let's not port #3162.
5385df6 to
5850573
Compare
57d3980 to
a50ab92
Compare
|
This is ready for review @rdblue, but should come after #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 |
|
Looks great! Thanks, @kbendick. I merged this. |
This includes the last round of cherry-picks for the 0.12.1 rc0.
It is a continuation of #3402 and #3429.
I will marked the things I needed to change, but mostly it was just updating files as we did not backport the project structural reorganization for multiple spark versions by path.
The only PR cherry-picked that wasn't 100% clean was #3338
Hive: Fix Catalogs.hiveCatalog method for default catalogs. But even that one, the differences between the two were exactly the same, it just wasn't considered clean as there were PRs prior to it that weren't grabbed but they didn't affect the parts we care about (just the line numbers).Otherwise, everything cherry-picked cleanly, except for PRs that came after the structural reorganization which I changed to the new structure but everything was the same.