Skip to content

Impl Drivers for User-Defined Functions#77128

Draft
dahbka-lis wants to merge 31 commits intoClickHouse:masterfrom
dahbka-lis:driver-udf
Draft

Impl Drivers for User-Defined Functions#77128
dahbka-lis wants to merge 31 commits intoClickHouse:masterfrom
dahbka-lis:driver-udf

Conversation

@dahbka-lis
Copy link
Copy Markdown

@dahbka-lis dahbka-lis commented Mar 4, 2025

Changelog category (leave one):

  • New Feature

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Expand CREATE FUNCTION query for easy creating User-Defined Functions and its execution by some driver like #71172.
...

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 4, 2025

CLA assistant check
All committers have signed the CLA.

@vdimir vdimir self-assigned this Mar 10, 2025
@vdimir vdimir added the can be tested Allows running workflows for external contributors label Mar 14, 2025
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 14, 2025

Workflow [PR], commit [d1213c9]

@clickhouse-gh clickhouse-gh bot added the pr-feature Pull request with new product feature label Mar 14, 2025

if (execute_direct)
{
boost::split(command_arguments, command_value, [](char c) { return c == ' '; });
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is strange.

const auto & code = query.getFunctionBody();
if (command_arguments.empty())
{
command += " \"" + code + '\"';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work for two reasons:

  1. A limit on the command length.
  2. Different shells may require different escaping.

This needs a solution with temporary files.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double-quoted string in sh or bash is a huge mess, it does interpolation ($var substitution) and even has a special treatment of ! (an obscure feature, I never bothered to check what it is).

namespace
{

String formatCodeBlock(const String & code)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this damages the code, because it can be whitespace-sensitive.

continue;

line.erase(0, min_tabs_count);
boost::replace_all(line, "\"", "\\\"");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is obviously incorrect because you also have to escape backslashes.

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Jun 24, 2025

Dear @vdimir, this PR hasn't been updated for a while. You will be unassigned. Will you continue working on it? If so, please feel free to reassign yourself.

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Aug 13, 2025

Workflow [PR], commit [f316061]

Summary:
15 failures out of 108 shown:

job_name test_name status info comment
Style check failure
cpp failure
various failure
Fast test failure
Build ClickHouse failure
Build (arm_tidy) failure
Build ClickHouse failure
Build (amd_debug) dropped
Build (amd_release) dropped
Build (amd_asan) dropped
Build (amd_tsan) dropped
Build (amd_msan) dropped
Build (amd_ubsan) dropped
Build (amd_binary) dropped
Build (arm_release) dropped
Build (arm_asan) dropped
Build (arm_coverage) dropped
Build (arm_binary) dropped
Build (amd_darwin) dropped

@dahbka-lis dahbka-lis marked this pull request as draft August 14, 2025 08:33
@zoomxi
Copy link
Copy Markdown
Contributor

zoomxi commented Sep 2, 2025

@dahbka-lis Hi, is this PR still ongoing? We really need this feature.

@alexey-milovidov
Copy link
Copy Markdown
Member

The coursework has finished, but the feature request remains, and this pull request has to be taken over.

@dahbka-lis
Copy link
Copy Markdown
Author

@dahbka-lis Hi, is this PR still ongoing? We really need this feature.

Hi @zoomxi @alexey-milovidov! I would like to refactor UserDefined folder to merge the factories and resolve issues above, I've already discussed this with @ vdimir. Now I'm trying to find time for this. Is there any deadline to complete it?

@alexey-milovidov
Copy link
Copy Markdown
Member

No deadline.

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Nov 4, 2025

Dear @dahbka-lis, this PR hasn't been updated for a while. Will you continue working on it? If not, please close it. Otherwise, ignore this message.

1 similar comment
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Jan 6, 2026

Dear @dahbka-lis, this PR hasn't been updated for a while. Will you continue working on it? If not, please close it. Otherwise, ignore this message.

@dahbka-lis
Copy link
Copy Markdown
Author

Dear @dahbka-lis, this PR hasn't been updated for a while. Will you continue working on it? If not, please close it. Otherwise, ignore this message.

wip

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 10, 2026

Dear @dahbka-lis, this PR hasn't been updated for a while. Will you continue working on it? If not, please close it. Otherwise, ignore this message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

can be tested Allows running workflows for external contributors pr-feature Pull request with new product feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants