You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@gluten.apache.org by "kevincmchen (via GitHub)" <gi...@apache.org> on 2025/03/24 11:14:17 UTC

[PR] [GLUTEN-9113][VL] Remove Unused not_equal Function Mapping [incubator-gluten]

kevincmchen opened a new pull request, #9114:
URL: https://github.com/apache/incubator-gluten/pull/9114

   ## What changes were proposed in this pull request?
   
   Spark internally rewrites (a != b) as NOT (a = b), this meas spark never use the mapping(not_equal -> notequalto). velox does not implement a native notequalto function. so this maping should be removed.
   
   (Fixes: \#9113)
   
   
   


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9113][VL] Remove Unused not_equal Function Mapping [incubator-gluten]

Posted by "rui-mo (via GitHub)" <gi...@apache.org>.

rui-mo commented on PR #9114:
URL: https://github.com/apache/incubator-gluten/pull/9114#issuecomment-2748972523

   > I would prefer keep notequalto, because we should so some optimization in Velox to support this expression instead of using 2 operators. Is it that? @rui-mo
   
   Currently we are implementation Velox functions as a one-one mapping way, which means one Spark function corresponds to a Velox function for most cases. Perhaps we will need a benchmark to show noticeable performance improvement before we implementing a combined function.


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9113][VL] Remove Unused not_equal Function Mapping [incubator-gluten]

Posted by "jinchengchenghh (via GitHub)" <gi...@apache.org>.

jinchengchenghh commented on PR #9114:
URL: https://github.com/apache/incubator-gluten/pull/9114#issuecomment-2748540192

   I would prefer keep notequalto, because we should so some optimization in Velox to support this expression instead of using 2 operators. Is it that? @rui-mo 


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9113][VL] Remove Unused not_equal Function Mapping [incubator-gluten]

Posted by "PHILO-HE (via GitHub)" <gi...@apache.org>.

PHILO-HE commented on PR #9114:
URL: https://github.com/apache/incubator-gluten/pull/9114#issuecomment-2749895873

   @rui-mo, seems this mapping `{"not_equal", "notequalto"}` is legacy code that has never been used now.
   
   CH backend engineer is doing some optimization to collapse nested expression into one, e.g., And((a And b), c) into And(a, b, c). The collapse optimization can generally bring perf. benefit. But for this legacy involved in this pr, I think we firstly can remove it.


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9113][VL] Remove Unused not_equal Function Mapping [incubator-gluten]

Posted by "PHILO-HE (via GitHub)" <gi...@apache.org>.

PHILO-HE commented on PR #9114:
URL: https://github.com/apache/incubator-gluten/pull/9114#issuecomment-2749911792

   FYI. The registering for "notequalto" (a presto function) was removed by https://github.com/facebookincubator/velox/pull/5569, which seems for fixing semantic inconsistency issue.


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9113][VL] Remove unused `not_equal` function mapping [incubator-gluten]

Posted by "rui-mo (via GitHub)" <gi...@apache.org>.

rui-mo commented on PR #9114:
URL: https://github.com/apache/incubator-gluten/pull/9114#issuecomment-2750530750

   > And((a And b), c) into And(a, b, c)
   
   @PHILO-HE It appears that Velox contains this optimisation https://facebookincubator.github.io/velox/develop/expression-evaluation.html#flatten-ands-and-ors. I agree we remove the legacy code first.


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9113][VL] Remove Unused not_equal Function Mapping [incubator-gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #9114:
URL: https://github.com/apache/incubator-gluten/pull/9114#issuecomment-2747767262

   https://github.com/apache/incubator-gluten/issues/9113


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9113][VL] Remove unused `not_equal` function mapping [incubator-gluten]

Posted by "PHILO-HE (via GitHub)" <gi...@apache.org>.

PHILO-HE merged PR #9114:
URL: https://github.com/apache/incubator-gluten/pull/9114


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org