Skip to content

Fixes chdb-ds: UrlTableFunction.to_sql() emits HEADERS outside url() call#563

Merged
auxten merged 4 commits into
chdb-io:mainfrom
nklmish:fix-url-headers-positional
Apr 30, 2026
Merged

Fixes chdb-ds: UrlTableFunction.to_sql() emits HEADERS outside url() call#563
auxten merged 4 commits into
chdb-io:mainfrom
nklmish:fix-url-headers-positional

Conversation

@nklmish

@nklmish nklmish commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Per the ClickHouse url() table function reference, headers must be the 4th positional argument inside the function call, with structure as the 3rd positional argument (defaulting to 'auto' when not provided — chdb rejects '' with Code: 36 Table structure is empty).

Changes

  • Move headers into sql_params before f"url(...)" construction so they end up inside url() as the 4th positional arg
  • Pad missing format/structure positional slots to 'auto' via sql_params.extend(["'auto'"] * (3 - len(sql_params)))
  • Convert "Key: Value" strings to 'Key'='Value' pairs per ClickHouse's headers() function syntax
  • Support dict input directly (headers={"Authorization": "Bearer xyz"})
  • Existing format/structure handling block is preserved unchanged

Tests

New file datastore/tests/test_url_table_function_headers.py with class TestUrlTableFunctionHeaders (8 methods, matching the existing test_table_functions.py class-based convention):

  • test_to_sql_no_headers_unchanged — regression guard for no-headers case
  • test_to_sql_with_headers_inside_url_call — confirms headers emit inside url()
  • test_to_sql_headers_pad_structure_auto — structure auto-padded when format provided
  • test_to_sql_headers_pad_format_and_structure_auto — both auto-padded when neither provided
  • test_to_sql_headers_dict_input — dict input support
  • test_to_sql_headers_multiple_entries — multiple headers
  • test_to_sql_headers_missing_url_raises — confirms the new headers code path doesn't suppress the existing url-required validation
  • test_to_sql_headers_executes_via_chdb — end-to-end runtime check: the generated SQL parses and executes via chdb.query() against https://httpbin.org/ip (skips gracefully if chdb or network unavailable)

Updated existing test_url_with_headers_list and test_url_with_headers_string in test_table_functions.py — they were asserting the broken output, now assert the correct fixed shape.

Verification

  • All 10,688 existing datastore/tests/ tests PASS (no regressions; 9 skipped, 85 xfailed, 4 xpassed all unchanged from baseline)
  • 8 new tests PASS, including the runtime SQL execution check
  • Empirically verified: fixed SQL executes cleanly against https://httpbin.org/ip with chdb 4.1.6 (Python 3.13, macOS arm64) — returns expected JSON, no parse error

Issue

#564

@CLAassistant

CLAassistant commented Apr 27, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@nklmish nklmish changed the title fix: emit headers() as 4th positional arg of url() table function fixes #564 Apr 27, 2026
@wudidapaopao wudidapaopao self-assigned this Apr 27, 2026
@wudidapaopao

wudidapaopao commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Hi @nklmish, thanks a lot for spotting this issue and providing the fix. Your changes are fully reasonable. I added escape handling, class comments and new test cases. Also fixed the edge case where format_name is empty but structure is set. Previously, the URL table function would ignore the structure parameter in this case.

@nklmish

nklmish commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

Thank you @wudidapaopao for your kind support with adding new test cases and documentation !

@auxten auxten changed the title fixes #564 Fixes chdb-ds: UrlTableFunction.to_sql() emits HEADERS outside url() call Apr 30, 2026
@auxten auxten merged commit 420ff5d into chdb-io:main Apr 30, 2026
6 of 7 checks passed
@nklmish nklmish deleted the fix-url-headers-positional branch June 6, 2026 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants