Copilot commented on code in PR #64737:
URL: https://github.com/apache/doris/pull/64737#discussion_r3458488231
##########
be/test/CMakeLists.txt:
##########
Review Comment:
`VARIANT_NESTED_GROUP_MODULE_DIR` is documented as “under be/src”, but here
it’s being resolved relative to `be/test` (`CMAKE_CURRENT_SOURCE_DIR`), which
will point to `be/test/<module>` instead of `be/src/<module>` and likely glob
nothing. Resolve the module directory consistently (e.g., relative to `be/src`
or using the same base path approach as `be/src/storage/CMakeLists.txt`).
##########
be/src/storage/CMakeLists.txt:
##########
@@ -29,6 +29,14 @@ file(GLOB_RECURSE SRC_FILES CONFIGURE_DEPENDS *.cpp)
# and linked by Storage.
list(FILTER SRC_FILES EXCLUDE REGEX ".*/storage/index/ann/.*\\.cpp$")
+if (ENABLE_VARIANT_NESTED_GROUP)
+ list(REMOVE_ITEM SRC_FILES
+
"${CMAKE_CURRENT_SOURCE_DIR}/segment/variant/nested_group_provider.cpp")
+ file(GLOB_RECURSE VARIANT_NESTED_GROUP_SOURCES CONFIGURE_DEPENDS
+
"${CMAKE_CURRENT_SOURCE_DIR}/../${VARIANT_NESTED_GROUP_MODULE_DIR}/*.cpp")
+ list(APPEND SRC_FILES ${VARIANT_NESTED_GROUP_SOURCES})
+endif()
Review Comment:
`list(REMOVE_ITEM ...)` fails silently if the path doesn’t exactly match an
entry in `SRC_FILES` (e.g., file moved/renamed), which can leave the default
provider compiled alongside extension sources and create hard-to-debug
ODR/duplicate symbol issues. Consider making this removal more robust (e.g.,
`list(FILTER SRC_FILES EXCLUDE REGEX ".*/nested_group_provider\\.cpp$")`) or
explicitly erroring when the provider source isn’t found in `SRC_FILES` under
`ENABLE_VARIANT_NESTED_GROUP`.
--
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]
