from:"Karen Coppage via Review Board"

Re: Review Request 72532: HIVE-23495 AcidUtils.getAcidState cleanup

2020-06-10 Thread Karen Coppage via Review Board


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


Ship it!




Ship It!

- Karen Coppage


On June 8, 2020, 10:58 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72532/
> ---
> 
> (Updated June 8, 2020, 10:58 a.m.)
> 
> 
> Review request for hive, Karen Coppage, Marta Kuczora, and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> since HIVE-21225 there are two redundant implementation of the 
> AcidUtils.getAcidState.
> 
> The previous implementation (without the recursive listing) can be removed.
> 
> Also the performance can be improved, by removing unnecessary fileStatus 
> calls.
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 635ed3149c 
>   ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java ca234cfb37 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 1059cb227f 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRawRecordMerger.java 
> 16c915959c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/orc/VectorizedOrcAcidRowBatchReader.java
>  598220b0c4 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 2a15913f9f 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 
> 4e5d5b003b 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 7913295380 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MinorQueryCompactor.java 
> d83a50f555 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMajorQueryCompactor.java
>  5e11d8d2d8 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMinorQueryCompactor.java
>  1bdec7df2d 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java 75941b3f33 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 337f469d1a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/TestAcidUtils.java f351f04b08 
>   ql/src/test/org/apache/hadoop/hive/ql/io/orc/TestInputOutputFormat.java 
> e4440e9136 
>   ql/src/test/org/apache/hadoop/hive/ql/io/orc/TestOrcRawRecordMerger.java 
> f63c40a7b5 
>   streaming/src/test/org/apache/hive/streaming/TestStreaming.java 3a3b267927 
> 
> 
> Diff: https://reviews.apache.org/r/72532/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Peter Varga
> 
>

Re: Review Request 72532: HIVE-23495 AcidUtils.getAcidState cleanup

2020-06-10 Thread Karen Coppage via Review Board



> On June 5, 2020, 2:04 p.m., Karen Coppage wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
> > Line 1411 (original)
> > 
> >
> > Are these originals not needed, or collected elsewhere?
> 
> Peter Varga wrote:
> This one is bothering me, these lines were added when the snapshot way 
> was introduced, but I do not see why. When we calculated the AcidState 
> without the snapshot these files were not added to the originals list. It is 
> explicitly there few lines above, that if we have a base we consider every 
> original files as obsolete. The 
> TestTxnCommandsForMmTable#testInsertOverwriteForPartitionedMmTable breaks for 
> example if these files are added to the list. After an insert-overwrite to a 
> mm table and calling the major compaction, the compaction will create a new 
> base dir, not leaving the perfectly fine base dir generated by the insert 
> overwrite. I did not dig into the compaction to see why the original files 
> are triggering it, but I do not think these files needed in the original list.

Ok. I see that files in a base, even if in original format, should be 
considered base files and not original files. It's still not clear to me why 
this block was added but to me it makes sense to get rid of it.


- Karen


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


On June 8, 2020, 10:58 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72532/
> ---
> 
> (Updated June 8, 2020, 10:58 a.m.)
> 
> 
> Review request for hive, Karen Coppage, Marta Kuczora, and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> since HIVE-21225 there are two redundant implementation of the 
> AcidUtils.getAcidState.
> 
> The previous implementation (without the recursive listing) can be removed.
> 
> Also the performance can be improved, by removing unnecessary fileStatus 
> calls.
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 635ed3149c 
>   ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java ca234cfb37 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 1059cb227f 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRawRecordMerger.java 
> 16c915959c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/orc/VectorizedOrcAcidRowBatchReader.java
>  598220b0c4 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 2a15913f9f 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 
> 4e5d5b003b 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 7913295380 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MinorQueryCompactor.java 
> d83a50f555 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMajorQueryCompactor.java
>  5e11d8d2d8 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMinorQueryCompactor.java
>  1bdec7df2d 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java 75941b3f33 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 337f469d1a 
>   ql/src/test/org/apache/hadoop/hive/ql/io/TestAcidUtils.java f351f04b08 
>   ql/src/test/org/apache/hadoop/hive/ql/io/orc/TestInputOutputFormat.java 
> e4440e9136 
>   ql/src/test/org/apache/hadoop/hive/ql/io/orc/TestOrcRawRecordMerger.java 
> f63c40a7b5 
>   streaming/src/test/org/apache/hive/streaming/TestStreaming.java 3a3b267927 
> 
> 
> Diff: https://reviews.apache.org/r/72532/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Peter Varga
> 
>

Re: Review Request 72532: HIVE-23495 AcidUtils.getAcidState cleanup

2020-06-05 Thread Karen Coppage via Review Board


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



LGTM, a few minor suggestions.
(Non-binding)


ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
Line 1411 (original)


Are these originals not needed, or collected elsewhere?



ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
Lines 1438 (patched)


might want to add a check + error message before the while loop in case 
someone added a file that has no delta or base dir in its path. you never know 
what people are capable of :)



ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
Lines 1441 (patched)


nit: typo



ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
Lines 1442 (patched)


nit: typo



ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
Line 1937 (original), 1880 (patched)


Does this need to be public?


- Karen Coppage


On June 4, 2020, 7:35 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72532/
> ---
> 
> (Updated June 4, 2020, 7:35 a.m.)
> 
> 
> Review request for hive, Karen Coppage, Marta Kuczora, and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> since HIVE-21225 there are two redundant implementation of the 
> AcidUtils.getAcidState.
> 
> The previous implementation (without the recursive listing) can be removed.
> 
> Also the performance can be improved, by removing unnecessary fileStatus 
> calls.
> 
> 
> Diffs
> -
> 
>   
> hcatalog/streaming/src/test/org/apache/hive/hcatalog/streaming/TestStreaming.java
>  569de706df 
>   
> hcatalog/streaming/src/test/org/apache/hive/hcatalog/streaming/mutate/StreamingAssert.java
>  86f762e97c 
>   ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java bf332bc0b8 
>   ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java ca234cfb37 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 1059cb227f 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRawRecordMerger.java 
> 16c915959c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/orc/VectorizedOrcAcidRowBatchReader.java
>  598220b0c4 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 5fa3d9ad42 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 
> 018c73376f 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> fa2ede3738 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MinorQueryCompactor.java 
> d83a50f555 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMajorQueryCompactor.java
>  5e11d8d2d8 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMinorQueryCompactor.java
>  1bdec7df2d 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java a96cf1e731 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 366282a30f 
>   ql/src/test/org/apache/hadoop/hive/ql/io/TestAcidUtils.java 9e6d47ebc5 
>   ql/src/test/org/apache/hadoop/hive/ql/io/orc/TestInputOutputFormat.java 
> 12a15a16eb 
>   ql/src/test/org/apache/hadoop/hive/ql/io/orc/TestOrcRawRecordMerger.java 
> f63c40a7b5 
>   streaming/src/test/org/apache/hive/streaming/TestStreaming.java 6101caac66 
> 
> 
> Diff: https://reviews.apache.org/r/72532/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Peter Varga
> 
>

Re: Review Request 72553: HIVE-23555 Cancel compaction jobs when hive.compactor.worker.timeout is reached

2020-05-28 Thread Karen Coppage via Review Board


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



LGTM, one nit:)


ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java
Lines 374 (patched)


Naming the thread masterThread.getName() + "_timeout_executor" would be 
more descriptive.


- Karen Coppage


On May 28, 2020, 8:58 a.m., Peter Vary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72553/
> ---
> 
> (Updated May 28, 2020, 8:58 a.m.)
> 
> 
> Review request for hive, Karen Coppage and Laszlo Pinter.
> 
> 
> Bugs: HIVE-23555
> https://issues.apache.org/jira/browse/HIVE-23555
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Run the actual execution in a new thread, and use Future.get with timeout
> 
> 
> Diffs
> -
> 
>   
> hcatalog/streaming/src/test/org/apache/hive/hcatalog/streaming/TestStreaming.java
>  569de706df 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorTestUtil.java
>  e70d8783bc 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java
>  32fe535b2b 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUpdaterThread.java 
> ecaad509ed 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 5fa3d9ad42 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorThread.java 
> b378d40964 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> fa2ede3738 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MetaStoreCompactorThread.java
>  aa258b331f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/RemoteCompactorThread.java
>  4235184fec 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java 8180adcd66 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 366282a30f 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 3ff68a3c7e 
>   ql/src/test/org/apache/hadoop/hive/ql/stats/TestStatsUpdaterThread.java 
> 84827d1604 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/CompactorTest.java 
> 9a9ab53fcc 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestWorker.java 
> 443f982d66 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  e20fdaf03d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreThread.java
>  ea6155200c 
>   streaming/src/test/org/apache/hive/streaming/TestStreaming.java 6101caac66 
> 
> 
> Diff: https://reviews.apache.org/r/72553/diff/2/
> 
> 
> Testing
> ---
> 
> Created unit tests to check the timeout functionality.
> 
> 
> Thanks,
> 
> Peter Vary
> 
>

Re: Review Request 72281: HIVE-22971: Eliminate file rename in insert-only compactor

2020-05-19 Thread Karen Coppage via Review Board



> On May 18, 2020, 12:51 p.m., Peter Vary wrote:
> > Minor comments only.
> > Thanks for the patch!

Thanks for the review!!


> On May 18, 2020, 12:51 p.m., Peter Vary wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java
> > Lines 305 (patched)
> > 
> >
> > Migth want to add asserts here to check non-null argument
> 
> Karen Coppage wrote:
> I think if the StorageDescriptor is null, a NPE *should* be thrown 
> because that would be a huge problem, "non-null" is in the JavaDoc contract, 
> this method is used once, and will make  it a private method in the next 
> patch.
> 
> Peter Vary wrote:
> Since it become private, it is even better! :)
> Normally for public methods I perfer using, so if someone tries to use 
> this then easier to identify what went wrong. Like this:
> ```
> assert sd != null : "Non-null sd is required"
> ```

