You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Sergey Shelukhin <se...@hortonworks.com> on 2017/12/12 21:46:16 UTC

Review Request 64555: HIVE-18230 create plan like plan, and replace plan commands for easy modification

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64555/
-----------------------------------------------------------

Review request for hive, Harish Jaiprakash and Prasanth_J.


Repository: hive-git


Description
-------

see jira


Diffs
-----

  itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java 62c9172ef5d9ae74b158d1b4f1b8c5c0eca4e375 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 55ef8de9a5c7144931d0a6ff13224765ee737fea 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 50bdce89a44a8dc87a97e394d00e5dadebbbd351 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java a09b7961c2dbc26b4d2fa912d0be7037885f63e4 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 1dcfe9dd2834ada2f2bb75bc67e436017c21f71f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 13789502c235be478659f9267b107e8580594bc8 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g f1ca30109fc40b452f0e1423a586818bcb783796 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ResourcePlanParser.g 95c87259c5bdb91a4533d7897d5d32a7c4f56503 
  ql/src/java/org/apache/hadoop/hive/ql/plan/AlterResourcePlanDesc.java d70d52054dbef5ecbe5be55fd24df25c5381c8d7 
  ql/src/java/org/apache/hadoop/hive/ql/plan/CreateResourcePlanDesc.java efdd05c5f270e26b84a0e022eac0445ff368969b 
  ql/src/test/queries/clientpositive/resourceplan.q fc924a2f95a849b6e14065cb62c2910db40d1b47 
  ql/src/test/results/clientpositive/llap/resourceplan.q.out 7f3e784457f87ff88c5f746a9fc2b74ea28c2a6e 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 6e0da5781ed8d536852613bad1793d27dd17dfd4 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java fc254c6f5338ebcfe5a19bea847ff8141552f15f 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 573ac0173dd51b81cb37902cc144983393ab1f51 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java 2c92bb20561d09998e400b458036f8e4a960a469 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java 75fbfa23d25c589826a44be493eb84f3cd375e5e 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java da518ab6e337cbd5befb9f7c1df1781335bd72f4 
  standalone-metastore/src/main/thrift/hive_metastore.thrift 1085ce566a5fba632ae2acb074ce538143bac6ed 
  standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java 24c59f2f1b0b017cff3b26f86c3f903b498d2553 
  standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java 1e4fe5d973542d6b0e3ae0760ca5c06ea9888089 


Diff: https://reviews.apache.org/r/64555/diff/1/


Testing
-------


Thanks,

Sergey Shelukhin


Re: Review Request 64555: HIVE-18230 create plan like plan, and replace plan commands for easy modification

Posted by Harish Jaiprakash <hj...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64555/#review194052
-----------------------------------------------------------


Ship it!




Ship It!

- Harish Jaiprakash


On Dec. 16, 2017, 3:46 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64555/
> -----------------------------------------------------------
> 
> (Updated Dec. 16, 2017, 3:46 a.m.)
> 
> 
> Review request for hive, Harish Jaiprakash and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java 62c9172ef5d9ae74b158d1b4f1b8c5c0eca4e375 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 55ef8de9a5c7144931d0a6ff13224765ee737fea 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 50bdce89a44a8dc87a97e394d00e5dadebbbd351 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java a09b7961c2dbc26b4d2fa912d0be7037885f63e4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 1dcfe9dd2834ada2f2bb75bc67e436017c21f71f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 13789502c235be478659f9267b107e8580594bc8 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g f1ca30109fc40b452f0e1423a586818bcb783796 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ResourcePlanParser.g 95c87259c5bdb91a4533d7897d5d32a7c4f56503 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterResourcePlanDesc.java d70d52054dbef5ecbe5be55fd24df25c5381c8d7 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/CreateResourcePlanDesc.java efdd05c5f270e26b84a0e022eac0445ff368969b 
>   ql/src/test/queries/clientpositive/resourceplan.q fc924a2f95a849b6e14065cb62c2910db40d1b47 
>   ql/src/test/results/clientpositive/llap/resourceplan.q.out 7f3e784457f87ff88c5f746a9fc2b74ea28c2a6e 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 6e0da5781ed8d536852613bad1793d27dd17dfd4 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java fc254c6f5338ebcfe5a19bea847ff8141552f15f 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 573ac0173dd51b81cb37902cc144983393ab1f51 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java 2c92bb20561d09998e400b458036f8e4a960a469 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java 75fbfa23d25c589826a44be493eb84f3cd375e5e 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java da518ab6e337cbd5befb9f7c1df1781335bd72f4 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 1085ce566a5fba632ae2acb074ce538143bac6ed 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java 24c59f2f1b0b017cff3b26f86c3f903b498d2553 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java 1e4fe5d973542d6b0e3ae0760ca5c06ea9888089 
> 
> 
> Diff: https://reviews.apache.org/r/64555/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 64555: HIVE-18230 create plan like plan, and replace plan commands for easy modification

Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64555/
-----------------------------------------------------------

