You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Miklos Gergely <mg...@hortonworks.com> on 2020/04/25 11:23:30 UTC
Review Request 72433: Extract Create View analyzer from
SemanticAnalyzer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72433/
-----------------------------------------------------------
Review request for hive and Jesús Camacho Rodríguez.
Bugs: HIVE-23244
https://issues.apache.org/jira/browse/HIVE-23244
Repository: hive-git
Description
-------
Create View commands are not queries, but commands which have queries as a part of them. Therefore a separate CreateViewAnalyzer is needed which uses SemanticAnalyer to analyze it's query.
Diffs
-----
common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java 8e643fe844
parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g b03b0989b8
ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLUtils.java b82fc5e91d
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewAnalyzer.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsAnalyzer.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewAnalyzer.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewDesc.java d1f36945fb
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewOperation.java f7952a5cc1
ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java b578d48ce1
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 4f1e23d7a6
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 7b2e201e5a
ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java bef02176c2
ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java 9d94b6e2dd
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 0de3730351
ql/src/java/org/apache/hadoop/hive/ql/parse/StorageFormat.java 2350646c36
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 2f3fc6c50a
ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java c75829c272
ql/src/java/org/apache/hadoop/hive/ql/plan/LoadFileDesc.java 07bcef8ee3
ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java 980f39b681
ql/src/test/results/clientnegative/create_or_replace_view4.q.out 767cc77596
ql/src/test/results/clientnegative/create_view_failure3.q.out 8b79272f46
ql/src/test/results/clientnegative/create_view_failure5.q.out b7b3984292
ql/src/test/results/clientnegative/create_view_failure6.q.out 6d9fb6461d
ql/src/test/results/clientnegative/create_view_failure7.q.out 337dbe889e
ql/src/test/results/clientnegative/create_view_failure8.q.out cccb7e4b06
ql/src/test/results/clientnegative/create_view_failure9.q.out eac8cb489e
ql/src/test/results/clientnegative/selectDistinctStarNeg_1.q.out 9496e528e2
ql/src/test/results/clientpositive/create_view.q.out 7414d4749d
ql/src/test/results/clientpositive/explain_ddl.q.out aea46d304f
ql/src/test/results/clientpositive/llap/create_view_translate.q.out b5d464e716
ql/src/test/results/clientpositive/llap/explainuser_1.q.out 7f0ce5a9c7
ql/src/test/results/clientpositive/llap/lineage3.q.out a4f6e61af4
ql/src/test/results/clientpositive/llap/selectDistinctStar.q.out 3fc0074ed4
ql/src/test/results/clientpositive/llap/union_top_level.q.out e0d86b3989
ql/src/test/results/clientpositive/llap/vector_windowing.q.out ca3c6337bf
ql/src/test/results/clientpositive/tez/explainanalyze_3.q.out dad999ab1a
ql/src/test/results/clientpositive/tez/explainuser_3.q.out 5218c90f0c
Diff: https://reviews.apache.org/r/72433/diff/1/
Testing
-------
All the tests are still running fine.
Thanks,
Miklos Gergely
Re: Review Request 72433: Extract Create View analyzer from
SemanticAnalyzer
Posted by Miklos Gergely <mg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72433/
-----------------------------------------------------------
(Updated June 3, 2020, 4:35 a.m.)
Review request for hive and Jesús Camacho Rodríguez.
Bugs: HIVE-23244
https://issues.apache.org/jira/browse/HIVE-23244
Repository: hive-git
Description
-------
Create View commands are not queries, but commands which have queries as a part of them. Therefore a separate CreateViewAnalyzer is needed which uses SemanticAnalyer to analyze it's query.
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java d732004e51
parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 768a3a17a7
ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLUtils.java b82fc5e91d
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewAnalyzer.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsAnalyzer.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewAnalyzer.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewDesc.java d1f36945fb
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewOperation.java f7952a5cc1
ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java 792e331884
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 377e8280e5
ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java da443f419c
ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java 9d94b6e2dd
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 8238a2a4a2
ql/src/java/org/apache/hadoop/hive/ql/parse/StorageFormat.java 2350646c36
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 2f3fc6c50a
ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java c75829c272
ql/src/java/org/apache/hadoop/hive/ql/plan/LoadFileDesc.java 07bcef8ee3
ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java 2fb452bea5
ql/src/test/results/clientnegative/create_or_replace_view4.q.out 767cc77596
ql/src/test/results/clientnegative/create_view_failure3.q.out 8b79272f46
ql/src/test/results/clientnegative/create_view_failure5.q.out b7b3984292
ql/src/test/results/clientnegative/create_view_failure6.q.out 6d9fb6461d
ql/src/test/results/clientnegative/create_view_failure7.q.out 337dbe889e
ql/src/test/results/clientnegative/create_view_failure8.q.out cccb7e4b06
ql/src/test/results/clientnegative/create_view_failure9.q.out eac8cb489e
ql/src/test/results/clientnegative/masking_mv.q.out 926c5062ce
ql/src/test/results/clientnegative/selectDistinctStarNeg_1.q.out 9496e528e2
ql/src/test/results/clientpositive/llap/create_view.q.out 52b77c7505
ql/src/test/results/clientpositive/llap/create_view_translate.q.out b5d464e716
ql/src/test/results/clientpositive/llap/explain_ddl.q.out 3cada1bbae
ql/src/test/results/clientpositive/llap/explainuser_1.q.out a2f241c470
ql/src/test/results/clientpositive/llap/lineage3.q.out c87d7c0c92
ql/src/test/results/clientpositive/llap/masking_mv.q.out 196688a17d
ql/src/test/results/clientpositive/llap/materialized_view_cluster.q.out e34147d1da
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_3.q.out 6e6ee34361
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_4.q.out 23489244bd
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_rebuild_dummy.q.out d4bf2a6d8b
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_time_window.q.out dc0e861863
ql/src/test/results/clientpositive/llap/materialized_view_distribute_sort.q.out cdcc356ae3
ql/src/test/results/clientpositive/llap/materialized_view_partition_cluster.q.out 4d6de9d8f7
ql/src/test/results/clientpositive/llap/materialized_view_partitioned.q.out e463b3cba6
ql/src/test/results/clientpositive/llap/materialized_view_partitioned_3.q.out 8bdbf13d8a
ql/src/test/results/clientpositive/llap/selectDistinctStar.q.out 3fc0074ed4
ql/src/test/results/clientpositive/llap/sketches_materialized_view_safety.q.out ff079587fd
ql/src/test/results/clientpositive/llap/union_top_level.q.out f846cb2fc1
ql/src/test/results/clientpositive/llap/vector_windowing.q.out ba31832201
ql/src/test/results/clientpositive/tez/explainanalyze_3.q.out 33546dd74f
ql/src/test/results/clientpositive/tez/explainuser_3.q.out f1c245b671
Diff: https://reviews.apache.org/r/72433/diff/4/
Changes: https://reviews.apache.org/r/72433/diff/3-4/
Testing
-------
All the tests are still running fine.
Thanks,
Miklos Gergely
Re: Review Request 72433: Extract Create View analyzer from
SemanticAnalyzer
Posted by Miklos Gergely <mg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72433/
-----------------------------------------------------------
(Updated June 3, 2020, 2:54 a.m.)
Review request for hive and Jesús Camacho Rodríguez.
Bugs: HIVE-23244
https://issues.apache.org/jira/browse/HIVE-23244
Repository: hive-git
Description
-------
Create View commands are not queries, but commands which have queries as a part of them. Therefore a separate CreateViewAnalyzer is needed which uses SemanticAnalyer to analyze it's query.
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java d732004e51
parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 768a3a17a7
ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLUtils.java b82fc5e91d
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewAnalyzer.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsAnalyzer.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewAnalyzer.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewDesc.java d1f36945fb
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewOperation.java f7952a5cc1
ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java 792e331884
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 377e8280e5
ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java da443f419c
ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java 9d94b6e2dd
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 8238a2a4a2
ql/src/java/org/apache/hadoop/hive/ql/parse/StorageFormat.java 2350646c36
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 2f3fc6c50a
ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java c75829c272
ql/src/java/org/apache/hadoop/hive/ql/plan/LoadFileDesc.java 07bcef8ee3
ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java 2fb452bea5
ql/src/test/results/clientnegative/create_or_replace_view4.q.out 767cc77596
ql/src/test/results/clientnegative/create_view_failure3.q.out 8b79272f46
ql/src/test/results/clientnegative/create_view_failure5.q.out b7b3984292
ql/src/test/results/clientnegative/create_view_failure6.q.out 6d9fb6461d
ql/src/test/results/clientnegative/create_view_failure7.q.out 337dbe889e
ql/src/test/results/clientnegative/create_view_failure8.q.out cccb7e4b06
ql/src/test/results/clientnegative/create_view_failure9.q.out eac8cb489e
ql/src/test/results/clientnegative/selectDistinctStarNeg_1.q.out 9496e528e2
ql/src/test/results/clientpositive/llap/create_view.q.out 52b77c7505
ql/src/test/results/clientpositive/llap/create_view_translate.q.out b5d464e716
ql/src/test/results/clientpositive/llap/explain_ddl.q.out 3cada1bbae
ql/src/test/results/clientpositive/llap/explainuser_1.q.out a2f241c470
ql/src/test/results/clientpositive/llap/lineage3.q.out c87d7c0c92
ql/src/test/results/clientpositive/llap/masking_mv.q.out 196688a17d
ql/src/test/results/clientpositive/llap/materialized_view_cluster.q.out e34147d1da
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_3.q.out 6e6ee34361
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_4.q.out 23489244bd
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_rebuild_dummy.q.out d4bf2a6d8b
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_time_window.q.out dc0e861863
ql/src/test/results/clientpositive/llap/materialized_view_distribute_sort.q.out cdcc356ae3
ql/src/test/results/clientpositive/llap/materialized_view_partition_cluster.q.out 4d6de9d8f7
ql/src/test/results/clientpositive/llap/materialized_view_partitioned.q.out e463b3cba6
ql/src/test/results/clientpositive/llap/materialized_view_partitioned_3.q.out 8bdbf13d8a
ql/src/test/results/clientpositive/llap/selectDistinctStar.q.out 3fc0074ed4
ql/src/test/results/clientpositive/llap/union_top_level.q.out f846cb2fc1
ql/src/test/results/clientpositive/llap/vector_windowing.q.out ba31832201
ql/src/test/results/clientpositive/tez/explainanalyze_3.q.out 33546dd74f
ql/src/test/results/clientpositive/tez/explainuser_3.q.out f1c245b671
Diff: https://reviews.apache.org/r/72433/diff/3/
Changes: https://reviews.apache.org/r/72433/diff/2-3/
Testing
-------
All the tests are still running fine.
Thanks,
Miklos Gergely
Re: Review Request 72433: Extract Create View analyzer from
SemanticAnalyzer
Posted by Miklos Gergely <mg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72433/
-----------------------------------------------------------
(Updated May 24, 2020, 8:13 a.m.)
Review request for hive and Jesús Camacho Rodríguez.
Changes
-------
Rebase.
Bugs: HIVE-23244
https://issues.apache.org/jira/browse/HIVE-23244
Repository: hive-git
Description
-------
Create View commands are not queries, but commands which have queries as a part of them. Therefore a separate CreateViewAnalyzer is needed which uses SemanticAnalyer to analyze it's query.
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java d732004e51
parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 768a3a17a7
ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLUtils.java b82fc5e91d
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewAnalyzer.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsAnalyzer.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewAnalyzer.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewDesc.java d1f36945fb
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewOperation.java f7952a5cc1
ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java 7a309626cc
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 19eb1dffc8
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 377e8280e5
ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java da443f419c
ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java 9d94b6e2dd
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java aa8d84ec9c
ql/src/java/org/apache/hadoop/hive/ql/parse/StorageFormat.java 2350646c36
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 2f3fc6c50a
ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java c75829c272
ql/src/java/org/apache/hadoop/hive/ql/plan/LoadFileDesc.java 07bcef8ee3
ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java 2fb452bea5
ql/src/test/results/clientnegative/create_or_replace_view4.q.out 767cc77596
ql/src/test/results/clientnegative/create_view_failure3.q.out 8b79272f46
ql/src/test/results/clientnegative/create_view_failure5.q.out b7b3984292
ql/src/test/results/clientnegative/create_view_failure6.q.out 6d9fb6461d
ql/src/test/results/clientnegative/create_view_failure7.q.out 337dbe889e
ql/src/test/results/clientnegative/create_view_failure8.q.out cccb7e4b06
ql/src/test/results/clientnegative/create_view_failure9.q.out eac8cb489e
ql/src/test/results/clientnegative/selectDistinctStarNeg_1.q.out 9496e528e2
ql/src/test/results/clientpositive/llap/create_view.q.out 52b77c7505
ql/src/test/results/clientpositive/llap/create_view_translate.q.out b5d464e716
ql/src/test/results/clientpositive/llap/explain_ddl.q.out 20a7ee846f
ql/src/test/results/clientpositive/llap/explainuser_1.q.out f10356ab47
ql/src/test/results/clientpositive/llap/lineage3.q.out d762fbc049
ql/src/test/results/clientpositive/llap/selectDistinctStar.q.out 3fc0074ed4
ql/src/test/results/clientpositive/llap/union_top_level.q.out e0d86b3989
ql/src/test/results/clientpositive/llap/vector_windowing.q.out e1a10916e5
ql/src/test/results/clientpositive/tez/explainanalyze_3.q.out 5af43def9c
ql/src/test/results/clientpositive/tez/explainuser_3.q.out d886a26f78
Diff: https://reviews.apache.org/r/72433/diff/2/
Changes: https://reviews.apache.org/r/72433/diff/1-2/
Testing
-------
All the tests are still running fine.
Thanks,
Miklos Gergely
Re: Review Request 72433: Extract Create View analyzer from
SemanticAnalyzer
Posted by Miklos Gergely <mg...@hortonworks.com>.
> On May 22, 2020, 10:18 p.m., Jesús Camacho Rodríguez wrote:
> > common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java
> > Line 442 (original)
> > <https://reviews.apache.org/r/72433/diff/1/?file=2227713#file2227713line442>
> >
> > Why is this error code going away? Even if it is not used probably makes sense to keep it around and mark it as deprecated for backwards reference.
I agree, put it back, and added deprecation with explanation.
> On May 22, 2020, 10:18 p.m., Jesús Camacho Rodríguez wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewDesc.java
> > Lines 49 (patched)
> > <https://reviews.apache.org/r/72433/diff/1/?file=2227721#file2227721line49>
> >
> > It seems more appropiate to update the display name too at this point.
I was planning to do that in the next patch, when I'm going to clean up materialized view creation, but it can be done here too. I've modified the patch.
> On May 22, 2020, 10:18 p.m., Jesús Camacho Rodríguez wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewDesc.java
> > Lines 67 (patched)
> > <https://reviews.apache.org/r/72433/diff/1/?file=2227721#file2227721line67>
> >
> > // only used for materialized views
> >
> > These comments can go away.
I was planning to do that in the next patch, when I'm going to clean up materialized view creation, but it can be done here too. I've modified the patch.
> On May 22, 2020, 10:18 p.m., Jesús Camacho Rodríguez wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java
> > Line 1818 (original), 1818 (patched)
> > <https://reviews.apache.org/r/72433/diff/1/?file=2227727#file2227727line1818>
> >
> > Can we make this protected again? I did not see any usage that is not by a subclass?
Sure, it's protected again.
> On May 22, 2020, 10:18 p.m., Jesús Camacho Rodríguez wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
> > Lines 566 (patched)
> > <https://reviews.apache.org/r/72433/diff/1/?file=2227728#file2227728line566>
> >
> > What is the goal of this block? Why is this needed now and it was not needed before? This seems more than a refactoring.
> >
> > Fwiw there is some logic below executed when we create a view in _handleCreateViewDDL_.
We needed this before as well, this is what replaces handleCreateViewDDL for view creation. Please see the explanation below in the next issue about the new paradigm. From now on the condition
cboCtx.type == PreCboCtx.Type.VIEW
will not be true for CREATE VIEW commands as the SA won't "know" that it is parsing a query for a view creation. I've tried to move all DDL related things into the DDL analyzer (CreateViewAnalyzer), right now these two commands found here had to stay here, but the rest of handleCreateViewDDL is now happening in the CreateViewAnalyzer.
Just to be clear, in the near future I'm planning to remove handleCreateViewDDL completely, once CREATE MATERIALIZED VIEW command will have their own analyzer too, and they will use SA only to analyze their query in a similar fashion.
> On May 22, 2020, 10:18 p.m., Jesús Camacho Rodríguez wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
> > Line 1848 (original), 1854 (patched)
> > <https://reviews.apache.org/r/72433/diff/1/?file=2227728#file2227728line1854>
> >
> > Why are we changing this? We should not have to change this logic since isView semantics should not change.
isView was removed. When there is a CREATE VIEW command then it is parsed into an AST tree, which containes some info about the view (lets call it V) and a query (lets call it Q).
So far the whole tree was sent to the SemanticAnalyzer, kind of like saying that a CREATE VIEW command is just a special query. Then the SA identified that it is actually a CREATE VIEW command, so it created a CreateViewDesc for it, analyzing the V part, and saved it in the actual QB instance - no idea why there, I believe it just had to be put somewhere. Then the Q part was analyzed like every other query, and the results of this analysis were added to the CreateViewDesc, which could be always found in the QB.
After this change we'll have a new paradigm: a CREATE VIEW command is not a special query, it is a DDL, which contains a query. We'll have a CreateViewAnalyzer, which analyzes the V part, and as the Q part is a query, it sends the Q part, and only the Q part to a new SA. Ideally the SA would be 100% agnostic of what it is analyzing, whether it is a "regular" query, or a query of a CREATE VIEW command, but so far there are parts of the code which needs to know this. After this change it can't be done by looking at the QB if it contains a CreateViewDesc, but we still need this information, so I've introduced a forViewCreation global variable for now. I hope that in the future we'll find a way to rermove it, and make the SA totally agnostic of the source of the query, and then the forViewCreation will disappear too, but for now it is needed.
After this one is merged I'm planning to have a patch for removing the CREATE MATERIALIZED VIEW commands from the SA, then the getQB().isMaterializedView() command will disappear as well.
> On May 22, 2020, 10:18 p.m., Jesús Camacho Rodríguez wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
> > Line 12614 (original), 12620 (patched)
> > <https://reviews.apache.org/r/72433/diff/1/?file=2227731#file2227731line12624>
> >
> > Not sure why we need this change since information should be in qb.
Same as above.
- Miklos
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72433/#review220850
-----------------------------------------------------------
On June 3, 2020, 2:54 a.m., Miklos Gergely wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72433/
> -----------------------------------------------------------
>
> (Updated June 3, 2020, 2:54 a.m.)
>
>
> Review request for hive and Jesús Camacho Rodríguez.
>
>
> Bugs: HIVE-23244
> https://issues.apache.org/jira/browse/HIVE-23244
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create View commands are not queries, but commands which have queries as a part of them. Therefore a separate CreateViewAnalyzer is needed which uses SemanticAnalyer to analyze it's query.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java d732004e51
> parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 768a3a17a7
> ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLUtils.java b82fc5e91d
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewAnalyzer.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsAnalyzer.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewAnalyzer.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewDesc.java d1f36945fb
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewOperation.java f7952a5cc1
> ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java 792e331884
> ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 377e8280e5
> ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java da443f419c
> ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java 9d94b6e2dd
> ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 8238a2a4a2
> ql/src/java/org/apache/hadoop/hive/ql/parse/StorageFormat.java 2350646c36
> ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 2f3fc6c50a
> ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java c75829c272
> ql/src/java/org/apache/hadoop/hive/ql/plan/LoadFileDesc.java 07bcef8ee3
> ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java 2fb452bea5
> ql/src/test/results/clientnegative/create_or_replace_view4.q.out 767cc77596
> ql/src/test/results/clientnegative/create_view_failure3.q.out 8b79272f46
> ql/src/test/results/clientnegative/create_view_failure5.q.out b7b3984292
> ql/src/test/results/clientnegative/create_view_failure6.q.out 6d9fb6461d
> ql/src/test/results/clientnegative/create_view_failure7.q.out 337dbe889e
> ql/src/test/results/clientnegative/create_view_failure8.q.out cccb7e4b06
> ql/src/test/results/clientnegative/create_view_failure9.q.out eac8cb489e
> ql/src/test/results/clientnegative/selectDistinctStarNeg_1.q.out 9496e528e2
> ql/src/test/results/clientpositive/llap/create_view.q.out 52b77c7505
> ql/src/test/results/clientpositive/llap/create_view_translate.q.out b5d464e716
> ql/src/test/results/clientpositive/llap/explain_ddl.q.out 3cada1bbae
> ql/src/test/results/clientpositive/llap/explainuser_1.q.out a2f241c470
> ql/src/test/results/clientpositive/llap/lineage3.q.out c87d7c0c92
> ql/src/test/results/clientpositive/llap/masking_mv.q.out 196688a17d
> ql/src/test/results/clientpositive/llap/materialized_view_cluster.q.out e34147d1da
> ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_3.q.out 6e6ee34361
> ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_4.q.out 23489244bd
> ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_rebuild_dummy.q.out d4bf2a6d8b
> ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_time_window.q.out dc0e861863
> ql/src/test/results/clientpositive/llap/materialized_view_distribute_sort.q.out cdcc356ae3
> ql/src/test/results/clientpositive/llap/materialized_view_partition_cluster.q.out 4d6de9d8f7
> ql/src/test/results/clientpositive/llap/materialized_view_partitioned.q.out e463b3cba6
> ql/src/test/results/clientpositive/llap/materialized_view_partitioned_3.q.out 8bdbf13d8a
> ql/src/test/results/clientpositive/llap/selectDistinctStar.q.out 3fc0074ed4
> ql/src/test/results/clientpositive/llap/union_top_level.q.out f846cb2fc1
> ql/src/test/results/clientpositive/llap/vector_windowing.q.out ba31832201
> ql/src/test/results/clientpositive/tez/explainanalyze_3.q.out 33546dd74f
> ql/src/test/results/clientpositive/tez/explainuser_3.q.out f1c245b671
>
>
> Diff: https://reviews.apache.org/r/72433/diff/3/
>
>
> Testing
> -------
>
> All the tests are still running fine.
>
>
> Thanks,
>
> Miklos Gergely
>
>
Re: Review Request 72433: Extract Create View analyzer from
SemanticAnalyzer
Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72433/#review220850
-----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java
Line 442 (original)
<https://reviews.apache.org/r/72433/#comment309568>
Why is this error code going away? Even if it is not used probably makes sense to keep it around and mark it as deprecated for backwards reference.
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewDesc.java
Lines 49 (patched)
<https://reviews.apache.org/r/72433/#comment309566>
It seems more appropiate to update the display name too at this point.
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewDesc.java
Lines 67 (patched)
<https://reviews.apache.org/r/72433/#comment309567>
// only used for materialized views
These comments can go away.
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java
Line 1818 (original), 1818 (patched)
<https://reviews.apache.org/r/72433/#comment309569>
Can we make this protected again? I did not see any usage that is not by a subclass?
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Lines 566 (patched)
<https://reviews.apache.org/r/72433/#comment309570>
What is the goal of this block? Why is this needed now and it was not needed before? This seems more than a refactoring.
Fwiw there is some logic below executed when we create a view in _handleCreateViewDDL_.
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Line 1848 (original), 1854 (patched)
<https://reviews.apache.org/r/72433/#comment309571>
Why are we changing this? We should not have to change this logic since isView semantics should not change.
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
Line 12614 (original), 12620 (patched)
<https://reviews.apache.org/r/72433/#comment309572>
Not sure why we need this change since information should be in qb.
- Jesús Camacho Rodríguez
On April 25, 2020, 11:23 a.m., Miklos Gergely wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72433/
> -----------------------------------------------------------
>
> (Updated April 25, 2020, 11:23 a.m.)
>
>
> Review request for hive and Jesús Camacho Rodríguez.
>
>
> Bugs: HIVE-23244
> https://issues.apache.org/jira/browse/HIVE-23244
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create View commands are not queries, but commands which have queries as a part of them. Therefore a separate CreateViewAnalyzer is needed which uses SemanticAnalyer to analyze it's query.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java 8e643fe844
> parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g b03b0989b8
> ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLUtils.java b82fc5e91d
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewAnalyzer.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsAnalyzer.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewAnalyzer.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewDesc.java d1f36945fb
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewOperation.java f7952a5cc1
> ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java b578d48ce1
> ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 4f1e23d7a6
> ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 7b2e201e5a
> ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java bef02176c2
> ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java 9d94b6e2dd
> ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 0de3730351
> ql/src/java/org/apache/hadoop/hive/ql/parse/StorageFormat.java 2350646c36
> ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 2f3fc6c50a
> ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java c75829c272
> ql/src/java/org/apache/hadoop/hive/ql/plan/LoadFileDesc.java 07bcef8ee3
> ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java 980f39b681
> ql/src/test/results/clientnegative/create_or_replace_view4.q.out 767cc77596
> ql/src/test/results/clientnegative/create_view_failure3.q.out 8b79272f46
> ql/src/test/results/clientnegative/create_view_failure5.q.out b7b3984292
> ql/src/test/results/clientnegative/create_view_failure6.q.out 6d9fb6461d
> ql/src/test/results/clientnegative/create_view_failure7.q.out 337dbe889e
> ql/src/test/results/clientnegative/create_view_failure8.q.out cccb7e4b06
> ql/src/test/results/clientnegative/create_view_failure9.q.out eac8cb489e
> ql/src/test/results/clientnegative/selectDistinctStarNeg_1.q.out 9496e528e2
> ql/src/test/results/clientpositive/create_view.q.out 7414d4749d
> ql/src/test/results/clientpositive/explain_ddl.q.out aea46d304f
> ql/src/test/results/clientpositive/llap/create_view_translate.q.out b5d464e716
> ql/src/test/results/clientpositive/llap/explainuser_1.q.out 7f0ce5a9c7
> ql/src/test/results/clientpositive/llap/lineage3.q.out a4f6e61af4
> ql/src/test/results/clientpositive/llap/selectDistinctStar.q.out 3fc0074ed4
> ql/src/test/results/clientpositive/llap/union_top_level.q.out e0d86b3989
> ql/src/test/results/clientpositive/llap/vector_windowing.q.out ca3c6337bf
> ql/src/test/results/clientpositive/tez/explainanalyze_3.q.out dad999ab1a
> ql/src/test/results/clientpositive/tez/explainuser_3.q.out 5218c90f0c
>
>
> Diff: https://reviews.apache.org/r/72433/diff/1/
>
>
> Testing
> -------
>
> All the tests are still running fine.
>
>
> Thanks,
>
> Miklos Gergely
>
>