Upgrade to Hive 4.0.1#299
Merged
Merged
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpgrades the project to Hive 4.0.1 and Thrift 0.16.0, and updates the HiveThriftClient to use the newer Hive metastore API and a safer TSocket initialization pattern compatible with the upgraded libraries. Sequence diagram for updated setStatistics Hive metastore interactionsequenceDiagram
actor TestRunner
participant HiveThriftClient
participant ThriftHiveMetastoreClient
TestRunner->>HiveThriftClient: setStatistics(tableName, tableStatistics)
HiveThriftClient->>HiveThriftClient: getSchema(tableName)
HiveThriftClient->>GetTableRequest: new GetTableRequest(schema, schemalessName)
HiveThriftClient->>ThriftHiveMetastoreClient: get_table_req(getTableRequest)
ThriftHiveMetastoreClient-->>HiveThriftClient: GetTableResult(table)
HiveThriftClient->>HiveThriftClient: setRowsCount(tableName, tableStatistics, table)
HiveThriftClient->>HiveThriftClient: setColumnStatistics(tableName, tableStatistics, table, predicate)
HiveThriftClient-->>TestRunner: return
Sequence diagram for updated HiveThriftClient constructor with TSocket initializationsequenceDiagram
actor TestRunner
participant HiveThriftClient
participant TSocket
TestRunner->>HiveThriftClient: new HiveThriftClient(thriftHost, thriftPort)
activate HiveThriftClient
HiveThriftClient->>TSocket: new TSocket(thriftHost, thriftPort)
HiveThriftClient->>TSocket: open()
TSocket-->>HiveThriftClient: success
HiveThriftClient-->>TestRunner: instance created
deactivate HiveThriftClient
Class diagram for updated HiveThriftClient with new Hive metastore API usageclassDiagram
class HiveThriftClient {
- TTransport transport
- ThriftHiveMetastoreClient client
+ HiveThriftClient(thriftHost, thriftPort)
+ setStatistics(tableName, tableStatistics) void
}
class TableName {
+ getSchemalessNameInDatabase() String
}
class TableStatistics {
}
class GetTableRequest {
+ GetTableRequest(schemaName, tableName)
}
class Table {
}
HiveThriftClient --> ThriftHiveMetastoreClient : uses
HiveThriftClient --> TableName : parameter
HiveThriftClient --> TableStatistics : parameter
HiveThriftClient --> GetTableRequest : constructs
HiveThriftClient --> Table : retrieves
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- By moving
transport = new TSocket(thriftHost, thriftPort);inside the try block,transportcan now remain null ifnew TSocketthrows, which may change downstream behavior (e.g., inclose()or other usages); consider either keeping the assignment outside the try or adding explicit null handling wheretransportis used. - With the switch from
get_tabletoget_table_req, it may be helpful to add a small helper method to construct and invoke theGetTableRequestso the call site remains simple and consistent if additional request parameters or options need to be set in future Hive upgrades.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- By moving `transport = new TSocket(thriftHost, thriftPort);` inside the try block, `transport` can now remain null if `new TSocket` throws, which may change downstream behavior (e.g., in `close()` or other usages); consider either keeping the assignment outside the try or adding explicit null handling where `transport` is used.
- With the switch from `get_table` to `get_table_req`, it may be helpful to add a small helper method to construct and invoke the `GetTableRequest` so the call site remains simple and consistent if additional request parameters or options need to be set in future Hive upgrades.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Draft
6 tasks
Member
|
@imjalpreet Looks good. Did you verify by running any tempto tests? |
Member
Author
|
@nmahadevuni, yes, as I mentioned in the PR description, all the changes have been validated in the linked presto PR. |
tdcmeehan
approved these changes
Apr 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Required for prestodb/presto#24571
The changes have already been tested with CI in the above-linked PR.