(Updated Dec. 15, 2017, 10:16 p.m.)


Review request for hive, Harish Jaiprakash and Prasanth_J.


Repository: hive-git


Description
-------

see jira


Diffs (updated)
-----

  itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java 62c9172ef5d9ae74b158d1b4f1b8c5c0eca4e375 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 55ef8de9a5c7144931d0a6ff13224765ee737fea 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 50bdce89a44a8dc87a97e394d00e5dadebbbd351 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java a09b7961c2dbc26b4d2fa912d0be7037885f63e4 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 1dcfe9dd2834ada2f2bb75bc67e436017c21f71f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 13789502c235be478659f9267b107e8580594bc8 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g f1ca30109fc40b452f0e1423a586818bcb783796 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ResourcePlanParser.g 95c87259c5bdb91a4533d7897d5d32a7c4f56503 
  ql/src/java/org/apache/hadoop/hive/ql/plan/AlterResourcePlanDesc.java d70d52054dbef5ecbe5be55fd24df25c5381c8d7 
  ql/src/java/org/apache/hadoop/hive/ql/plan/CreateResourcePlanDesc.java efdd05c5f270e26b84a0e022eac0445ff368969b 
  ql/src/test/queries/clientpositive/resourceplan.q fc924a2f95a849b6e14065cb62c2910db40d1b47 
  ql/src/test/results/clientpositive/llap/resourceplan.q.out 7f3e784457f87ff88c5f746a9fc2b74ea28c2a6e 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 6e0da5781ed8d536852613bad1793d27dd17dfd4 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java fc254c6f5338ebcfe5a19bea847ff8141552f15f 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 573ac0173dd51b81cb37902cc144983393ab1f51 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java 2c92bb20561d09998e400b458036f8e4a960a469 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java 75fbfa23d25c589826a44be493eb84f3cd375e5e 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java da518ab6e337cbd5befb9f7c1df1781335bd72f4 
  standalone-metastore/src/main/thrift/hive_metastore.thrift 1085ce566a5fba632ae2acb074ce538143bac6ed 
  standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java 24c59f2f1b0b017cff3b26f86c3f903b498d2553 
  standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java 1e4fe5d973542d6b0e3ae0760ca5c06ea9888089 


Diff: https://reviews.apache.org/r/64555/diff/2/

Changes: https://reviews.apache.org/r/64555/diff/1-2/


Testing
-------


Thanks,

Sergey Shelukhin


Re: Review Request 64555: HIVE-18230 create plan like plan, and replace plan commands for easy modification

Posted by Sergey Shelukhin <se...@hortonworks.com>.

> On Dec. 15, 2017, 4:48 a.m., Harish Jaiprakash wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
> > Lines 9869 (patched)
> > <https://reviews.apache.org/r/64555/diff/1/?file=1914833#file1914833line9882>
> >
> >     Hmm... if people use replace plan frequently as a way of updating an existing plan then this list will grow long and we'll be making one call to db per rp. Would using a like query to get all the names and using a set to store the name make it faster?
> >     
> >     Another way would be to use as version field used in conjunction with name to create a unique field. Since we want to implement version sometime this might be useful.

We can implement versioning in phase 2 :) 
As for the long list, note that the versioned name includes a timestamp, so in realistic use cases, we will only check once and there won't be a match.


- Sergey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64555/#review193774
-----------------------------------------------------------


