fix(udf/win): udf c and python plugins for windows#35313
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances Windows compatibility for User Defined Functions (UDFs) by replacing POSIX-specific environment handling with libuv's portable alternatives and adjusting pipe initialization for Windows IOCP requirements. Feedback identifies a missing path separator in the Windows UDF directory creation logic, inconsistent naming conventions for binary plugins on Windows, and a leftover commented-out line that should be removed.
There was a problem hiding this comment.
Pull request overview
This PR adds/extends Windows support for UDF execution by enabling udfd on Windows, adjusting libuv process/pipe spawning to work with Windows overlapped pipes, and removing Windows platform blocks around UDF creation.
Changes:
- Add Windows-compatible spawning of
udfd(libuv pipes/env inheritance, PATH vs LD_LIBRARY_PATH). - Enable Python UDF plugin loading on Windows and adjust UDF body path handling.
- Remove Windows-only rejection of CREATE FUNCTION and fix a Windows-incompatible
__typeof__usage in executor code.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| source/libs/function/src/udfd.c | Windows support for python plugin lib naming, UDF file path formatting, control pipe init, and UDF directory creation. |
| source/libs/function/src/tudf.c | Windows-safe udfd spawn: pipe flags, PATH/LD_LIBRARY_PATH handling, and portable environment inheritance via libuv. |
| source/libs/executor/src/projectoperator.c | Replace __typeof__ with sclIsTaskKilled for Windows compiler compatibility. |
| source/dnode/mnode/impl/src/mndFunc.c | Allow CREATE FUNCTION on Windows by removing a platform block. |
| source/common/src/tglobal.c | Enable tsStartUdfd by default (including Windows). |
Comments suppressed due to low confidence (1)
source/libs/function/src/udfd.c:1776
- The fallback path after taosMkDir failure is hard-coded to "%s/.udf" even on Windows. This is inconsistent with the Windows primary path ("%s.udf") and will flip directory naming depending on whether the first mkdir succeeds. Use the same Windows naming scheme in the fallback branch as well.
if (code != TSDB_CODE_SUCCESS) {
snprintf(global.udfDataDir, PATH_MAX, "%s/.udf", tsTempDir);
code = taosMkDir(global.udfDataDir);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request focuses on improving Windows compatibility for User Defined Functions (UDF). Key changes include replacing POSIX-specific environment handling with libuv's portable uv_os_environ, updating pipe initialization to support Windows IOCP requirements, and enabling UDF functionality on Windows by removing previous platform restrictions. The review feedback identifies several opportunities to improve type safety in tudf.c by using size_t for string lengths and buffer size calculations, which ensures consistency with strlen and prevents potential signedness or overflow issues.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances Windows support for User Defined Functions (UDF) by replacing POSIX-specific environment variable handling with libuv's portable uv_os_environ and adjusting pipe initialization for Windows compatibility. It also refines path construction and library naming across platforms. Feedback was provided regarding the use of the POSIX-specific strncasecmp function within a Windows-specific code block, suggesting the use of _strnicmp for better compatibility with MSVC.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances Windows compatibility and portability for the UDF daemon. Significant changes include replacing POSIX-specific environ usage with libuv's uv_os_environ, adjusting pipe initialization for Windows requirements, and implementing platform-aware library naming and path construction. Review feedback suggests improving the portability of string comparison functions and refining the memory allocation logic for environment variable arrays to ensure long-term robustness.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enables and improves User Defined Function (UDF) support on Windows. Key changes include replacing POSIX-specific environment handling with libuv's portable uv_os_environ, configuring pipes with UV_OVERLAPPED_PIPE and ipc=0 for Windows compatibility, and using platform-native directory separators and library extensions. It also removes previous Windows-specific restrictions on UDF creation. I have no feedback to provide.
Code Review发现 2 个需要处理的问题：
已分别发 inline comment 到对应代码行。 |
| int32_t pipeInitErr = uv_pipe_init(&pData->loop, &pData->ctrlPipe, 0); | ||
| if (pipeInitErr != 0) { | ||
| fnError("uv_pipe_init failed: %s", uv_strerror(pipeInitErr)); | ||
| err = TSDB_CODE_UDF_UV_EXEC_FAILURE; | ||
| goto _OVER; |
| #ifdef WINDOWS | ||
| snprintf(plugin->libPath, PATH_MAX, "%s", "taospyudf.dll"); |
Description
Issue(s)
https://project.feishu.cn/taosdata_td/feature/detail/6876989393
Checklist
Please check the items in the checklist if applicable.