anshuksi282-ksolves commented on code in PR #6650:
URL: https://github.com/apache/hive/pull/6650#discussion_r3671891533
##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java:
##########
@@ -225,12 +242,19 @@ public HiveRelOptMaterialization
createMaterialization(HiveConf conf, Table mate
" ignored; error creating view replacement");
return null;
}
+ final Context context = createContext(conf);
final CBOPlan plan;
try {
- plan = ParseUtils.parseQuery(createContext(conf), viewQuery);
+ plan = ParseUtils.parseQuery(context, viewQuery);
} catch (Exception e) {
LOG.warn("Materialized view " + materializedViewTable.getCompleteName() +
" ignored; error parsing original query; " + e);
+ try {
+ context.clear();
+ } catch (IOException ioe) {
+ LOG.warn("Error while cleaning up staging directories for materialized
view " +
+ materializedViewTable.getCompleteName(), ioe);
+ }
return null;
Review Comment:
> @anshuksi282-ksolves context.clear() is only called on exception, but I
think the success path also abandons the Context without cleanup. Do you think
we should have context.clear() in finally block so it is always called?
Thanks for catching that, @rtrivedi12! You're right — moved context.clear()
into a finally block now, so it's called on both the success and exception
paths. Also fixed the checkstyle indentation issues flagged by SonarQube.
Please take a look when you get a chance.
--
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]
