[ 
https://issues.apache.org/jira/browse/HIVE-18053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16300760#comment-16300760
 ] 

Ashutosh Chauhan commented on HIVE-18053:
-----------------------------------------

Code changes look good. Few comments w.r.t comments:
{code}
 final List<String> qualifiedTableName;
   if (tableRel instanceof Project) {
              qualifiedTableName = 
tableRel.getInput(0).getTable().getQualifiedName();
                    } else {
           qualifiedTableName = tableRel.getTable().getQualifiedName();
            }
{code}
Good to add a comment on when tableRel can be a Project. Also, that it can't be 
of 3rd type.

{code}
DruidQuery.create(cluster, cluster.traitSetOf(BindableConvention.INSTANCE),
{code}
Good to a comment of effect that ideally we shall use HiveRelNode convention. 
But since Volcano planner throws in that case, we set it as Bindable. Since,
we do't really use convention currently its ok. In future if we want to make 
use of convention (e.g., while directly generation op tree instead of AST) this 
needs to be changed.

{code}
String viewText = tab.getViewExpandedText();
{code}
Good to add a comment on why this needs expanded text.

Also patch introduces checkstyle warnings. 

> Support different table types for MVs
> -------------------------------------
>
>                 Key: HIVE-18053
>                 URL: https://issues.apache.org/jira/browse/HIVE-18053
>             Project: Hive
>          Issue Type: Sub-task
>          Components: Materialized views
>    Affects Versions: 3.0.0
>            Reporter: Jesus Camacho Rodriguez
>            Assignee: Jesus Camacho Rodriguez
>            Priority: Critical
>         Attachments: HIVE-18053.patch
>
>
> MVs backed by MM tables, managed tables, external tables. This might work 
> already, but we need to add tests.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to