Skip to content
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

HIVE-23820: [HS2] Send tableId in request for get_table_request API #2153

Merged
merged 4 commits into from
Apr 24, 2021

Conversation

ashish-kumar-sharma
Copy link
Contributor

@ashish-kumar-sharma ashish-kumar-sharma commented Apr 6, 2021

What changes were proposed in this pull request?

Sending table Id as part of get_table_request to verify weather table cached in HMS cache store is fresh or stale.

Why are the changes needed?

Change signature of all get_tables apis to request object model to accept tableId.

Does this PR introduce any user-facing change?

New api get_table( GetTableRequest )

How was this patch tested?

by adding and running all exisiting UTest and Qtest

@kishendas
Copy link
Contributor

@ashish-kumar-sharma Are you planning to create one more PR to actually store the tableId in the session and to send the tableId in the request from the current session ?
For example I don't see any changes in "public Table getTable(final String dbName, final String tableName, boolean throwException, boolean checkTransactional, boolean getColumnStats)" method in Hive.java, where you might want to include the tableId in the request which is cached at the session level. Also, don't see how you are storing the tableId in the session.

@ashish-kumar-sharma
Copy link
Contributor Author

@ashish-kumar-sharma Are you planning to create one more PR to actually store the tableId in the session and to send the tableId in the request from the current session ?
For example I don't see any changes in "public Table getTable(final String dbName, final String tableName, boolean throwException, boolean checkTransactional, boolean getColumnStats)" method in Hive.java, where you might want to include the tableId in the request which is cached at the session level. Also, don't see how you are storing the tableId in the session.

@kishendas As of now I am working on that part. I will push these changes as part of this PR only.

@ashish-kumar-sharma
Copy link
Contributor Author

@kishendas @adesh-rao @sankarh

PR complete with following logic

  1. Store table id in session
  2. send table Id as part of all getTable() call
  3. Move redundant getTable() methods to request model and mark them deprecated.
  4. Fixed all failing test failing due to this change

Please review the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants