Add InSubqueryExec support (to support DPP) #121

Activity

1. viirya

   added

   enhancementNew feature or requestNew feature or request

   on Feb 27, 2024
2. advancedxy commented on Feb 29, 2024

   

   advancedxy

   on Feb 29, 2024

   Contributor

   More actions

   I can work on this if no one is currently working on it.
3. advancedxy commented on Mar 6, 2024

   

   advancedxy

   on Mar 6, 2024

   Contributor

   More actions

   I did some research to support InSubqueryExec. I think we should postpone the support a little bit, at least after Comet supporting Join operators.

   The InSubqueryExec is mainly used for

   1. DPP(dynamic partition pruning), which evaluates the in predicate in the driver side.
   2. Some special cases, which actually performs the inSet evaluation in the executor side(for Comet, the native side).

   For the first part, it would be pretty straightforward to support in the Comet side as all the evaluations happens at the driver(/JVM) side. We can model that like InSubqueryExec to prepare subqueries first and do some potential expression and plan transforms. We are good to go. However, DPP applies to Join operators. It would be reasonable to add DPP support after we have Join operators in Comet.

   For the second part, it's slightly complicated. Per my understanding, we have multiple options:

   1. Like we did for ScalarSubqueryExec, we can add a InSubquery PhysicalExpr implementation. The main problem is how to transform the list data from JVM to the native side. I'm skeptical to just transfer the java object array via the JNI call as the list might be pretty big. Maybe we should transform that to a RecordBatch/CometVector and then pass it back to the native side?
   2. Instead of implementing InSubquery, we can rewrite it with the InSet expression as we have already has the subquery list collected before we actually execute the plan. The problem is that:
      • Currently, we don't have a way to rewrite/transform the native operator after we created it
      • The proto message should have a size limit, something like 64MB? It will not work for the huge inSet.

   cc @viirya @sunchao appreciate if you guys have more insights about this topic.
4. viirya commented on Mar 6, 2024

   

   viirya

   on Mar 6, 2024

   MemberAuthor

   More actions

   For the first part, it would be pretty straightforward to support in the Comet side as all the evaluations happens at the driver(/JVM) side.

   Hm? For DPP, the only difference is we don't need to broadcast the evaluation result. I'm not sure about "all evaluations happen at the driver side". I think the subplan is still needed to be executed on executors like ScalarSubqueryExec does.

   Like we did for ScalarSubqueryExec, we can add a InSubquery PhysicalExpr implementation. The main problem is how to transform the list data from JVM to the native side. I'm skeptical to just transfer the java object array via the JNI call as the list might be pretty big. Maybe we should transform that to a RecordBatch/CometVector and then pass it back to the native side?

   I think we can have an initial version which does simple first by passing java objects through JNI call. Or, instead, we can keep the output of subplan in JVM, and we evaluate the JVM InSet expression through JNI call from native side. As InSet evaluation should be fast as it's only a hash table lookup.

   Instead of implementing InSubquery, we can rewrite it with the InSet expression as we have already has the subquery list collected before we actually execute the plan. The problem is that:

   InSubqueryExec is a subquery expression which is different to InSet. I'm not sure how do you plan to rewrite it to an InSet but also keep the subquery plan.
5. advancedxy commented on Mar 7, 2024

   

   advancedxy

   on Mar 7, 2024

   Last edited by advancedxy

   Contributor

   More actions

   I'm not sure about "all evaluations happen at the driver side"

   Emmm, I should be more specific. I meant all the in evaluations are happened at the driver side. The subplan/subquery is of course needed to be executed/prepared first.

   I think we can have an initial version which does simple first by passing java objects through JNI call

   Hmm, this is always a valid option to go. I'm wondering whether it's simple enough to just convert the list of literal into an Arrow's ColumnVector as we can reuse all the infrastructure.

   Or, instead, we can keep the output of subplan in JVM, and we evaluate the JVM InSet expression through JNI call from native side. As InSet evaluation should be fast as it's only a hash table lookup.

   I'm not sure about this approach, and never consider this as an option. The problem about evaluate the InSet expression in the JVM side is that now it requires that we do RecordBatch to InternalRow conversation in the native side(the InSuqueryExec requires an InternalRow to evaluate the child), and passing it back to JVM and get back the result. It generally defeats columnar execution?

   I'm not sure how do you plan to rewrite it to an InSet but also keep the subquery plan.

   Currently, in org.apache.spark.sql.comet.CometNativeExec#doExecuteColumnar, instead simply copy the serializedPlan message, we convert it back to operator message and transform the InSubQuery expr to InSet expr. There's no such utility to support that through, or ,maybe we are never going to go with that way.
6. andygrove

   self-assigned this

   on Jan 7, 2025
7. andygrove commented on Jan 7, 2025

   

   andygrove

   on Jan 7, 2025

   Member

   More actions

   I'm going to pick this issue up. I am currently studying Spark's code to understand how it works without Comet.
8. andygrove

   added this to the 0.6.0 milestone on Jan 7, 2025
9. andygrove

   changed the title [-]Add `InSubqueryExec` support[/-] [+]Add `InSubqueryExec` support (to support DPP)[/+] on Jan 9, 2025
10. advancedxy commented on Jan 23, 2025

    

    advancedxy

    on Jan 23, 2025

    Contributor

    More actions

    Thanks for taking this over, Andy. I was kind of busy in 2024H2. Hope I can find some more spare time to contribute back to Comet in 2025.
11. andygrove

    removed this from the 0.6.0 milestone on Jan 24, 2025
12. andygrove

    removed their assignment

    on Aug 11, 2025
13. coderfender commented on May 6, 2026

    

    coderfender

    on May 6, 2026

    Contributor

    More actions

    @mbutrovich ,I believe we can close this issue ?
14. mbutrovich commented on May 6, 2026

    

    mbutrovich

    on May 6, 2026

    Contributor

    More actions

    Yep, I think we can! Thanks @coderfender!
15. mbutrovich

    closed this as completedon May 6, 2026