I see. Thanks!


- Karen


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


On May 19, 2020, 5:58 a.m., Karen Coppage wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72281/
> ---
> 
> (Updated May 19, 2020, 5:58 a.m.)
> 
> 
> Review request for hive, Laszlo Pinter and Peter Vary.
> 
> 
> Bugs: HIVE-22971
> https://issues.apache.org/jira/browse/HIVE-22971
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> File rename is expensive for object stores, so MM (insert-only) compaction 
> should skip that step when committing and write directly to base_x_cZ or 
> delta_x_y_cZ.
> 
> This also fixes the issue that for MM QB compaction the temp tables were 
> stored under the table directory, and these temp dirs were never cleaned up.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java f5ad3a882b7 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java
>  b9db1d1bb98 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java
>  89920ccebf4 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 
> 9410a963518 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MajorQueryCompactor.java 
> c70d4f33a80 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MinorQueryCompactor.java 
> 4d0e5f703e7 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMajorQueryCompactor.java
>  724a4375b75 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMinorQueryCompactor.java
>  1cd95f80155 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactor.java 
> 7f3ccfa04ed 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactorFactory.java
>  6542eef58af 
> 
> 
> Diff: https://reviews.apache.org/r/72281/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Karen Coppage
> 
>

Re: Review Request 72281: HIVE-22971: Eliminate file rename in insert-only compactor

2020-05-18 Thread Karen Coppage via Review Board


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

(Updated May 19, 2020, 5:58 a.m.)


Review request for hive, Laszlo Pinter and Peter Vary.


Changes
---

Patch 6


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


Repository: hive-git


Description
---

File rename is expensive for object stores, so MM (insert-only) compaction 
should skip that step when committing and write directly to base_x_cZ or 
delta_x_y_cZ.

This also fixes the issue that for MM QB compaction the temp tables were stored 
under the table directory, and these temp dirs were never cleaned up.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java f5ad3a882b7 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java
 b9db1d1bb98 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java
 89920ccebf4 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 
9410a963518 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MajorQueryCompactor.java 
c70d4f33a80 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MinorQueryCompactor.java 
4d0e5f703e7 
  
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMajorQueryCompactor.java 
724a4375b75 
  
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMinorQueryCompactor.java 
1cd95f80155 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactor.java 
7f3ccfa04ed 
  
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactorFactory.java 
6542eef58af 


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

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


Testing
---


Thanks,

Karen Coppage

Re: Review Request 72281: HIVE-22971: Eliminate file rename in insert-only compactor

2020-05-18 Thread Karen Coppage via Review Board



> On May 18, 2020, 12:51 p.m., Peter Vary wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java
> > Lines 305 (patched)
> > 
> >
> > Migth want to add asserts here to check non-null argument

I think if the StorageDescriptor is null, a NPE *should* be thrown because that 
would be a huge problem, "non-null" is in the JavaDoc contract, this method is 
used once, and will make  it a private method in the next patch.


- Karen


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


On March 30, 2020, 6:18 a.m., Karen Coppage wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72281/
> ---
> 
> (Updated March 30, 2020, 6:18 a.m.)
> 
> 
> Review request for hive and Laszlo Pinter.
> 
> 
> Bugs: HIVE-22971
> https://issues.apache.org/jira/browse/HIVE-22971
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> File rename is expensive for object stores, so MM (insert-only) compaction 
> should skip that step when committing and write directly to base_x_cZ or 
> delta_x_y_cZ.
> 
> This also fixes the issue that for MM QB compaction the temp tables were 
> stored under the table directory, and these temp dirs were never cleaned up.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 34df01e60e 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java
>  95fa6641f2 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java
>  9659a3f048 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 
> 543ec0b991 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MajorQueryCompactor.java 
> f47c23a6de 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MinorQueryCompactor.java 
> 1bf0beea40 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMajorQueryCompactor.java
>  114b6f7a74 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMinorQueryCompactor.java
>  383891bfad 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactor.java 
> 7f3ccfa04e 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactorFactory.java
>  6542eef58a 
> 
> 
> Diff: https://reviews.apache.org/r/72281/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Karen Coppage
> 
>

Re: Review Request 72444: HIVE-23280: Trigger compaction with old aborted txns

2020-04-28 Thread Karen Coppage via Review Board


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

(Updated April 28, 2020, 10:37 a.m.)


Review request for hive, Laszlo Pinter and Peter Vary.


Changes
---

Patch 4, addressed review comments


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


Repository: hive-git


Description
---

When a txn is aborted and the compaction threshold for number of aborted txns 
is not reached then the aborted transaction can remain forever in the RDBMS 
database. This could result in several serious performance degradations:

getOpenTxns has to list this aborted txn forever
TXN_TO_WRITE_ID table is not cleaned
We should add a threshold, so after a given time the compaction is started 
anyway.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e3ddbf197b 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 37a5862791 
  
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java 
15fcfc0e35 
  ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
1151466f8c 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java
 31b6ed450b 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php
 9fb7ff011a 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py
 4f317b3453 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb
 e64ae0ead2 
  standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift 
1e3d6e9b8b 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionInfo.java
 70d63ab18b 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
 2344c2d5f6 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
 87130a519d 


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

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


Testing
---


Thanks,

Karen Coppage

Re: Review Request 72444: HIVE-23280: Trigger compaction with old aborted txns

2020-04-28 Thread Karen Coppage via Review Board



> On April 28, 2020, 8:54 a.m., Peter Vary wrote:
> > Thanks for the patch Karen!
> > Few questions below.
> > 
> > Thanks,
> > Peter

Thanks for the review, Peter! Questions are addressed below.


> On April 28, 2020, 8:54 a.m., Peter Vary wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
> > Lines 2852 (patched)
> > 
> >
> > Can we add a comment about valid values, and how to turn this off?

Done


> On April 28, 2020, 8:54 a.m., Peter Vary wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java
> > Lines 92 (patched)
> > 
> >
> > Can we check the valid values, and can we turn this function off?

We can turn it off by setting to a negative number, which used to be checked at 
org.apache.hadoop.hive.metastore.txn.CompactionTxnHandler#isPastAbortedTimeThreshold
 and is now checked before the query to metastore is executed.
Valid values are checked by org.apache.hadoop.hive.conf.Validator.TimeValidator.


> On April 28, 2020, 8:54 a.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
> > Lines 113 (patched)
> > 
> >
> > Do we know for sure, that for null partition that this query is working 
> > for all of the supported databases?

Group by partition was already present, I just removed a conditional clause and 
added 2 columns to the projection without touching partition handling. Do you 
think it's worth testing in each supported db even for these changes?


> On April 28, 2020, 8:54 a.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
> > Lines 151 (patched)
> > 
> >
> > Why Instant? I usually use System.currentTimeMillis(). Also it might be 
> > worth to get it only once, and not for every compactionInfo

Ok, will do. (I thought wrongly that System.currentTimeMillis was somehow 
timezone dependendent)


- Karen


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


On April 28, 2020, 8:39 a.m., Karen Coppage wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72444/
> ---
> 
> (Updated April 28, 2020, 8:39 a.m.)
> 
> 
> Review request for hive, Laszlo Pinter and Peter Vary.
> 
> 
> Bugs: HIVE-23280
> https://issues.apache.org/jira/browse/HIVE-23280
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> When a txn is aborted and the compaction threshold for number of aborted txns 
> is not reached then the aborted transaction can remain forever in the RDBMS 
> database. This could result in several serious performance degradations:
> 
> getOpenTxns has to list this aborted txn forever
> TXN_TO_WRITE_ID table is not cleaned
> We should add a threshold, so after a given time the compaction is started 
> anyway.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e3ddbf197b 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java
>  31b6ed450b 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php
>  9fb7ff011a 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py
>  4f317b3453 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb
>  e64ae0ead2 
>   standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift 
> 1e3d6e9b8b 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionInfo.java
>  70d63ab18b 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
> 
> 
> Diff: https://reviews.apache.org/r/72444/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Karen Coppage
> 
>

Re: Review Request 72444: HIVE-23280: Trigger compaction with old aborted txns

2020-04-28 Thread Karen Coppage via Review Board


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

(Updated April 28, 2020, 8:39 a.m.)


Review request for hive, Laszlo Pinter and Peter Vary.


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


Repository: hive-git


Description
---

When a txn is aborted and the compaction threshold for number of aborted txns 
is not reached then the aborted transaction can remain forever in the RDBMS 
database. This could result in several serious performance degradations:

getOpenTxns has to list this aborted txn forever
TXN_TO_WRITE_ID table is not cleaned
We should add a threshold, so after a given time the compaction is started 
anyway.


Diffs
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e3ddbf197b 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 37a5862791 
  
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java 
15fcfc0e35 
  ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
1151466f8c 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java
 31b6ed450b 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php
 9fb7ff011a 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py
 4f317b3453 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb
 e64ae0ead2 
  standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift 
1e3d6e9b8b 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionInfo.java
 70d63ab18b 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
 2344c2d5f6 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
 87130a519d 


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


Testing
---


Thanks,

Karen Coppage

Review Request 72281: HIVE-22971: Eliminate file rename in insert-only compactor

2020-03-30 Thread Karen Coppage via Review Board


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

Review request for hive and Laszlo Pinter.


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


Repository: hive-git


Description
---

File rename is expensive for object stores, so MM (insert-only) compaction 
should skip that step when committing and write directly to base_x_cZ or 
delta_x_y_cZ.

