You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/10/12 12:17:24 UTC

[GitHub] [arrow-datafusion] alex-natzka commented on issue #3635: after type coercion `CommonSubexprEliminate` will produce invalid projection

alex-natzka commented on issue #3635:
URL: https://github.com/apache/arrow-datafusion/issues/3635#issuecomment-1276067797

   > @alex-natzka Could you please take a look the test `multiple_or_predicates`, I think it is failed now.
   > 
   > https://github.com/apache/arrow-datafusion/blob/d72eb9a1c4c18bcabbf941541a9c1defa83a592c/datafusion/core/tests/sql/predicates.rs#L433
   > 
   > I ignore this test, because it will produce unexpected result. cc @alamb @andygrove
   > 
   > ```
   > expected:
   > 
   > [
   >     "Explain [plan_type:Utf8, plan:Utf8]",
   >     "  Projection: lineitem.l_partkey [l_partkey:Int64]",
   >     "    Filter: part.p_brand = Utf8(\"Brand#12\") AND lineitem.l_quantity >= Decimal128(Some(100),15,2) AND lineitem.l_quantity <= Decimal128(Some(1100),15,2) AND CAST(part.p_size AS Int64) BETWEEN Int64(1) AND Int64(5) OR part.p_brand = Utf8(\"Brand#23\") AND lineitem.l_quantity >= Decimal128(Some(1000),15,2) AND lineitem.l_quantity <= Decimal128(Some(2000),15,2) AND CAST(part.p_size AS Int64) BETWEEN Int64(1) AND Int64(10) OR part.p_brand = Utf8(\"Brand#34\") AND lineitem.l_quantity >= Decimal128(Some(2000),15,2) AND lineitem.l_quantity <= Decimal128(Some(3000),15,2) AND CAST(part.p_size AS Int64) BETWEEN Int64(1) AND Int64(15) [l_partkey:Int64, l_quantity:Decimal128(15, 2), p_partkey:Int64, p_brand:Utf8, p_size:Int32]",
   >     "      Inner Join: lineitem.l_partkey = part.p_partkey [l_partkey:Int64, l_quantity:Decimal128(15, 2), p_partkey:Int64, p_brand:Utf8, p_size:Int32]",
   >     "        TableScan: lineitem projection=[l_partkey, l_quantity] [l_partkey:Int64, l_quantity:Decimal128(15, 2)]",
   >     "        TableScan: part projection=[p_partkey, p_brand, p_size] [p_partkey:Int64, p_brand:Utf8, p_size:Int32]",
   > ]
   > actual:
   > 
   > [
   >     "Explain [plan_type:Utf8, plan:Utf8]",
   >     "  Projection: lineitem.l_partkey [l_partkey:Int64]",
   >     "    Projection: part.p_size >= Int32(1) AS part.p_size >= Int32(1)Int32(1)part.p_size, lineitem.l_partkey, lineitem.l_quantity, part.p_brand, part.p_size [part.p_size >= Int32(1)Int32(1)part.p_size:Boolean;N, l_partkey:Int64, l_quantity:Decimal128(15, 2), p_brand:Utf8, p_size:Int32]",
   >     "      Filter: part.p_brand = Utf8(\"Brand#12\") AND lineitem.l_quantity >= Decimal128(Some(100),15,2) AND lineitem.l_quantity <= Decimal128(Some(1100),15,2) AND part.p_size <= Int32(5) OR part.p_brand = Utf8(\"Brand#23\") AND lineitem.l_quantity >= Decimal128(Some(1000),15,2) AND lineitem.l_quantity <= Decimal128(Some(2000),15,2) AND part.p_size <= Int32(10) OR part.p_brand = Utf8(\"Brand#34\") AND lineitem.l_quantity >= Decimal128(Some(2000),15,2) AND lineitem.l_quantity <= Decimal128(Some(3000),15,2) AND part.p_size <= Int32(15) [l_partkey:Int64, l_quantity:Decimal128(15, 2), p_partkey:Int64, p_brand:Utf8, p_size:Int32]",
   >     "        Inner Join: lineitem.l_partkey = part.p_partkey [l_partkey:Int64, l_quantity:Decimal128(15, 2), p_partkey:Int64, p_brand:Utf8, p_size:Int32]",
   >     "          TableScan: lineitem projection=[l_partkey, l_quantity] [l_partkey:Int64, l_quantity:Decimal128(15, 2)]",
   >     "          Filter: part.p_size >= Int32(1) [p_partkey:Int64, p_brand:Utf8, p_size:Int32]",
   >     "            TableScan: part projection=[p_partkey, p_brand, p_size], partial_filters=[part.p_size >= Int32(1)] [p_partkey:Int64, p_brand:Utf8, p_size:Int32]",
   > ]
   > ```
   
   @liukun4515 looks like you needed to ignore the test for your previous PR #3636 . I'm a bit confused because, at least at first glance, it looks like the `actual` output is not wrong. (I didn't look very thoroughly at this complicated expression, though.) Also, the unexpected test output seems unrelated to the wrong extra columns that the `CommonSubexprEliminate` optimizer rule created, which is now fixed.
   
   It may be safe to create a PR that un-ignores the test and updates the `expected` value with what we see here, though maybe @alamb and @andygrove can weigh in on this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org