Skip to content

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

Open
leftmain wants to merge 2 commits intoClickHouse:masterfrom
leftmain:fix-executable-table-function-parse-error
Open

Fix unhandled exception in executable table function for invalid escape sequences#101819
leftmain wants to merge 2 commits intoClickHouse:masterfrom
leftmain:fix-executable-table-function-parse-error

Conversation

@leftmain
Copy link
Copy Markdown
Contributor

@leftmain leftmain commented Apr 5, 2026

#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

@alexey-milovidov
Copy link
Copy Markdown
Member

This isn't a bug fix, it just changes one exception type (std::exception, boost::exception, which we don't expect, and prefer not to use) to another (which we expect). There was no unhandled exception, and there was no bug.

It does not fix the issue:

rejects unrecognized backslash escape sequences that were previously treated as literal characters by boost::split

Comment thread src/TableFunctions/TableFunctionExecutable.cpp Outdated
Copy link
Copy Markdown
Member

@alexey-milovidov alexey-milovidov left a comment

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
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Apr 7, 2026

Workflow [PR], commit [4af9dd1]

Summary:

job_name test_name status info comment
Stateless tests (arm_asan_ubsan, flaky check) failure
00746_sql_fuzzy FAIL cidb
Stateless tests (amd_asan_ubsan, flaky check) failure
00746_sql_fuzzy FAIL cidb
Stateless tests (amd_tsan, flaky check) failure
01355_CSV_input_format_allow_errors FAIL cidb
01355_CSV_input_format_allow_errors FAIL cidb
01355_CSV_input_format_allow_errors FAIL cidb

AI Review

Summary

This PR converts an unhandled parser exception path in the executable table function into a regular ClickHouse BAD_ARGUMENTS exception and adds an integration test for invalid escape sequences in script arguments. The change is small, targeted, and addresses correctness/robustness in error handling without introducing obvious regressions.

ClickHouse Rules
Item Status Notes
Deletion logging
Serialization versioning
Core-area scrutiny
No test removal
Experimental gate
No magic constants
Backward compatibility
SettingsChangesHistory.cpp
PR metadata quality
Safe rollout
Compilation time
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
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Apr 7, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 84.00% 84.10% +0.10%
Functions 90.90% 90.90% +0.00%
Branches 76.60% 76.60% +0.00%

Changed lines: 86.67% (13/15) | lost baseline coverage: 1 line(s) · Uncovered code

Full report · Diff report

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants