jhungund commented on PR #3435:
URL: https://github.com/apache/hive/pull/3435#issuecomment-1197987231
Hi Francis,
Thank you for taking a look at this change.
Below are my replies to your comments (starting with **[JH]**) below
> Can you add an analysis? Like the reason for the bug and how you are
fixing it?
>
**[JH]**I have added a detailed comment about the root cause analysis.
Please take a look.
> From the changes, I can infer that the access is "deferred", moved from
occurring semantic analysis "load table" time, to later when the task actually
runs and does the copy.
**[JH]** Yes, this is right.
>
> But what necessitates needing this? Did some state change between the two
points?
**[JH]** Please check the detailed comment added earlier.
>
> Is this the significant change?
>
> > table = ImportSemanticAnalyzer.tableIfExists(tblDesc, hive);
> > if (table == null) {
> > table = ImportSemanticAnalyzer.createNewTableMetadataObject(tblDesc,
true);
> > }
>
**[JH]** Yes, the access and the subsequent check:
if (AcidUtils.isTransactionalTable(table)) {
returns different results at task creation time and task execution time.
> So at deferred time, the copy/move/etc is only done if the table exists?
>
> Just wondering if you can pass runnable/callable/function from
ImportSemanticAnaylzer, so to keep all that current logic in the same place and
not moved to another file. Then the DeferredWorkHelperImpl just needs to
execute it, and logic can be removed from there.
**[JH]** The current logic of moving code in deferred implementation
simplifies the callers which only need to set the deferred implementation
class. This keeps the logic at the same place as the execution. Hence, it looks
OK to me.
Do let me know, if it looks ok. If not, I can try to change it as per your
comment.
Thanks,
Janardhan
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
