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




ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java
Line 1171 (original), 1177 (patched)
<https://reviews.apache.org/r/69019/#comment294331>

    wrong comparisions spelling



ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java
Lines 1296 (patched)
<https://reviews.apache.org/r/69019/#comment294329>

    Should be this be more appropriate as assert? If this is not the case it 
sounds like an invalid IN



ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java
Lines 1323 (patched)
<https://reviews.apache.org/r/69019/#comment294330>

    assert?



ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java
Lines 1329 (patched)
<https://reviews.apache.org/r/69019/#comment294337>

    Can you add test case e.g. select 'expected 3',count(*) from ax where (s,t) 
in (('a','a'),(null, 'b'));
    
    I don't think returning null for whole struct is correct, null could be 
representing valid value and need to be typed appropriately.



ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java
Lines 1340 (patched)
<https://reviews.apache.org/r/69019/#comment294335>

    These utility methods would be more suited in ExprNodeDescUtils



ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java
Line 1287 (original), 1400 (patched)
<https://reviews.apache.org/r/69019/#comment294336>

    Currently we don't handle case such as (s,t) IN ('c', 'b'). Here right side 
is not struct but has two expr node constant but left side is struct


- Vineet Garg


On Oct. 15, 2018, 6:05 a.m., Zoltan Haindrich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69019/
> -----------------------------------------------------------
> 
> (Updated Oct. 15, 2018, 6:05 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-20617
>     https://issues.apache.org/jira/browse/HIVE-20617
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> For IN expressions the types were never corrected; and pointlookupoptimizer 
> was probably leaving behind fields already which were uncomparable; 
> HIVE-20296 exposed it further by changing the minimal number from  32 to 2.
> 
> This change generalizes the retyping of constants to also run it for the IN 
> operator ; and also for struct-s.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 
> 4968d16876c5c9cc36ec9a3ec48c2740c2c67dcd 
>   ql/src/test/queries/clientpositive/in_typecheck1.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/in_typecheck2.q PRE-CREATION 
>   ql/src/test/results/clientpositive/alter_partition_coltype.q.out 
> 5727f0a65c6e4736f41017e4e962d932dedbd6bd 
>   ql/src/test/results/clientpositive/cbo_rp_simple_select.q.out 
> 43cb5ab89fdebde8be168d7837d8e54a38f4d10b 
>   ql/src/test/results/clientpositive/cbo_simple_select.q.out 
> 2073c6b802a1ae0ff4228a86f18ec366ff92ab02 
>   ql/src/test/results/clientpositive/in_typecheck1.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/in_typecheck2.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/infer_const_type.q.out 
> 4129bd0c715635eb83c0c0d248eb43a5779c7be9 
>   ql/src/test/results/clientpositive/join45.q.out 
> 77dbaa2cd8b5be7158545c696b30dc1068238f91 
>   ql/src/test/results/clientpositive/join47.q.out 
> 2536f7f4b6e9295d1177632b7f32f0b66974e3a4 
>   ql/src/test/results/clientpositive/llap/cbo_simple_select.q.out 
> e61300b5c853eb733d4443c047344e3fc6fe0ff3 
>   ql/src/test/results/clientpositive/llap/dec_str.q.out 
> 3b7f92d735db79f7c4c0d96debe7fff8e3c05f11 
>   ql/src/test/results/clientpositive/llap/explainuser_1.q.out 
> bc1f97dd49ccc905d5e32d5a02a62bb692d444a6 
>   ql/src/test/results/clientpositive/llap/lineage3.q.out 
> cf388161272002ad6097839ceaea2bcfbcf9b7ef 
>   ql/src/test/results/clientpositive/llap/mapjoin_hint.q.out 
> 3c6270a05240097e9645e664ed30e0568052d98e 
>   ql/src/test/results/clientpositive/llap/subquery_scalar.q.out 
> feadbcd874818a78e2cc30b86cdebc1e4cb6a04f 
>   ql/src/test/results/clientpositive/llap/vectorization_13.q.out 
> 398cb56915f1b24b7c4dc325b60cb114d7ff2b8c 
>   ql/src/test/results/clientpositive/llap/vectorization_6.q.out 
> 70542ac7bd69e46098cf8158cae347e6c896c5b2 
>   ql/src/test/results/clientpositive/llap/vectorization_8.q.out 
> 662409d4f148cc3da5c4f788ddf59c6f40ede572 
>   ql/src/test/results/clientpositive/llap/vectorization_short_regress.q.out 
> a59a586144fc9dc14ac2fa87177c189feae47402 
>   ql/src/test/results/clientpositive/mapjoin47.q.out 
> c42094d7858fa70626a5184485a13fdacd45be7c 
>   ql/src/test/results/clientpositive/parquet_vectorization_13.q.out 
> e60548cb779826c0e50b5086cabe0b1408f4f182 
>   ql/src/test/results/clientpositive/parquet_vectorization_6.q.out 
> 85b075666f9cdcd15b2adbab07d665b8def863c0 
>   ql/src/test/results/clientpositive/parquet_vectorization_8.q.out 
> c089aab12554caa6ea38d1a0d90abf0c050b8ba0 
>   ql/src/test/results/clientpositive/ppd_udf_col.q.out 
> dfc2d0465d49f7c77452113d3791104a05aa42ef 
>   ql/src/test/results/clientpositive/spark/cbo_simple_select.q.out 
> e61300b5c853eb733d4443c047344e3fc6fe0ff3 
>   ql/src/test/results/clientpositive/spark/parquet_vectorization_13.q.out 
> 78a2428cfbd508c768100896955b8853a9bd2a50 
>   ql/src/test/results/clientpositive/spark/parquet_vectorization_6.q.out 
> 362d19c39b60ec2193fc73098787723042f53aab 
>   ql/src/test/results/clientpositive/spark/parquet_vectorization_8.q.out 
> b10b550009d383f84f19ac72207a9c94b12c497d 
>   ql/src/test/results/clientpositive/spark/subquery_scalar.q.out 
> af325200ba7ae7aa5ab0aa42ecf714eaa4afb122 
>   ql/src/test/results/clientpositive/spark/vectorization_13.q.out 
> a49738ecaac6324d529954ca14b952b4898e33dd 
>   ql/src/test/results/clientpositive/spark/vectorization_6.q.out 
> 8f7d3f2530da2afc10e8dfb23339568279d8c454 
>   ql/src/test/results/clientpositive/spark/vectorization_short_regress.q.out 
> 3844c79e1a3e1d37b8b93e4f39bc9b52992888f3 
>   ql/src/test/results/clientpositive/vectorization_13.q.out 
> d2f34481371122b2d6a33578a68310b739eb023f 
>   ql/src/test/results/clientpositive/vectorization_6.q.out 
> 99d917fdfa101cba42a1f91b5ac39d9401584a7d 
>   ql/src/test/results/clientpositive/vectorization_8.q.out 
> b9b0c8af17b13cebb2301012fadf3395786b4f62 
> 
> 
> Diff: https://reviews.apache.org/r/69019/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zoltan Haindrich
> 
>

Reply via email to