This also fixes the issue that for MM QB compaction the temp tables were stored 
under the table directory, and these temp dirs were never cleaned up.


Diffs
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 34df01e60e 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java
 95fa6641f2 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java
 9659a3f048 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 
543ec0b991 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MajorQueryCompactor.java 
f47c23a6de 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MinorQueryCompactor.java 
1bf0beea40 
  
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMajorQueryCompactor.java 
114b6f7a74 
  
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMinorQueryCompactor.java 
383891bfad 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactor.java 
7f3ccfa04e 
  
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactorFactory.java 
6542eef58a 


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


Testing
---


Thanks,

Karen Coppage

Re: Review Request 71812: HIVE-22534: ACID: Improve Compactor thread logging

2020-03-27 Thread Karen Coppage via Review Board


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



LGTM, one suggestion below.
Also Zoltan Chovan seemed excited about this change, maybe consider asking him 
if this fulfills his logging wishes :)


ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java
Lines 374-375 (original), 360-361 (patched)


Not regression, but it looks like these arguments are switched up:
deltaNumThreshold
noBase ? "without" : "with"


- Karen Coppage


On March 26, 2020, 2:43 p.m., Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71812/
> ---
> 
> (Updated March 26, 2020, 2:43 p.m.)
> 
> 
> Review request for hive and Karen Coppage.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-22534: ACID: Improve Compactor thread logging
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 
> 54b616e60c73fa1005c6d679ea76d65e01a0749d 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 
> 543ec0b99124cb38c8508aa2ec2f99cababdbdbd 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a58627913c33be37cee7f11d9ca4ee5fd8aff2 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MinorQueryCompactor.java 
> 1bf0beea4022dbabe65a9d2ee0972186b5cb3396 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMinorQueryCompactor.java
>  383891bfadfc1352430e50eac2d60366ad699bf0 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java 
> a96cf1e73183232d587b3f4937c0d7e96764e662 
> 
> 
> Diff: https://reviews.apache.org/r/71812/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Laszlo Pinter
> 
>

Re: Review Request 72193: HIVE-22977: Merge delta files instead of running a query in major/minor compaction

2020-03-05 Thread Karen Coppage via Review Board


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



How feasible would it be to launch this process from CompactorMR instead of the 
QueryCompactor implementations, and fall back to QueryCompactors if it fails? 
Because it's not exactly a "query compaction" but more of a 
"FileMergeCompaction"...


ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/OrcFileMerger.java
Lines 39 (patched)


Should this class be public?



ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/OrcFileMerger.java
Lines 62-64 (patched)


If the readers aren't compatible, compaction fails silently? Consider 
returning a boolean or throwing an exception and falling back to query-based 
compaction.



ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/OrcFileMerger.java
Lines 67 (patched)


Does the Reader need to be closed as well? Not sure



ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/OrcFileMerger.java
Lines 76 (patched)


nit: if nextBatch(batch) is true, will batch ever be null?
Idk, at the end of the day I guess it's good to be on the safe side:)



ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/OrcFileMerger.java
Lines 104 (patched)


Output path should be logged once, not every time setupWriter is called, 
unless outPath's value changes somewhere...



ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactor.java
Lines 220 (patched)


Since there's no need to check for delete dirs for insert-only tables, 
maybe 
(a) skip the delete dir check in this function or
(b) split this function into 2: hasDeletes and hasAborted



ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactor.java
Lines 227 (patched)


Wouldn't it be more efficient to check
!directory.getAbortedDirectories().isEmpty()
at the very beginning of this function, before starting all the streaming 
and filtering?



ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactor.java
Lines 293 (patched)


Idea:
Could use this in 
org.apache.hadoop.hive.ql.txn.compactor.MinorQueryCompactor#getCreateQueries
for delta creation. Or possibly for both deltas and delete deltas, if you 
added a param.



ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactor.java
Lines 314-315 (patched)


Consider debug/info log message about merge starting



ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactor.java
Lines 318 (patched)


nit: rename e to something meaningful like listOfDeltaPaths


- Karen Coppage


On March 4, 2020, 3:31 p.m., Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72193/
> ---
> 
> (Updated March 4, 2020, 3:31 p.m.)
> 
> 
> Review request for hive, Karen Coppage and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-22977: Merge delta files instead of running a query in major/minor 
> compaction
> 
> 
> Diffs
> -
> 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorOnTezTest.java
>  78174f345b35709cd654aa81578ab598e0d9ed9c 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java
>  9659a3f0481dcb2446b197688459f0c1dba867fa 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestMmCompactorOnTez.java
>  074430ce7fa0f0617e8fb50c334c14f33cc74d8a 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 
> c44f2b5026558c9f0a7d6fa03cb6950f24b77da2 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MajorQueryCompactor.java 
> 93850807137a4cfbd49beb256624b11801bd08d1 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MinorQueryCompactor.java 
> 01cd2fc93d12002249253added06df70b0c40181 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMajorQueryCompactor.java
>  41fdd7e210bfc42c3e41e9f1240d34a51add33a9 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMinorQueryCompactor.java
>  feb667cba960c0fdd19c030235eb31ebddfa7ca1 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/OrcFileMerger.java 
> PRE-CREATION 
>

Re: Review Request 72145: HIVE-21543: Use FilterHooks for show compactions

2020-02-17 Thread Karen Coppage via Review Board


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


Ship it!




Ship It!

- Karen Coppage


On Feb. 17, 2020, 3:30 p.m., Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72145/
> ---
> 
> (Updated Feb. 17, 2020, 3:30 p.m.)
> 
> 
> Review request for hive, Karen Coppage and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-21543: Use FilterHooks for show compactions
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  c7e1044589fd1dd970b86259b713d3a44716e7b8 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
>  d9da00dd2148b0408548ab7d5c88df014a7f7826 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  b6de1460a565f06217f163ca5f733594a0c8406a 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
>  23faa7444a8e9f9c010290539f89b9d8b44f3aa8 
> 
> 
> Diff: https://reviews.apache.org/r/72145/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Laszlo Pinter
> 
>

Re: Review Request 72145: HIVE-21543: Use FilterHooks for show compactions

2020-02-17 Thread Karen Coppage via Review Board


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




standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
Lines 403-405 (patched)


nit: My friendly IDE tells me this can be replaced with a 
Map.computeIfAbsent call. Also true for lines 410, 413.



standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
Lines 440-442 (patched)


I think these are unnecessary



standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
Lines 404 (patched)


Parameter isn't used


- Karen Coppage


On Feb. 17, 2020, 9:42 a.m., Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72145/
> ---
> 
> (Updated Feb. 17, 2020, 9:42 a.m.)
> 
> 
> Review request for hive, Karen Coppage and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-21543: Use FilterHooks for show compactions
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  c7e1044589fd1dd970b86259b713d3a44716e7b8 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
>  d9da00dd2148b0408548ab7d5c88df014a7f7826 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  7b7c2d77914ad56d3ae568aa050725b12969895a 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
>  23faa7444a8e9f9c010290539f89b9d8b44f3aa8 
> 
> 
> Diff: https://reviews.apache.org/r/72145/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Laszlo Pinter
> 
>

Re: Review Request 72105: HIVE-22610: Minor compaction for MM (insert-only) tables

2020-02-12 Thread Karen Coppage via Review Board


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

(Updated Feb. 12, 2020, 3:23 p.m.)


Review request for hive and Laszlo Pinter.


Changes
---

Patch 04


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


Repository: hive-git


Description
---

Minor compaction for MM (insert-only) tables


Diffs (updated)
-

  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorOnTezTest.java
 PRE-CREATION 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java
 4c01311 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestMmCompactorOnTez.java
 PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java bb70db4 
  
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMajorQueryCompactor.java 
bad5d00 
  
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmMinorQueryCompactor.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MmQueryCompactorUtils.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/QueryCompactorFactory.java 
2f2bb21 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 88ca683 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java aabf15c 


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

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


Testing
---

The 3 unit tests removed here are really 2 tests (one is a duplicate). 
Insert-only minor compaction used to do nothing but remove aborted directories; 
these 2 tests were written for this functionality specifically. The unit tests 
added here cover that functionality as well as compaction, and rendered the old 
unit tests obsolete.


Thanks,

Karen Coppage

Re: Review Request 72063: HIVE-10362: Support Type check/conversion in dynamic partition column

2020-02-07 Thread Karen Coppage via Review Board


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

(Updated Feb. 7, 2020, 8:40 a.m.)


Review request for hive and Peter Vary.


Changes
---

Patch 12


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


Repository: hive-git


Description
---

Example:
create table dynparttypechecknum (key int, value string) partitioned by (part 
int);
insert into dynparttypechecknum partition (part) select key, value, '1' 
from src limit 1;
show partitions dynparttypechecknum;

Partition created will be named:
part=1
even though the type of `part` is int.

Solution is to cast the inserted DP columns in the SelectOperator before 
FileSinkOperator which creates the partition dir, not after.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 12a022c590 
  itests/src/test/resources/testconfiguration.properties 99ca9867b1 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java c2514eedb1 
  ql/src/test/queries/clientpositive/dynpart_cast.q PRE-CREATION 
  ql/src/test/results/clientpositive/autoColumnStats_6.q.out da3be3e5bb 
  ql/src/test/results/clientpositive/dynpart_cast.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/dynpart_sort_optimization_acid2.q.out 
43bb789840 
  ql/src/test/results/clientpositive/infer_bucket_sort_num_buckets.q.out 
f745b46899 
  ql/src/test/results/clientpositive/llap/dynpart_cast.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/dynpart_sort_opt_bucketing.q.out 
