Copilot commented on code in PR #12817:
URL: https://github.com/apache/gluten/pull/12817#discussion_r3807666871
##########
docs/developers/velox-function-development-guide.md:
##########
@@ -39,6 +39,33 @@ registerBinaryIntegral<BitwiseAndFunction>({prefix +
"bitwise_and"});
Functions for complex types have similar implementations.
See `ArrayAverageFunction` in
[velox/functions/prestosql/ArrayFunctions.h](https://github.com/facebookincubator/velox/blob/main/velox/functions/prestosql/ArrayFunctions.h).
+# Gluten Function Overlay
+
+Upstreaming a function to Velox can take a long time. To avoid being blocked
on that, Gluten provides a function overlay in
+[cpp/velox/operators/functions/overlay](https://github.com/apache/incubator-gluten/tree/main/cpp/velox/operators/functions/overlay),
Review Comment:
This link points to the old apache/incubator-gluten repository. It should
point to the current apache/gluten repo so the documentation doesn’t send
readers to the wrong location.
##########
docs/developers/velox-function-development-guide.md:
##########
@@ -39,6 +39,33 @@ registerBinaryIntegral<BitwiseAndFunction>({prefix +
"bitwise_and"});
Functions for complex types have similar implementations.
See `ArrayAverageFunction` in
[velox/functions/prestosql/ArrayFunctions.h](https://github.com/facebookincubator/velox/blob/main/velox/functions/prestosql/ArrayFunctions.h).
+# Gluten Function Overlay
Review Comment:
This page already has a single top-level H1 heading ("# Developer
Guide..."). The new overlay section should be a second-level heading to keep a
consistent heading hierarchy (H1 for the page title, H2 for sections).
##########
cpp/velox/operators/functions/RegistrationAllFunctions.cc:
##########
@@ -51,14 +50,6 @@ namespace gluten {
namespace {
void registerFunctionOverwrite() {
Review Comment:
`registerFunctionOverwrite` no longer overwrites any Velox functions (the
round override moved to the overlay). Consider renaming this helper to reflect
what it does now (registers Gluten special forms / helper functions), and
update the call site accordingly to avoid future confusion.
This issue also appears on line 89 of the same file.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