On Dec. 12, 2017, 9:46 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64555/
> -----------------------------------------------------------
> 
> (Updated Dec. 12, 2017, 9:46 p.m.)
> 
> 
> Review request for hive, Harish Jaiprakash and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java 62c9172ef5d9ae74b158d1b4f1b8c5c0eca4e375 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 55ef8de9a5c7144931d0a6ff13224765ee737fea 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 50bdce89a44a8dc87a97e394d00e5dadebbbd351 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java a09b7961c2dbc26b4d2fa912d0be7037885f63e4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 1dcfe9dd2834ada2f2bb75bc67e436017c21f71f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 13789502c235be478659f9267b107e8580594bc8 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g f1ca30109fc40b452f0e1423a586818bcb783796 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ResourcePlanParser.g 95c87259c5bdb91a4533d7897d5d32a7c4f56503 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterResourcePlanDesc.java d70d52054dbef5ecbe5be55fd24df25c5381c8d7 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/CreateResourcePlanDesc.java efdd05c5f270e26b84a0e022eac0445ff368969b 
>   ql/src/test/queries/clientpositive/resourceplan.q fc924a2f95a849b6e14065cb62c2910db40d1b47 
>   ql/src/test/results/clientpositive/llap/resourceplan.q.out 7f3e784457f87ff88c5f746a9fc2b74ea28c2a6e 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 6e0da5781ed8d536852613bad1793d27dd17dfd4 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java fc254c6f5338ebcfe5a19bea847ff8141552f15f 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 573ac0173dd51b81cb37902cc144983393ab1f51 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java 2c92bb20561d09998e400b458036f8e4a960a469 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java 75fbfa23d25c589826a44be493eb84f3cd375e5e 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java da518ab6e337cbd5befb9f7c1df1781335bd72f4 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 1085ce566a5fba632ae2acb074ce538143bac6ed 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java 24c59f2f1b0b017cff3b26f86c3f903b498d2553 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java 1e4fe5d973542d6b0e3ae0760ca5c06ea9888089 
> 
> 
> Diff: https://reviews.apache.org/r/64555/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 64555: HIVE-18230 create plan like plan, and replace plan commands for easy modification

Posted by Harish Jaiprakash <hj...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64555/#review193774
-----------------------------------------------------------



Small nits.


ql/src/java/org/apache/hadoop/hive/ql/parse/ResourcePlanParser.g
Line 62 (original), 63 (patched)
<https://reviews.apache.org/r/64555/#comment272540>

    Nit: Can we use another token instead of re-using TOK_LIKETABLE?



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 9562 (patched)
<https://reviews.apache.org/r/64555/#comment272423>

    Extract into another private method? Just for readablility.



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 9857 (patched)
<https://reviews.apache.org/r/64555/#comment272427>

    Extra: replacedPlan =



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 9869 (patched)
<https://reviews.apache.org/r/64555/#comment272541>

    Hmm... if people use replace plan frequently as a way of updating an existing plan then this list will grow long and we'll be making one call to db per rp. Would using a like query to get all the names and using a set to store the name make it faster?
    
    Another way would be to use as version field used in conjunction with name to create a unique field. Since we want to implement version sometime this might be useful.


- Harish Jaiprakash


On Dec. 13, 2017, 3:16 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64555/
> -----------------------------------------------------------
> 
> (Updated Dec. 13, 2017, 3:16 a.m.)
> 
> 
> Review request for hive, Harish Jaiprakash and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java 62c9172ef5d9ae74b158d1b4f1b8c5c0eca4e375 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 55ef8de9a5c7144931d0a6ff13224765ee737fea 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 50bdce89a44a8dc87a97e394d00e5dadebbbd351 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java a09b7961c2dbc26b4d2fa912d0be7037885f63e4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 1dcfe9dd2834ada2f2bb75bc67e436017c21f71f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 13789502c235be478659f9267b107e8580594bc8 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g f1ca30109fc40b452f0e1423a586818bcb783796 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ResourcePlanParser.g 95c87259c5bdb91a4533d7897d5d32a7c4f56503 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterResourcePlanDesc.java d70d52054dbef5ecbe5be55fd24df25c5381c8d7 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/CreateResourcePlanDesc.java efdd05c5f270e26b84a0e022eac0445ff368969b 
>   ql/src/test/queries/clientpositive/resourceplan.q fc924a2f95a849b6e14065cb62c2910db40d1b47 
>   ql/src/test/results/clientpositive/llap/resourceplan.q.out 7f3e784457f87ff88c5f746a9fc2b74ea28c2a6e 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 6e0da5781ed8d536852613bad1793d27dd17dfd4 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java fc254c6f5338ebcfe5a19bea847ff8141552f15f 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 573ac0173dd51b81cb37902cc144983393ab1f51 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java 2c92bb20561d09998e400b458036f8e4a960a469 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java 75fbfa23d25c589826a44be493eb84f3cd375e5e 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java da518ab6e337cbd5befb9f7c1df1781335bd72f4 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 1085ce566a5fba632ae2acb074ce538143bac6ed 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java 24c59f2f1b0b017cff3b26f86c3f903b498d2553 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java 1e4fe5d973542d6b0e3ae0760ca5c06ea9888089 
> 
> 
> Diff: https://reviews.apache.org/r/64555/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>