453d2451df 
  ql/src/test/results/clientpositive/llap/orc_merge1.q.out 9da73e65ac 
  ql/src/test/results/clientpositive/llap/orc_merge10.q.out a6ea33493f 
  ql/src/test/results/clientpositive/llap/orc_merge2.q.out 9b0d3b4234 
  ql/src/test/results/clientpositive/llap/orc_merge_diff_fs.q.out d35f44b10a 
  ql/src/test/results/clientpositive/llap/rcfile_merge2.q.out fcff20a68e 
  ql/src/test/results/clientpositive/llap/tez_dml.q.out 4ad78d8582 
  ql/src/test/results/clientpositive/orc_merge1.q.out 9c07816340 
  ql/src/test/results/clientpositive/orc_merge10.q.out 4a5f03c82f 
  ql/src/test/results/clientpositive/orc_merge2.q.out d132d62b18 
  ql/src/test/results/clientpositive/orc_merge_diff_fs.q.out 7f9a04b09f 
  ql/src/test/results/clientpositive/smb_join_partition_key.q.out c18d01d26a 
  ql/src/test/results/clientpositive/spark/infer_bucket_sort_num_buckets.q.out 
56d5ed945b 
  ql/src/test/results/clientpositive/spark/orc_merge1.q.out 977c4cbfc1 
  ql/src/test/results/clientpositive/spark/orc_merge2.q.out 4647b86ea3 
  ql/src/test/results/clientpositive/spark/orc_merge_diff_fs.q.out b7d3dd725d 


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

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


Testing
---


Thanks,

Karen Coppage

Re: Review Request 72063: HIVE-10362: Support Type check/conversion in dynamic partition column

2020-02-06 Thread Karen Coppage via Review Board


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

(Updated Feb. 6, 2020, 2:22 p.m.)


Review request for hive and Peter Vary.


Changes
---

Patch 11 - qtest changes, dropped use of DynamicPartitionCtx


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


Repository: hive-git


Description
---

Example:
create table dynparttypechecknum (key int, value string) partitioned by (part 
int);
insert into dynparttypechecknum partition (part) select key, value, '1' 
from src limit 1;
show partitions dynparttypechecknum;

Partition created will be named:
part=1
even though the type of `part` is int.

Solution is to cast the inserted DP columns in the SelectOperator before 
FileSinkOperator which creates the partition dir, not after.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 12a022c590 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java c2514eedb1 
  ql/src/test/queries/clientpositive/dynpart_cast.q PRE-CREATION 
  ql/src/test/results/clientpositive/autoColumnStats_6.q.out da3be3e5bb 
  ql/src/test/results/clientpositive/dynpart_cast.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/dynpart_sort_optimization_acid2.q.out 
43bb789840 
  ql/src/test/results/clientpositive/infer_bucket_sort_num_buckets.q.out 
f745b46899 
  ql/src/test/results/clientpositive/llap/dynpart_sort_opt_bucketing.q.out 
453d2451df 
  ql/src/test/results/clientpositive/llap/orc_merge1.q.out 9da73e65ac 
  ql/src/test/results/clientpositive/llap/orc_merge10.q.out a6ea33493f 
  ql/src/test/results/clientpositive/llap/orc_merge2.q.out 9b0d3b4234 
  ql/src/test/results/clientpositive/llap/orc_merge_diff_fs.q.out d35f44b10a 
  ql/src/test/results/clientpositive/llap/rcfile_merge2.q.out fcff20a68e 
  ql/src/test/results/clientpositive/llap/tez_dml.q.out 4ad78d8582 
  ql/src/test/results/clientpositive/orc_merge1.q.out 9c07816340 
  ql/src/test/results/clientpositive/orc_merge10.q.out 4a5f03c82f 
  ql/src/test/results/clientpositive/orc_merge2.q.out d132d62b18 
  ql/src/test/results/clientpositive/orc_merge_diff_fs.q.out 7f9a04b09f 
  ql/src/test/results/clientpositive/smb_join_partition_key.q.out c18d01d26a 
  ql/src/test/results/clientpositive/spark/infer_bucket_sort_num_buckets.q.out 
56d5ed945b 
  ql/src/test/results/clientpositive/spark/orc_merge1.q.out 977c4cbfc1 
  ql/src/test/results/clientpositive/spark/orc_merge2.q.out 4647b86ea3 
  ql/src/test/results/clientpositive/spark/orc_merge_diff_fs.q.out b7d3dd725d 


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

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


Testing
---

There were changes in query output in two spark auto_sortmerge_join_16.q.out 
files. They now match the query output of llap/auto_sortmerge_join_16.q.out.


Thanks,

Karen Coppage

Re: Review Request 72084: HIVE-21216: Write Parquet INT64 timestamp

2020-02-06 Thread Karen Coppage via Review Board


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

(Updated Feb. 6, 2020, 2:12 p.m.)


Review request for hive and Marta Kuczora.


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


Repository: hive-git


Description
---

This patch enables Hive to start writing int64 timestamps in Parquet.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 12a022c590 
  
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java 
ba235f761d 
  
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveSchemaConverter.java
 21bfb2e1a2 
  
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/ParquetTimestampUtils.java
 9ce07e74f3 
  
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriteSupport.java
 8acde81a3d 
  
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java 
bd519eb66e 
  
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/HiveParquetSchemaTestUtils.java
 181894f106 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestDataWritableWriter.java 
b242392a9a 
  ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestHiveSchemaConverter.java 
dc80af1b76 
  
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/serde/TestParquetTimestampUtils.java
 9ea78508ee 
  ql/src/test/queries/clientpositive/parquet_int64_timestamp.q PRE-CREATION 
  
ql/src/test/queries/clientpositive/parquet_int64_timestamp_int96_compatibility.q
 PRE-CREATION 
  ql/src/test/results/clientpositive/parquet_int64_timestamp.q.out PRE-CREATION 
  
ql/src/test/results/clientpositive/parquet_int64_timestamp_int96_compatibility.q.out
 PRE-CREATION 


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

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


Testing
---


Thanks,

Karen Coppage

Re: Review Request 72063: HIVE-10362: Support Type check/conversion in dynamic partition column

2020-02-05 Thread Karen Coppage via Review Board



> On Feb. 4, 2020, 2:03 p.m., Zoltan Haindrich wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
> > Lines 8475 (patched)
> > 
> >
> > do we really need this much if-s ?
> > 
> > 
> > what do you think about:
> > * throw real exceptions - if appropriate
> > * introduce feature toggle for this case/check and make it enabled by 
> > default - so that there will be a way back to the old behaviour

Good idea, will do.


> On Feb. 4, 2020, 2:03 p.m., Zoltan Haindrich wrote:
> > ql/src/test/results/clientpositive/spark/auto_sortmerge_join_16.q.out_spark
> > Lines 458 (patched)
> > 
> >
> > is this change expected?
> > is the new resultset the correct one?
> > do we have an agreement between spark/llap ?

With this change we have an agreement with llap. I also checked the results and 
llap was correct.


- Karen


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


On Jan. 30, 2020, 3:30 p.m., Karen Coppage wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72063/
> ---
> 
> (Updated Jan. 30, 2020, 3:30 p.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Bugs: HIVE-10362
> https://issues.apache.org/jira/browse/HIVE-10362
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Example:
> create table dynparttypechecknum (key int, value string) partitioned by (part 
> int);
> insert into dynparttypechecknum partition (part) select key, value, '1' 
> from src limit 1;
> show partitions dynparttypechecknum;
> 
> Partition created will be named:
> part=1
> even though the type of `part` is int.
> 
> Solution is to cast the inserted DP columns in the SelectOperator before 
> FileSinkOperator which creates the partition dir, not after.
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 
> 5fcc367cc9 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicPartitionCtx.java 
> c1aeb8f136 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/TestFileSinkOperator.java 
> 2c4b69b2fe 
>   ql/src/test/queries/clientpositive/dynpart_cast.q PRE-CREATION 
>   ql/src/test/results/clientpositive/autoColumnStats_6.q.out da3be3e5bb 
>   ql/src/test/results/clientpositive/dynpart_cast.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/dynpart_sort_optimization_acid2.q.out 
> 43bb789840 
>   ql/src/test/results/clientpositive/infer_bucket_sort_num_buckets.q.out 
> f745b46899 
>   ql/src/test/results/clientpositive/llap/auto_sortmerge_join_16.q.out 
> fc9050b2c3 
>   
> ql/src/test/results/clientpositive/llap/dynpart_sort_optimization_acid.q.out 
> 95aae7286f 
>   ql/src/test/results/clientpositive/llap/llap_smb.q.out 24026d0bab 
>   ql/src/test/results/clientpositive/llap/orc_merge1.q.out 9da73e65ac 
>   ql/src/test/results/clientpositive/llap/orc_merge10.q.out a6ea33493f 
>   ql/src/test/results/clientpositive/llap/orc_merge2.q.out 9b0d3b4234 
>   ql/src/test/results/clientpositive/llap/orc_merge_diff_fs.q.out d35f44b10a 
>   ql/src/test/results/clientpositive/llap/rcfile_merge2.q.out fcff20a68e 
>   ql/src/test/results/clientpositive/llap/tez_dml.q.out 4ad78d8582 
>   ql/src/test/results/clientpositive/orc_merge1.q.out 9c07816340 
>   ql/src/test/results/clientpositive/orc_merge10.q.out 4a5f03c82f 
>   ql/src/test/results/clientpositive/orc_merge2.q.out d132d62b18 
>   ql/src/test/results/clientpositive/orc_merge_diff_fs.q.out 7f9a04b09f 
>   ql/src/test/results/clientpositive/smb_join_partition_key.q.out c18d01d26a 
>   ql/src/test/results/clientpositive/spark/auto_sortmerge_join_16.q.out 
> bc6c3add54 
>   ql/src/test/results/clientpositive/spark/auto_sortmerge_join_16.q.out_spark 
> 67b62c1265 
>   
> ql/src/test/results/clientpositive/spark/infer_bucket_sort_num_buckets.q.out 
> 56d5ed945b 
>   ql/src/test/results/clientpositive/spark/orc_merge1.q.out 977c4cbfc1 
>   ql/src/test/results/clientpositive/spark/orc_merge2.q.out 4647b86ea3 
>   ql/src/test/results/clientpositive/spark/orc_merge_diff_fs.q.out b7d3dd725d 
> 
> 
> Diff: https://reviews.apache.org/r/72063/diff/1/
> 
> 
> Testing
> ---
> 
> There were changes in query output in two spark auto_sortmerge_join_16.q.out 
> files. They now match the query output of llap/auto_sortmerge_join_16.q.out.
> 
> 
> Thanks,
> 
> Karen Coppage
> 
>

Re: Review Request 72074: HIVE-21215: Read Parquet INT64 timestamp

2020-02-03 Thread Karen Coppage via Review Board


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


Ship it!




Ship It!

- Karen Coppage


On Feb. 3, 2020, 12:31 p.m., Marta Kuczora wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72074/
> ---
> 
> (Updated Feb. 3, 2020, 12:31 p.m.)
> 
> 
> Review request for hive, Karen Coppage and Peter Vary.
> 
> 
> Bugs: HIVE-21215
> https://issues.apache.org/jira/browse/HIVE-21215
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Implemented the read path for Parquet INT64 timestamp.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/common/type/Timestamp.java 
> f2c1493f56 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java 
> d67b030648 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/ParquetTimestampUtils.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java
>  519bd813e9 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedPrimitiveColumnReader.java
>  2803baf90c 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/convert/TestETypeConverter.java
>  f6ee57140c 
> 
> 
> Diff: https://reviews.apache.org/r/72074/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marta Kuczora
> 
>

Re: Review Request 72074: HIVE-21215: Read Parquet INT64 timestamp

2020-02-03 Thread Karen Coppage via Review Board


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



Thanks for the patch, looks good! Two ideas:
1. It would be nice to have a unit test that reads a date before October 1582, 
so it's clear that we're using the Proleptic Gregorian calendar.
2. ParquetTimestampUtils would be more readable if the big multipliers were 
declared as constants and/or in this format: e.g. 1_000_000.

Thanks!

- Karen Coppage


On Jan. 31, 2020, 7:23 p.m., Marta Kuczora wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72074/
> ---
> 
> (Updated Jan. 31, 2020, 7:23 p.m.)
> 
> 
> Review request for hive, Karen Coppage and Peter Vary.
> 
> 
> Bugs: HIVE-21215
> https://issues.apache.org/jira/browse/HIVE-21215
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Implemented the read path for Parquet INT64 timestamp.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/common/type/Timestamp.java 
> f2c1493f56 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/ETypeConverter.java 
> d67b030648 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/ParquetTimestampUtils.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java
>  519bd813e9 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedPrimitiveColumnReader.java
>  2803baf90c 
>   
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/convert/TestETypeConverter.java
>  f6ee57140c 
> 
> 
> Diff: https://reviews.apache.org/r/72074/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marta Kuczora
> 
>

Re: Review Request 72063: HIVE-10362: Support Type check/conversion in dynamic partition column

2020-01-30 Thread Karen Coppage via Review Board


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

(Updated Jan. 30, 2020, 3:30 p.m.)


Review request for hive and Peter Vary.


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


Repository: hive-git


Description
---

Example:
create table dynparttypechecknum (key int, value string) partitioned by (part 
int);
insert into dynparttypechecknum partition (part) select key, value, '1' 
from src limit 1;
show partitions dynparttypechecknum;

Partition created will be named:
part=1
even though the type of `part` is int.

Solution is to cast the inserted DP columns in the SelectOperator before 
FileSinkOperator which creates the partition dir, not after.


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5fcc367cc9 
  ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicPartitionCtx.java 
c1aeb8f136 
  ql/src/test/org/apache/hadoop/hive/ql/exec/TestFileSinkOperator.java 
2c4b69b2fe 
  ql/src/test/queries/clientpositive/dynpart_cast.q PRE-CREATION 
  ql/src/test/results/clientpositive/autoColumnStats_6.q.out da3be3e5bb 
  ql/src/test/results/clientpositive/dynpart_cast.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/dynpart_sort_optimization_acid2.q.out 
43bb789840 
  ql/src/test/results/clientpositive/infer_bucket_sort_num_buckets.q.out 
f745b46899 
  ql/src/test/results/clientpositive/llap/auto_sortmerge_join_16.q.out 
fc9050b2c3 
  ql/src/test/results/clientpositive/llap/dynpart_sort_optimization_acid.q.out 
95aae7286f 
  ql/src/test/results/clientpositive/llap/llap_smb.q.out 24026d0bab 
  ql/src/test/results/clientpositive/llap/orc_merge1.q.out 9da73e65ac 
  ql/src/test/results/clientpositive/llap/orc_merge10.q.out a6ea33493f 
  ql/src/test/results/clientpositive/llap/orc_merge2.q.out 9b0d3b4234 
  ql/src/test/results/clientpositive/llap/orc_merge_diff_fs.q.out d35f44b10a 
  ql/src/test/results/clientpositive/llap/rcfile_merge2.q.out fcff20a68e 
  ql/src/test/results/clientpositive/llap/tez_dml.q.out 4ad78d8582 
  ql/src/test/results/clientpositive/orc_merge1.q.out 9c07816340 
  ql/src/test/results/clientpositive/orc_merge10.q.out 4a5f03c82f 
  ql/src/test/results/clientpositive/orc_merge2.q.out d132d62b18 
  ql/src/test/results/clientpositive/orc_merge_diff_fs.q.out 7f9a04b09f 
  ql/src/test/results/clientpositive/smb_join_partition_key.q.out c18d01d26a 
  ql/src/test/results/clientpositive/spark/auto_sortmerge_join_16.q.out 
bc6c3add54 
  ql/src/test/results/clientpositive/spark/auto_sortmerge_join_16.q.out_spark 
67b62c1265 
  ql/src/test/results/clientpositive/spark/infer_bucket_sort_num_buckets.q.out 
56d5ed945b 
  ql/src/test/results/clientpositive/spark/orc_merge1.q.out 977c4cbfc1 
  ql/src/test/results/clientpositive/spark/orc_merge2.q.out 4647b86ea3 
  ql/src/test/results/clientpositive/spark/orc_merge_diff_fs.q.out b7d3dd725d 


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


Testing
---

There were changes in query output in two spark auto_sortmerge_join_16.q.out 
files. They now match the query output of llap/auto_sortmerge_join_16.q.out.


Thanks,

Karen Coppage

Re: Review Request 72028: HIVE-22729: Provide a failure reason for failed compactions

2020-01-29 Thread Karen Coppage via Review Board


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


Ship it!




Ship It!

- Karen Coppage


On Jan. 29, 2020, 9:20 a.m., Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72028/
> ---
> 
> (Updated Jan. 29, 2020, 9:20 a.m.)
> 
> 
> Review request for hive, Denys Kuzmenko, Karen Coppage, and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-22729: Provide a failure reason for failed compactions
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/process/show/compactions/ShowCompactionsDesc.java
>  9348efc5a12b50f55f5952094882e941158405fd 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/process/show/compactions/ShowCompactionsOperation.java
>  517d88237cc3b8f0316727bf1eebfc6535152fae 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 
> 6f642901203ab73699ed694009d48ca77263fb10 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> dedc990d0f1e9123497f0fb7c7b9945c7b29bde2 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java 
> 5aff71e0e981c429f85663300d3e5c21089529a9 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  e5895547e6006f30a37b5ba0b1ce42253129d3b6 
>   ql/src/test/results/clientpositive/dbtxnmgr_showlocks.q.out 
> 03c6724ec2e50ae1f7c642339c1806d0786a9ec5 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java
>  4aee45ce5f0e534823194bc84d13b88210ce0b3c 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ShowCompactResponseElement.java
>  8a5682a013b24f8dcf7ad3fdb0b0b606d82cc7c0 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php
>  e8556dcea68f34336df2925c4108e71185d6377f 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py
>  b05e61e84a310273911ef592258bcd3b34e87734 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb
>  868cf69200f69aa89e82b34e22ee0ad792e6d025 
>   standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift 
> 61a94fee4d82a714c12aeeb27f31e24774592c98 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionInfo.java
>  ba45f3945274853fdc84487d93c4c00ff2982541 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  aded6f5486cc840f397347b39049310009fd3bad 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  da5dd61d08e2ca8fe5e80ffdf9fb4a6f4c4d0ba3 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  c2c97d96c6cc98f9746069fa725d17d12f6c8642 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  67102718867233f29ddb2ea8ec3fbcb6560c6c30 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  ae0a32541a4bb9179b2bb71ae9f9098d7b35a88e 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  221d4f1fffb682aaec3af22a339e7a3077a75f6a 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  bc98d5fc4a5637988c97f2e5a0e02d3be16ae0cb 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  dd761a66db4826580a67d64879e4c85278b8e20c 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  6a040a6a64c2086b5eb68a397697c9e2d2ca4d76 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  f5ec1ba1aff89d02b66d6a2cd1da8de1b3b08d06 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  c7738be2732b839aa2b460733c092e368909f935 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  455f98b72578ff977e29301cd2fc595ae80ee4ca 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  5c39b0d9f4d27ab82ef44392818c1810cb7664ce 
> 
> 
> Diff: https://reviews.apache.org/r/72028/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Laszlo Pinter
> 
>

Re: Review Request 72028: HIVE-22729: Provide a failure reason for failed compactions

2020-01-29 Thread Karen Coppage via Review Board


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



- Karen Coppage


On Jan. 29, 2020, 9:20 a.m., Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72028/
> ---
> 
> (Updated Jan. 29, 2020, 9:20 a.m.)
> 
> 
> Review request for hive, Denys Kuzmenko, Karen Coppage, and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-22729: Provide a failure reason for failed compactions
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/process/show/compactions/ShowCompactionsDesc.java
>  9348efc5a12b50f55f5952094882e941158405fd 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/process/show/compactions/ShowCompactionsOperation.java
>  517d88237cc3b8f0316727bf1eebfc6535152fae 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 
> 6f642901203ab73699ed694009d48ca77263fb10 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> dedc990d0f1e9123497f0fb7c7b9945c7b29bde2 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java 
> 5aff71e0e981c429f85663300d3e5c21089529a9 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  e5895547e6006f30a37b5ba0b1ce42253129d3b6 
>   ql/src/test/results/clientpositive/dbtxnmgr_showlocks.q.out 
> 03c6724ec2e50ae1f7c642339c1806d0786a9ec5 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java
>  4aee45ce5f0e534823194bc84d13b88210ce0b3c 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ShowCompactResponseElement.java
>  8a5682a013b24f8dcf7ad3fdb0b0b606d82cc7c0 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php
>  e8556dcea68f34336df2925c4108e71185d6377f 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py
>  b05e61e84a310273911ef592258bcd3b34e87734 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb
>  868cf69200f69aa89e82b34e22ee0ad792e6d025 
>   standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift 
> 61a94fee4d82a714c12aeeb27f31e24774592c98 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionInfo.java
>  ba45f3945274853fdc84487d93c4c00ff2982541 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  aded6f5486cc840f397347b39049310009fd3bad 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  da5dd61d08e2ca8fe5e80ffdf9fb4a6f4c4d0ba3 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  c2c97d96c6cc98f9746069fa725d17d12f6c8642 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  67102718867233f29ddb2ea8ec3fbcb6560c6c30 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  ae0a32541a4bb9179b2bb71ae9f9098d7b35a88e 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  221d4f1fffb682aaec3af22a339e7a3077a75f6a 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  bc98d5fc4a5637988c97f2e5a0e02d3be16ae0cb 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  dd761a66db4826580a67d64879e4c85278b8e20c 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  6a040a6a64c2086b5eb68a397697c9e2d2ca4d76 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  f5ec1ba1aff89d02b66d6a2cd1da8de1b3b08d06 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  c7738be2732b839aa2b460733c092e368909f935 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  455f98b72578ff977e29301cd2fc595ae80ee4ca 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  5c39b0d9f4d27ab82ef44392818c1810cb7664ce 
> 
> 
> Diff: https://reviews.apache.org/r/72028/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Laszlo Pinter
> 
>

Re: Review Request 71271: HIVE-21580: Introduce ISO 8601 week numbering SQL:2016 formats

2019-08-21 Thread Karen Coppage via Review Board


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

(Updated Aug. 21, 2019, 11:55 a.m.)


Review request for hive and Marta Kuczora.


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


Repository: hive-git


Description
---

Enable Hive to parse the following datetime formats when any combination/subset 
of these or previously implemented patterns is provided in one string. Also 
catch combinations that conflict.

IYYY
IYY
IY
I
IW


Diffs (updated)
-

  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveSqlDateTimeFormatter.java
 9443e8ec78 
  
common/src/test/org/apache/hadoop/hive/common/format/datetime/TestHiveSqlDateTimeFormatter.java
 ff41534fce 


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

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


Testing
---


Thanks,

Karen Coppage

Re: Review Request 71016: HIVE-21578: Introduce SQL:2016 formats FM, FX, and nested strings

2019-07-26 Thread Karen Coppage via Review Board


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

(Updated July 26, 2019, 10:01 a.m.)


Review request for hive and Marta Kuczora.


Changes
---

Patch 6


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


Repository: hive-git


Description
---

Enable Hive to parse the following datetime formats when any combination or 
subset of these or previously implemented formats is provided in one string. 

"text" (nested strings)
FM
FX


Diffs (updated)
-

  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveSqlDateTimeFormatter.java
 998e5a2f6a 
  
common/src/test/org/apache/hadoop/hive/common/format/datetime/TestHiveSqlDateTimeFormatter.java
 ac57842148 
  ql/src/test/queries/clientpositive/cast_datetime_with_sql_2016_format.q 
5a2a6d7894 
  ql/src/test/results/clientpositive/cast_datetime_with_sql_2016_format.q.out 
e1fd341050 


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

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


Testing
---


Thanks,

Karen Coppage

Re: Review Request 70920: HIVE-21868: Vectorize CAST...FORMAT

2019-07-04 Thread Karen Coppage via Review Board


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

(Updated July 4, 2019, 3:04 p.m.)


Review request for hive and Marta Kuczora.


Changes
---

Patch 7


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


Repository: hive-git


Description
---

Vectorize UDFs for CAST ( AS STRING/CHAR/VARCHAR FORMAT 
) and CAST ( AS TIMESTAMP/DATE FORMAT ).


Diffs (updated)
-

  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveSqlDateTimeFormatter.java
 4e024a357b 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 
fa9d1e9783 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastDateToCharWithFormat.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastDateToString.java
 dfa9f8a00d 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastDateToStringWithFormat.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastDateToVarCharWithFormat.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastStringToDate.java
 a6dff12e1a 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastStringToDateWithFormat.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastStringToTimestamp.java
 b48b0136eb 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastStringToTimestampWithFormat.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastTimestampToCharWithFormat.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastTimestampToString.java
 adc3a9d7b9 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastTimestampToStringWithFormat.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastTimestampToVarCharWithFormat.java
 PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCastFormat.java 
16742eee9b 
  
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorMathFunctions.java
 663237739e 
  
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorTypeCasts.java
 58fd7b030e 
  
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorTypeCastsWithFormat.java
 PRE-CREATION 
  ql/src/test/queries/clientnegative/udf_cast_format_bad_pattern.q PRE-CREATION 
  ql/src/test/queries/clientpositive/cast_datetime_with_sql_2016_format.q 
269edf6da6 
  ql/src/test/results/clientnegative/udf_cast_format_bad_pattern.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/cast_datetime_with_sql_2016_format.q.out 
4a502b9700 


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

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


Testing
---


Thanks,

Karen Coppage

Re: Review Request 70920: HIVE-21868: Vectorize CAST...FORMAT

2019-07-03 Thread Karen Coppage via Review Board



> On July 3, 2019, 10:51 a.m., Marta Kuczora wrote:
> > Thanks a lot Karen for the patch!
> > I have some questions, but otherwise the change looks good to me.

Thanks very much for the review!


> On July 3, 2019, 10:51 a.m., Marta Kuczora wrote:
> > common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveSqlDateTimeFormatter.java
> > Line 223 (original), 224 (patched)
> > 
> >
> > Why did you change the type of this variable to ArrayList from List?

I thought it was necessary in order to make the class serializable but now I 
see it's not. I'll fix this.


> On July 3, 2019, 10:51 a.m., Marta Kuczora wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastDateToString.java
> > Lines 59 (patched)
> > 
> >
> > Do the CastDateToString, CastDateToChar and CastDateToVarchar udfs use 
> > this method, or is this just a typo and the CastDateToStringWithFormat, ... 
> > udfs use this?

They do, all 3 classes eventually inherit from CastDateToString.


> On July 3, 2019, 10:51 a.m., Marta Kuczora wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCastFormat.java
> > Line 200 (original), 202 (patched)
> > 
> >
> > Is the formattedOutput variable never going to be null after this 
> > change? If there is a scenario where it can be null, it will cause problems 
> > when trying to cast it.

The output of format(Timestamp|Date) is a .toString() which I 
believe will never return null.


> On July 3, 2019, 10:51 a.m., Marta Kuczora wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCastFormat.java
> > Line 217 (original), 220 (patched)
> > 
> >
> > The same question about being null (previous comment) applies to the t 
> > and d variable as well.

This is not the case now but may be later on (if timestamp range of - 
is ever strictly enforced). I will include a null check here and above just in 
case.


- Karen


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


On June 26, 2019, 8:44 a.m., Karen Coppage wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70920/
> ---
> 
> (Updated June 26, 2019, 8:44 a.m.)
> 
> 
> Review request for hive and Marta Kuczora.
> 
> 
> Bugs: HIVE-21868
> https://issues.apache.org/jira/browse/HIVE-21868
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Vectorize UDFs for CAST ( AS STRING/CHAR/VARCHAR FORMAT 
> ) and CAST ( AS TIMESTAMP/DATE FORMAT ).
> 
> 
> Diffs
> -
> 
>   
> common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveSqlDateTimeFormatter.java
>  4e024a357b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 
> fa9d1e9783 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastDateToCharWithFormat.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastDateToString.java
>  dfa9f8a00d 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastDateToStringWithFormat.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastDateToVarCharWithFormat.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastStringToDate.java
>  a6dff12e1a 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastStringToDateWithFormat.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastStringToTimestamp.java
>  b48b0136eb 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastStringToTimestampWithFormat.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastTimestampToCharWithFormat.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastTimestampToString.java
>  adc3a9d7b9 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastTimestampToStringWithFormat.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastTimestampToVarCharWithFormat.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCastFormat.java 
> 16742eee9b 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorMathFunctions.java
>  663237739e 
>   
>

Re: Review Request 70841: HIVE-21576: Introduce CAST...FORMAT and limited list of SQL:2016 datetime formats

2019-06-14 Thread Karen Coppage via Review Board


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

(Updated June 14, 2019, 8:30 a.m.)


Review request for hive and Gabor Kaszab.


Changes
---

Patch 14


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


Repository: hive-git


Description
---

Timestamp and date handling and formatting are currently implemented in Hive 
using (sometimes very specific) Java SimpleDateFormat patterns with both 
SimpleDateFormat and java.time.DateTimeFormatter, however, these patterns are 
not what most standard SQL systems use. For example see Vertica, Netezza, 
Oracle, and PostgreSQL.

**Cast...Format**

SQL:2016 introduced the FORMAT clause for CAST which is the standard way to do 
string <-> datetime conversions

For example:

CAST( AS  [FORMAT ])
CAST( AS  [FORMAT ])
cast(dt as string format 'DD-MM-')
cast('01-05-2017' as date format 'DD-MM-')
Stuff like this wouldn't need to happen.

**New SQL:2016 Patterns**

Some conflicting examples:

SimpleDateTime: 'MMM dd,  HH:mm:ss'
SQL:2016: 'mon dd,  hh24:mi:ss'

SimpleDateTime: '-MM-dd HH:mm:ss'
SQL:2016: '-mm-dd hh24:mi:ss'

For the full list of patterns, see subsection "Proposal for Impala’s datetime 
patterns" in this doc: 
https://docs.google.com/document/d/1V7k6-lrPGW7_uhqM-FhKl3QsxwCRy69v2KIxPsGjc1k/edit

**Continued usage of SimpleDateFormat patterns**

[Update] This feature will NOT be behind a flag in order to keep things simple 
for users. Existing Hive functions that accept SimpleDateFormat patterns as 
input will continue to do so. Please let me know if you disagree with this 
decision. These are the functions (afaik) affected:

from_unixtime(bigint unixtime[, string format])
unix_timestamp(string date, string pattern)
to_unix_timestamp(date[, pattern])
add_months(string start_date, int num_months, output_date_format)
date_format(date/timestamp/string ts, string fmt)
This description is a heavily edited description of IMPALA-4018.


Diffs (updated)
-

  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveDateTimeFormatter.java
 PRE-CREATION 
  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveJavaDateTimeFormatter.java
 PRE-CREATION 
  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveSimpleDateFormatter.java
 PRE-CREATION 
  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveSqlDateTimeFormatter.java
 PRE-CREATION 
  
common/src/java/org/apache/hadoop/hive/common/format/datetime/package-info.java 
PRE-CREATION 
  
common/src/test/org/apache/hadoop/hive/common/format/datetime/TestHiveJavaDateTimeFormatter.java
 PRE-CREATION 
  
common/src/test/org/apache/hadoop/hive/common/format/datetime/TestHiveSimpleDateFormatter.java
 PRE-CREATION 
  
common/src/test/org/apache/hadoop/hive/common/format/datetime/TestHiveSqlDateTimeFormatter.java
 PRE-CREATION 
  
common/src/test/org/apache/hadoop/hive/common/format/datetime/package-info.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java d08b05fb68 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 58fe0cd32e 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCastFormat.java 
PRE-CREATION 
  
ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFCastFormat.java 
PRE-CREATION 
  ql/src/test/queries/clientpositive/cast_datetime_with_sql_2016_format.q 
PRE-CREATION 
  ql/src/test/results/clientpositive/cast_datetime_with_sql_2016_format.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/show_functions.q.out 374e9c4fce 


Diff: https://reviews.apache.org/r/70841/diff/5/

Changes: https://reviews.apache.org/r/70841/diff/4-5/


Testing
---


Thanks,

Karen Coppage

Re: Review Request 70841: HIVE-21576: Introduce CAST...FORMAT and limited list of SQL:2016 datetime formats

2019-06-14 Thread Karen Coppage via Review Board


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

(Updated June 14, 2019, 7:05 a.m.)


Review request for hive and Gabor Kaszab.


Changes
---

Patch 12


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


Repository: hive-git


Description
---

Timestamp and date handling and formatting are currently implemented in Hive 
using (sometimes very specific) Java SimpleDateFormat patterns with both 
SimpleDateFormat and java.time.DateTimeFormatter, however, these patterns are 
not what most standard SQL systems use. For example see Vertica, Netezza, 
Oracle, and PostgreSQL.

**Cast...Format**

SQL:2016 introduced the FORMAT clause for CAST which is the standard way to do 
string <-> datetime conversions

For example:

CAST( AS  [FORMAT ])
CAST( AS  [FORMAT ])
cast(dt as string format 'DD-MM-')
cast('01-05-2017' as date format 'DD-MM-')
Stuff like this wouldn't need to happen.

**New SQL:2016 Patterns**

Some conflicting examples:

SimpleDateTime: 'MMM dd,  HH:mm:ss'
SQL:2016: 'mon dd,  hh24:mi:ss'

SimpleDateTime: '-MM-dd HH:mm:ss'
SQL:2016: '-mm-dd hh24:mi:ss'

For the full list of patterns, see subsection "Proposal for Impala’s datetime 
patterns" in this doc: 
https://docs.google.com/document/d/1V7k6-lrPGW7_uhqM-FhKl3QsxwCRy69v2KIxPsGjc1k/edit

**Continued usage of SimpleDateFormat patterns**

[Update] This feature will NOT be behind a flag in order to keep things simple 
for users. Existing Hive functions that accept SimpleDateFormat patterns as 
input will continue to do so. Please let me know if you disagree with this 
decision. These are the functions (afaik) affected:

from_unixtime(bigint unixtime[, string format])
unix_timestamp(string date, string pattern)
to_unix_timestamp(date[, pattern])
add_months(string start_date, int num_months, output_date_format)
date_format(date/timestamp/string ts, string fmt)
This description is a heavily edited description of IMPALA-4018.


Diffs (updated)
-

  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveDateTimeFormatter.java
 PRE-CREATION 
  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveJavaDateTimeFormatter.java
 PRE-CREATION 
  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveSimpleDateFormatter.java
 PRE-CREATION 
  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveSqlDateTimeFormatter.java
 PRE-CREATION 
  
common/src/java/org/apache/hadoop/hive/common/format/datetime/package-info.java 
PRE-CREATION 
  
common/src/test/org/apache/hadoop/hive/common/format/datetime/TestHiveJavaDateTimeFormatter.java
 PRE-CREATION 
  
common/src/test/org/apache/hadoop/hive/common/format/datetime/TestHiveSimpleDateFormatter.java
 PRE-CREATION 
  
common/src/test/org/apache/hadoop/hive/common/format/datetime/TestHiveSqlDateTimeFormatter.java
 PRE-CREATION 
  
common/src/test/org/apache/hadoop/hive/common/format/datetime/package-info.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java d08b05fb68 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 58fe0cd32e 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCastFormat.java 
PRE-CREATION 
  
ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFCastWithFormat.java
 PRE-CREATION 
  ql/src/test/queries/clientpositive/cast_datetime_with_sql_2016_format.q 
PRE-CREATION 
  ql/src/test/results/clientpositive/cast_datetime_with_sql_2016_format.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/show_functions.q.out 374e9c4fce 


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

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


Testing
---


Thanks,

Karen Coppage

Review Request 70841: HIVE-21576: Introduce CAST...FORMAT and limited list of SQL:2016 datetime formats

2019-06-12 Thread Karen Coppage via Review Board


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

Review request for hive.


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


Repository: hive-git


Description
---

Timestamp and date handling and formatting are currently implemented in Hive 
using (sometimes very specific) Java SimpleDateFormat patterns with both 
SimpleDateFormat and java.time.DateTimeFormatter, however, these patterns are 
not what most standard SQL systems use. For example see Vertica, Netezza, 
Oracle, and PostgreSQL.

**Cast...Format**

SQL:2016 introduced the FORMAT clause for CAST which is the standard way to do 
string <-> datetime conversions

For example:

CAST( AS  [FORMAT ])
CAST( AS  [FORMAT ])
cast(dt as string format 'DD-MM-')
cast('01-05-2017' as date format 'DD-MM-')
Stuff like this wouldn't need to happen.

**New SQL:2016 Patterns**

Some conflicting examples:

SimpleDateTime: 'MMM dd,  HH:mm:ss'
SQL:2016: 'mon dd,  hh24:mi:ss'

SimpleDateTime: '-MM-dd HH:mm:ss'
SQL:2016: '-mm-dd hh24:mi:ss'

For the full list of patterns, see subsection "Proposal for Impala’s datetime 
patterns" in this doc: 
https://docs.google.com/document/d/1V7k6-lrPGW7_uhqM-FhKl3QsxwCRy69v2KIxPsGjc1k/edit

**Continued usage of SimpleDateFormat patterns**

[Update] This feature will NOT be behind a flag in order to keep things simple 
for users. Existing Hive functions that accept SimpleDateFormat patterns as 
input will continue to do so. Please let me know if you disagree with this 
decision. These are the functions (afaik) affected:

from_unixtime(bigint unixtime[, string format])
unix_timestamp(string date, string pattern)
to_unix_timestamp(date[, pattern])
add_months(string start_date, int num_months, output_date_format)
date_format(date/timestamp/string ts, string fmt)
This description is a heavily edited description of IMPALA-4018.


Diffs
-

  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveDateTimeFormatter.java
 PRE-CREATION 
  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveJavaDateTimeFormatter.java
 PRE-CREATION 
  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveSimpleDateFormatter.java
 PRE-CREATION 
  
common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveSqlDateTimeFormatter.java
 PRE-CREATION 
  
common/src/java/org/apache/hadoop/hive/common/format/datetime/package-info.java 
PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/type/Date.java 6ecfcf65c9 
  common/src/java/org/apache/hadoop/hive/common/type/Timestamp.java a8b7b6d186 
  common/src/java/org/apache/hadoop/hive/common/type/TimestampUtils.java 
f26f8ae01e 
  common/src/java/org/apache/hive/common/util/DateParser.java 5db14f1906 
  
common/src/test/org/apache/hadoop/hive/common/format/datetime/TestHiveJavaDateTimeFormatter.java
 PRE-CREATION 
  
common/src/test/org/apache/hadoop/hive/common/format/datetime/TestHiveSimpleDateFormatter.java
 PRE-CREATION 
  
common/src/test/org/apache/hadoop/hive/common/format/datetime/TestHiveSqlDateTimeFormatter.java
 PRE-CREATION 
  
common/src/test/org/apache/hadoop/hive/common/format/datetime/package-info.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java d08b05fb68 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 
fa9d1e9783 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastDateToCharWithFormat.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastDateToString.java
 dfa9f8a00d 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastDateToStringWithFormat.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastDateToVarCharWithFormat.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastStringToDate.java
 a6dff12e1a 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastStringToDateWithFormat.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastStringToTimestamp.java
 b48b0136eb 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastStringToTimestampWithFormat.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastTimestampToCharWithFormat.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastTimestampToString.java
 adc3a9d7b9 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastTimestampToStringWithFormat.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastTimestampToVarCharWithFormat.java
 PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 58fe0cd32e

Re: Review Request 69254: HIVE-20818: Views created with a WHERE subquery will regard views referenced in the subquery as direct input

2018-11-19 Thread Karen Coppage via Review Board


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

(Updated Nov. 19, 2018, 3:12 p.m.)


Review request for hive.


Changes
---

HIVE-20818.6.patch -- passed Ptests


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


Repository: hive-git


Description
---

If Hive is configured with an authorization hook like Sentry, and a view is 
created with a WHERE clause referencing a different view' user has no access 
to, user cannot access the view as view' is considered direct input.
WHERE IN and WHERE EXISTS cause the same issue.
Cascading views created with no WHERE clauses (i.e. with simple SELECTs and 
FROM clauses) work fine.

See Jira for example


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java b3c6806217 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1a2777bf45 
  ql/src/test/org/apache/hadoop/hive/ql/plan/TestViewEntity.java 6ad38b8467 
  ql/src/test/results/clientpositive/llap/explainuser_1.q.out c86450aae2 
  ql/src/test/results/clientpositive/masking_12.q.out 9ecd981797 
  ql/src/test/results/clientpositive/spark/spark_explainuser_1.q.out 1f681944cd 
  ql/src/test/results/clientpositive/spark/subquery_views.q.out c221392264 


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

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


Testing
---

Added unit test


Thanks,

Karen Coppage

Re: Review Request 69254: HIVE-20818: Views created with a WHERE subquery will regard views referenced in the subquery as direct input

2018-11-07 Thread Karen Coppage via Review Board


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

(Updated Nov. 7, 2018, 8:56 a.m.)


Review request for hive.


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


Repository: hive-git


Description
---

If Hive is configured with an authorization hook like Sentry, and a view is 
created with a WHERE clause referencing a different view' user has no access 
to, user cannot access the view as view' is considered direct input.
WHERE IN and WHERE EXISTS cause the same issue.
Cascading views created with no WHERE clauses (i.e. with simple SELECTs and 
FROM clauses) work fine.

See Jira for example


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java ab63ce2bc3 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1a2777bf45 
  ql/src/test/org/apache/hadoop/hive/ql/plan/TestViewEntity.java 6ad38b8467 


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

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


Testing
---

Added unit test


Thanks,

Karen Coppage

Re: Review Request 69254: HIVE-20818: Views created with a WHERE subquery will regard views referenced in the subquery as direct input

2018-11-06 Thread Karen Coppage via Review Board



> On Nov. 5, 2018, 8:41 p.m., Peter Vary wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
> > Lines 3321 (patched)
> > 
> >
> > Is the problem only affects CBO, or RBO as well?
> > What happens when CBO is off?

Great question, this version ignores RBO. Fix is coming in new patch.


- Karen


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


On Nov. 5, 2018, 3:14 p.m., Karen Coppage wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69254/
> ---
> 
> (Updated Nov. 5, 2018, 3:14 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-20818
> https://issues.apache.org/jira/browse/HIVE-20818
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> If Hive is configured with an authorization hook like Sentry, and a view is 
> created with a WHERE clause referencing a different view' user has no access 
> to, user cannot access the view as view' is considered direct input.
> WHERE IN and WHERE EXISTS cause the same issue.
> Cascading views created with no WHERE clauses (i.e. with simple SELECTs and 
> FROM clauses) work fine.
> 
> See Jira for example
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java ab63ce2bc3 
>   ql/src/test/org/apache/hadoop/hive/ql/plan/TestViewEntity.java 6ad38b8467 
> 
> 
> Diff: https://reviews.apache.org/r/69254/diff/1/
> 
> 
> Testing
> ---
> 
> Added unit test
> 
> 
> Thanks,
> 
> Karen Coppage
> 
>

Review Request 69254: HIVE-20818: Views created with a WHERE subquery will regard views referenced in the subquery as direct input

2018-11-05 Thread Karen Coppage via Review Board


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

Review request for hive.


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


Repository: hive-git


Description
---

If Hive is configured with an authorization hook like Sentry, and a view is 
created with a WHERE clause referencing a different view' user has no access 
to, user cannot access the view as view' is considered direct input.
WHERE IN and WHERE EXISTS cause the same issue.
Cascading views created with no WHERE clauses (i.e. with simple SELECTs and 
FROM clauses) work fine.

See Jira for example


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java ab63ce2bc3 
  ql/src/test/org/apache/hadoop/hive/ql/plan/TestViewEntity.java 6ad38b8467 


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


Testing
---

Added unit test


Thanks,

Karen Coppage

Re: Review Request 68710: HIVE-20544: TOpenSessionReq logs password and username

2018-09-24 Thread Karen Coppage via Review Board


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

(Updated Sept. 24, 2018, 2:01 p.m.)


Review request for hive and Laszlo Pinter.


Changes
---

Fixed typo in last diff


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


Repository: hive-git


Description
---

TOpenSessionReq, if client protocol is unset, both username and password are 
logged. Logging a password is a security risk. This patch would hide it with 
asterisks.


Diffs (updated)
-

  service-rpc/pom.xml d6a07a55bc 
  
service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TOpenSessionReq.java
 3195e704f3 


Diff: https://reviews.apache.org/r/68710/diff/5/

Changes: https://reviews.apache.org/r/68710/diff/4-5/


Testing
---


File Attachments (updated)


HIVE-20544.3.patch
  
https://reviews.apache.org/media/uploaded/files/2018/09/24/9f8ef0d8-22df-40cf-a311-56335d88516a__HIVE-20544.3.patch
HIVE-20544.3.patch
  
https://reviews.apache.org/media/uploaded/files/2018/09/24/afdfc085-cc06-4a47-81f8-499029719bd0__HIVE-20544.3.patch


Thanks,

Karen Coppage

Re: Review Request 68710: HIVE-20544: TOpenSessionReq logs password and username

2018-09-24 Thread Karen Coppage via Review Board


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

(Updated Sept. 24, 2018, 1:47 p.m.)


Review request for hive and Laszlo Pinter.


Changes
---

Whether the password is set or not, "password:-" is printed to logs.


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


Repository: hive-git


Description
---

TOpenSessionReq, if client protocol is unset, both username and password are 
logged. Logging a password is a security risk. This patch would hide it with 
asterisks.


Diffs (updated)
-

  
service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TOpenSessionReq.java
 3195e704f3 


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

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


Testing
---


Thanks,

Karen Coppage

Re: Review Request 68710: HIVE-20544: TOpenSessionReq logs password and username

2018-09-24 Thread Karen Coppage via Review Board



> On Sept. 21, 2018, 3:41 p.m., Andrew Sherman wrote:
> > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TOpenSessionReq.java
> > Line 546 (original), 546 (patched)
> > 
> >
> > why give a clue about password length? Maybe just always print  or 
> > something?

Thanks for taking a look, Andrew! Fair point. I would worry that just printing 
some asterisks could confuse someone ("Is my password really that short?"), so 
i'll replace the password mask with a simple "-" in the next patch.


- Karen


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


On Sept. 21, 2018, 3:31 p.m., Karen Coppage wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68710/
> ---
> 
> (Updated Sept. 21, 2018, 3:31 p.m.)
> 
> 
> Review request for hive and Laszlo Pinter.
> 
> 
> Bugs: HIVE-20544
> https://issues.apache.org/jira/browse/HIVE-20544
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> TOpenSessionReq, if client protocol is unset, both username and password are 
> logged. Logging a password is a security risk. This patch would hide it with 
> asterisks.
> 
> 
> Diffs
> -
> 
>   service-rpc/pom.xml d6a07a55bc 
>   
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TOpenSessionReq.java
>  3195e704f3 
> 
> 
> Diff: https://reviews.apache.org/r/68710/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Karen Coppage
> 
>

Re: Review Request 68710: HIVE-20544: TOpenSessionReq logs password and username

2018-09-21 Thread Karen Coppage via Review Board


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

(Updated Sept. 21, 2018, 3:31 p.m.)


Review request for hive and Laszlo Pinter.


Changes
---

Password is masked by Maven in process-sources phase of build.


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


Repository: hive-git


Description
---

TOpenSessionReq, if client protocol is unset, both username and password are 
logged. Logging a password is a security risk. This patch would hide it with 
asterisks.


Diffs (updated)
-

  service-rpc/pom.xml d6a07a55bc 
  
service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TOpenSessionReq.java
 3195e704f3 


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

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


Testing
---


Thanks,

Karen Coppage

40 matches

Mail list logo