Re: Review Request 71811: Extract Compiler from Driver

[Miklos Gergely]

> On Nov. 28, 2019, 12:39 p.m., Zoltan Haindrich wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/Compiler.java
> > Lines 165 (patched)
> > 
> >
> > I think DriverState should live at the "Driver" level; and not get 
> > mixed into other classes (followup?)

Unfortunately the failure of some tests showed me that driverState must be in 
Compiler too. Compiler is a step of the driver, and during that step some state 
modifications are happening, thus it should be sent to the compiler to mark 
them.


- Miklos


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


On Dec. 1, 2019, 7:12 a.m., Miklos Gergely wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71811/
> ---
> 
> (Updated Dec. 1, 2019, 7:12 a.m.)
> 
> 
> Review request for hive and Zoltan Haindrich.
> 
> 
> Bugs: HIVE-22526
> https://issues.apache.org/jira/browse/HIVE-22526
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> The Driver class contains ~600 lines of code responsible for compiling the 
> command. That means that from the command String a Plan needs to be created, 
> and also a transaction needs to be started (in most of the cases). This is a 
> thing done by the compile function, which has a lot of sub functions to help 
> this task, while itself is also really big. All these codes should be put 
> into a separate class, where it can do it's job without getting mixed with 
> the other codes in the Driver.
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Compiler.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 54d12baa96 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java 1afcfc8969 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverUtils.java 26e904af0b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java 4d79ebc933 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java fa6c9d03ec 
> 
> 
> Diff: https://reviews.apache.org/r/71811/diff/4/
> 
> 
> Testing
> ---
> 
> All the tests are still running fine.
> 
> 
> Thanks,
> 
> Miklos Gergely
> 
>

Re: Review Request 71811: Extract Compiler from Driver

[Miklos Gergely]

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

(Updated Dec. 1, 2019, 7:12 a.m.)


Review request for hive and Zoltan Haindrich.


Bugs: HIVE-22526
https://issues.apache.org/jira/browse/HIVE-22526


Repository: hive-git


Description
---

The Driver class contains ~600 lines of code responsible for compiling the 
command. That means that from the command String a Plan needs to be created, 
and also a transaction needs to be started (in most of the cases). This is a 
thing done by the compile function, which has a lot of sub functions to help 
this task, while itself is also really big. All these codes should be put into 
a separate class, where it can do it's job without getting mixed with the other 
codes in the Driver.


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/Compiler.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 54d12baa96 
  ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java 1afcfc8969 
  ql/src/java/org/apache/hadoop/hive/ql/DriverUtils.java 26e904af0b 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java 4d79ebc933 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java fa6c9d03ec 


Diff: https://reviews.apache.org/r/71811/diff/4/

Changes: https://reviews.apache.org/r/71811/diff/3-4/


Testing
---

All the tests are still running fine.


Thanks,

Miklos Gergely

[RESULT][VOTE] Should we release Hive Storage API 2.7.1rc0?

[Owen O'Malley]

With three +1’s and no -1’s the release vote passes.

Thanks, Alan & Jesus for voting.

.. Owen

> On Nov 27, 2019, at 12:36 PM, Alan Gates  wrote:
> 
> +1.  Did a build, check the signature and hash, ran rat.  At some point we
> should update the copyright in the NOTICE file, it still says 2017.
> 
> Alan.
> 
> On Tue, Nov 26, 2019 at 11:57 PM Jesus Camacho Rodriguez <
> jcama...@apache.org> wrote:
> 
>> +1
>> 
>> Downloaded tar, built storage-api and ran test suite, ran rat check, and
>> verified checksum and signature.
>> 
>> Thanks Owen!
>> 
>> On Tue, Nov 26, 2019 at 4:39 PM Owen O'Malley 
>> wrote:
>> 
>>> All,
>>> I'd like to make a storage-api release with HIVE-22405
>>>  in it.
>>> 
>>> Should we release the following artifacts as Hive Storage API 2.7.1?
>>> 
>>> tar: http://home.apache.org/~omalley/storage-api-2.7.1/
>>> tag:
>> https://github.com/apache/hive/releases/tag/storage-release-2.7.1rc0
>>> jiras: https://issues.apache.org/jira/projects/HIVE/versions/12346553
>>> 
>>> Thanks!
>>> 
>> 

Re: Review Request 71811: Extract Compiler from Driver

[Miklos Gergely]

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

(Updated Nov. 30, 2019, 10:54 a.m.)


Review request for hive and Zoltan Haindrich.


Bugs: HIVE-22526
https://issues.apache.org/jira/browse/HIVE-22526


Repository: hive-git


Description
---

The Driver class contains ~600 lines of code responsible for compiling the 
command. That means that from the command String a Plan needs to be created, 
and also a transaction needs to be started (in most of the cases). This is a 
thing done by the compile function, which has a lot of sub functions to help 
this task, while itself is also really big. All these codes should be put into 
a separate class, where it can do it's job without getting mixed with the other 
codes in the Driver.


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/Compiler.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 54d12baa96 
  ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java 1afcfc8969 
  ql/src/java/org/apache/hadoop/hive/ql/DriverUtils.java 26e904af0b 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java 4d79ebc933 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java fa6c9d03ec 


Diff: https://reviews.apache.org/r/71811/diff/3/

Changes: https://reviews.apache.org/r/71811/diff/2-3/


Testing
---

All the tests are still running fine.


Thanks,

Miklos Gergely