Fix unhandled exception in executable table function for invalid escape sequences by leftmain · Pull Request #101819 · ClickHouse/ClickHouse · GitHub
Skip to content

Fix unhandled exception in executable table function for invalid escape sequences#101819

Merged
alexey-milovidov merged 5 commits into
ClickHouse:masterfrom
leftmain:fix-executable-table-function-parse-error
May 18, 2026
Merged

Fix unhandled exception in executable table function for invalid escape sequences#101819
alexey-milovidov merged 5 commits into
ClickHouse:masterfrom
leftmain:fix-executable-table-function-parse-error

Conversation

@leftmain

@leftmain leftmain commented Apr 5, 2026

Copy link
Copy Markdown
Contributor

#101565

Changelog category (leave one):

  • Not for changelog

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

Fix unhandled exception in executable table function for invalid escape sequences

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

N/A: just a bug fix

Version info

  • Merged into: 26.5.1.783

@alexey-milovidov

Copy link
Copy Markdown
Member

Comment thread src/TableFunctions/TableFunctionExecutable.cpp Outdated

@alexey-milovidov alexey-milovidov left a comment

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.

However, this change is good on its own.

@alexey-milovidov alexey-milovidov self-assigned this Apr 7, 2026
@alexey-milovidov alexey-milovidov added the can be tested Allows running workflows for external contributors label Apr 7, 2026
@clickhouse-gh

clickhouse-gh Bot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [f5e143c]

Summary:

job_name test_name status info comment
Stateless tests (amd_llvm_coverage, ParallelReplicas, s3 storage, parallel) FAIL
02180_group_by_lowcardinality FAIL cidb
02345_implicit_transaction NOT_FAILED cidb
02435_rollback_cancelled_queries NOT_FAILED cidb

AI Review

Summary

This PR wraps boost::program_options::split_unix in TableFunctionExecutable::parseArguments and converts boost::escaped_list_error into a DB::Exception with BAD_ARGUMENTS, and adds an integration assertion for that specific error contract. I found no unresolved correctness, safety, or test-evidence issues in the current diff.

Final Verdict
  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh Bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Apr 7, 2026
Comment thread tests/integration/test_executable_table_function/test.py
The new regression check previously used `query_and_get_error`, which only
verifies that some error is returned. Pre-patch behavior already returned
an error, so the test could pass even if the specific mapping to
`BAD_ARGUMENTS` regressed. Tighten the assertion to verify both the
`BAD_ARGUMENTS` error code and the new `Failed to parse script name and
arguments` message introduced by this PR.

Address AI review feedback on ClickHouse#101819

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@alexey-milovidov

Copy link
Copy Markdown
Member

This was fixed by #105146. Let's update the branch.

@clickhouse-gh

clickhouse-gh Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

LLVM Coverage Report

Metric Baseline Current Δ
Lines 84.20% 84.20% +0.00%
Functions 91.40% 91.40% +0.00%
Branches 76.60% 76.60% +0.00%

Changed lines: 90.00% (9/10) · Uncovered code

Full report · Diff report

@alexey-milovidov alexey-milovidov merged commit a47145f into ClickHouse:master May 18, 2026
164 of 167 checks passed
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-synced-to-cloud The PR is synced to the cloud repo label May 19, 2026
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-not-for-changelog This PR should not be mentioned in the changelog pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants