kishendas commented on pull request #1095: URL: https://github.com/apache/hive/pull/1095#issuecomment-643122983
Main comment: If I understand correctly this gigapatch has 2 mostly distinct part: Run DDL queries in transactions, and generate writeIds for them Optimize writeId handling, so locally committed changes do not cause us to refresh data from HMS I would vote for separating these changes out to 2 standalone patches, so it is easier to understand/review them. Kishen >> We can have a meeting to discuss this. But its more of a functional completeness rather than an optimization . Smaller comments: Commit message should contain the HIVE jira number. Check other pull requests for help :) Kishen >> Sure I think if we decide that we open/commit transaction for DDLs, we should do this on the same place where we do it for DMLs, if possible - I hate that we do this in arbitrary methods inside the Hive class Kishen >> I looked at couple of DMLs and it looked similar. Will discuss with you on this. I guess that the test changes highlight some basic behavioral change - like somehow we start from writeId 2 after the patch. Is this a planned / required change? Kishen >> It depends on how many DDLs have been executed. Please remove formatting only changes from this jira Kishen >> Sure . It came up couple of times because of some default IDE settings. Hopefully we will not see more of 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
