Copilot commented on code in PR #2066:
URL: https://github.com/apache/cloudberry/pull/2066#discussion_r4143362930
##########
contrib/pax_storage/CMakeLists.txt:
##########
@@ -118,13 +118,30 @@ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/")
include(FindDependencies)
add_subdirectory(src/cpp)
+# yyjson is only needed by the manifest API implementation, which is off by
+# default. It is fetched from its upstream release at configure time rather
+# than vendored as a submodule, so that it stays out of the source release
+# for builds that do not use it.
+set(YYJSON_VERSION "0.12.0" CACHE STRING "yyjson release to fetch for the
manifest API")
+
if(USE_MANIFEST_API AND NOT USE_PAX_CATALOG)
+ include(FetchContent)
+
+ message(STATUS "Fetching yyjson ${YYJSON_VERSION} for the manifest API")
+ FetchContent_Declare(
+ yyjson
+ GIT_REPOSITORY https://github.com/ibireme/yyjson.git
+ GIT_TAG ${YYJSON_VERSION}
+ GIT_SHALLOW TRUE
+ )
+
set(SAVED_BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS})
set(SAVED_CMAKE_INSTALL_SO_NO_EXE ${CMAKE_INSTALL_SO_NO_EXE})
set(BUILD_SHARED_LIBS ON)
set(CMAKE_INSTALL_SO_NO_EXE OFF)
- add_subdirectory(${CMAKE_SOURCE_DIR}/../../dependency/yyjson
${CMAKE_BINARY_DIR}/yyjson_build)
+
+ FetchContent_MakeAvailable(yyjson)
Review Comment:
The project still advertises CMake 3.11 (`CMakeLists.txt:17` and
`src/cpp/CMakeLists.txt:18`), but `FetchContent_MakeAvailable()` was added in
3.14. Configuring with CMake 3.11–3.13 and `USE_MANIFEST_API=ON`,
`USE_PAX_CATALOG=OFF` fails because that command is unavailable. Use the older
`FetchContent_Populate`/`add_subdirectory` pattern for those versions, or raise
and document the minimum version for this configuration.
##########
src/backend/utils/adt/genfile.c:
##########
@@ -680,6 +709,8 @@ pg_file_write_v1_1(PG_FUNCTION_ARGS)
bool replace = PG_GETARG_BOOL(2);
int64 count = 0;
+ requireWriteServerFiles();
Review Comment:
This now permits `pg_write_server_files` members to call the write
functions, but their subsequent filename checks still call
`convert_and_check_filename()`, which bypasses the DataDir/log-directory
restriction only for `pg_read_server_files`. A writer without the read role
cannot write, rename, or unlink a writable absolute path elsewhere on the
server, contrary to the documented write-role capability
(`doc/src/sgml/user-manag.sgml:628-630`). Make the filename checks in all three
write paths recognize the write role without changing read-only APIs, and test
an outside-DataDir path.
--
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]
