Skip to content

fix: Propagate context on internal casts#88929

Merged
Avogar merged 18 commits intoClickHouse:masterfrom
raimannma:fix/type_json_skip_duplicated_paths
Nov 10, 2025
Merged

fix: Propagate context on internal casts#88929
Avogar merged 18 commits intoClickHouse:masterfrom
raimannma:fix/type_json_skip_duplicated_paths

Conversation

@raimannma
Copy link
Copy Markdown
Contributor

@raimannma raimannma commented Oct 23, 2025 •
edited by clickhouse-gh bot
Loading

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

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

Propagate context on internal casts. Fixing multiple issues where cast-settings weren't propagated. Closes #88873. Closes #78025.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@alexey-milovidov alexey-milovidov added the can be tested Allows running workflows for external contributors label Oct 23, 2025
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Oct 23, 2025 •
edited
Loading

Workflow [PR], commit [2ae9fd1]

Summary: ❌

job_name test_name status info comment
Stateless tests (amd_debug, parallel) failure
03212_variant_dynamic_cast_or_default FAIL cidb, flaky
Integration tests (amd_asan, old analyzer, 3/6) failure
test_s3_access_headers/test.py::test_custom_access_header[test_access_over_custom_header] FAIL cidb
Integration tests (amd_binary, 5/5) failure
test_s3_access_headers/test.py::test_custom_access_header[test_access_over_custom_header] FAIL cidb
Integration tests (arm_binary, distributed plan, 4/4) failure
test_s3_access_headers/test.py::test_custom_access_header[test_access_over_custom_header] FAIL cidb
Integration tests (amd_tsan, 3/6) failure
test_s3_access_headers/test.py::test_custom_access_header[test_access_over_custom_header] FAIL cidb

@clickhouse-gh clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label Oct 23, 2025
alexey-milovidov
alexey-milovidov reviewed Oct 23, 2025
View reviewed changes
@Avogar Avogar self-assigned this Oct 23, 2025
Avogar
Avogar reviewed Oct 24, 2025
View reviewed changes
@Avogar
Copy link
Copy Markdown
Member

Avogar commented Oct 24, 2025 •
edited
Loading

It should also fix #78025, please, add a test from this issue and make sure it passes

@UnamedRus
Copy link
Copy Markdown
Contributor

UnamedRus commented Oct 24, 2025

If duplicated paths dedup setting will be respected during insert, may be it will fix as well? #82206

@Avogar
Copy link
Copy Markdown
Member

Avogar commented Oct 24, 2025 •
edited
Loading

If duplicated paths dedup setting will be respected during insert, may be it will fix as well? #82206

In this particular issue error happens during merges, and I believe it happens during binary deserialization of a nested Array(JSON) from shared data where we always use type_json_skip_duplicated_paths = 0, because duplicated paths there are unexpected (they should not happen during inserts so only some bug could lead to it).

@raimannma raimannma changed the title fix: Respect type_json_skip_duplicated_paths in INSERT SELECT fix: Propagate context on internal casts Oct 24, 2025
@raimannma raimannma force-pushed the fix/type_json_skip_duplicated_paths branch from bf38592 to f5726ad Compare October 24, 2025 11:28
Avogar
Avogar reviewed Oct 24, 2025
View reviewed changes
Avogar
Avogar reviewed Oct 24, 2025
View reviewed changes
Avogar
Avogar reviewed Oct 24, 2025
View reviewed changes
Avogar
Avogar reviewed Oct 24, 2025
View reviewed changes
Avogar
Avogar reviewed Oct 24, 2025
View reviewed changes
Avogar
Avogar reviewed Oct 24, 2025
View reviewed changes
@raimannma raimannma force-pushed the fix/type_json_skip_duplicated_paths branch 4 times, most recently from 9450c70 to 8d96d4c Compare October 29, 2025 15:12
@raimannma raimannma force-pushed the fix/type_json_skip_duplicated_paths branch from 8d96d4c to b665dc0 Compare October 29, 2025 15:58
Avogar
Avogar reviewed Oct 30, 2025
View reviewed changes
Avogar
Avogar reviewed Oct 30, 2025
View reviewed changes
raimannma and others added 2 commits October 30, 2025 14:59
@raimannma @Avogar
Update src/Functions/CastOverloadResolver.cpp
32b2a1b 
Co-authored-by: Pavel Kruglov <48961922+Avogar@users.noreply.github.com>
@raimannma
Remove unnecessary context copy
f2a75bb
Avogar
Avogar reviewed Oct 30, 2025
View reviewed changes
Avogar
Avogar reviewed Oct 30, 2025
View reviewed changes
Avogar
Avogar reviewed Oct 30, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@Avogar Avogar left a comment

Choose a reason for hiding this comment

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

Great work, thanks! Just a few minor comments and let's merge it

@raimannma
Copy link
Copy Markdown
Contributor Author

raimannma commented Oct 31, 2025

Thanks for the fixes!

Can be merged or anything left to do?

@Avogar
Copy link
Copy Markdown
Member

Avogar commented Oct 31, 2025

Let's merge it right away!

@Avogar Avogar enabled auto-merge October 31, 2025 18:08
@raimannma
Copy link
Copy Markdown
Contributor Author

raimannma commented Nov 7, 2025

03212_variant_dynamic_cast_or_default is flaky #87583

@Avogar
Copy link
Copy Markdown
Member

Avogar commented Nov 10, 2025

Integration tests: #89758

@Avogar Avogar added this pull request to the merge queue Nov 10, 2025
Merged via the queue into ClickHouse:master with commit 3e9556d Nov 10, 2025
125 of 131 checks passed
@robot-clickhouse robot-clickhouse added the pr-synced-to-cloud The PR is synced to the cloud repo label Nov 10, 2025
@raimannma raimannma deleted the fix/type_json_skip_duplicated_paths branch November 11, 2025 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexey-milovidov alexey-milovidov alexey-milovidov left review comments

@Avogar Avogar Avogar approved these changes

Assignees

@Avogar Avogar

Labels

can be tested Allows running workflows for external contributors pr-bugfix Pull request with bugfix, not backported by default pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

type_json_skip_duplicated_paths didn't work Format settings are not respected during internal CAST

5 participants

@raimannma @Avogar @UnamedRus @alexey-milovidov @robot-clickhouse