Skip to content

Accept string values for allow_experimental_analyzer/enable_analyzer#86109

Merged
azat merged 1 commit intoClickHouse:masterfrom
azat:bool-analyzer-settings
Aug 25, 2025
Merged

Accept string values for allow_experimental_analyzer/enable_analyzer#86109
azat merged 1 commit intoClickHouse:masterfrom
azat:bool-analyzer-settings

Conversation

@azat
Copy link
Copy Markdown
Member

@azat azat commented Aug 25, 2025

All settings can be passed as strings except for this one, due to manual check in the code, which leads to "Bad get: has String, requested UInt64." before this patch.

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Fixes: azat/chdig#146

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Aug 25, 2025 •
edited
Loading

Workflow [PR], commit [3351274]

Summary: ❌

job_name test_name status info comment
Stateless tests (amd_binary, old analyzer, s3 storage, DatabaseReplicated, parallel) failure
02443_detach_attach_partition FAIL
Stateless tests (amd_debug, sequential) failure
01150_ddl_guard_rwr FAIL

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Aug 25, 2025
tuanpach
tuanpach reviewed Aug 25, 2025
View reviewed changes
@azat azat force-pushed the bool-analyzer-settings branch 2 times, most recently from 9583a58 to 5f8dfe1 Compare August 25, 2025 08:28
@azat azat enabled auto-merge August 25, 2025 08:48
@azat
Copy link
Copy Markdown
Member Author

azat commented Aug 25, 2025

https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=86109&sha=5f8dfe158d7f7b57b810b9f44a26b140b900de61&name_0=PR&name_1=Build%20%28amd_release%29

ERROR: AWS API Call Precondition Failed
  | awc error: An error occurred (PreconditionFailed) when calling the PutObject operation: At least one of the pre-conditions you specified did not hold

@maxknv what was the precondition? can you PTAL?

@azat
Accept string values for allow_experimental_analyzer/enable_analyzer
3351274 
All settings can be passed as strings except for this one, due to manual
check in the code, which leads to "Bad get: has String, requested
UInt64." before this patch.
@azat azat force-pushed the bool-analyzer-settings branch from 5f8dfe1 to 3351274 Compare August 25, 2025 12:12
@maxknv
Copy link
Copy Markdown
Member

maxknv commented Aug 25, 2025

https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=86109&sha=5f8dfe158d7f7b57b810b9f44a26b140b900de61&name_0=PR&name_1=Build%20%28amd_release%29

ERROR: AWS API Call Precondition Failed
  | awc error: An error occurred (PreconditionFailed) when calling the PutObject operation: At least one of the pre-conditions you specified did not hold

@maxknv what was the precondition? can you PTAL?

That's normal. Multiple jobs are updating the overall CI report concurrently.

@azat
Copy link
Copy Markdown
Member Author

azat commented Aug 25, 2025

But it fails CI, let's add retries at least?

@azat
Copy link
Copy Markdown
Member Author

azat commented Aug 25, 2025

CI issues are known

@azat azat added this pull request to the merge queue Aug 25, 2025
Merged via the queue into ClickHouse:master with commit 469f638 Aug 25, 2025
120 of 122 checks passed
@azat azat deleted the bool-analyzer-settings branch August 25, 2025 17:26
@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-synced-to-cloud The PR is synced to the cloud repo label Aug 25, 2025
@maxknv
Copy link
Copy Markdown
Member

maxknv commented Aug 26, 2025 •
edited
Loading

https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=86109&sha=5f8dfe158d7f7b57b810b9f44a26b140b900de61&name_0=PR&name_1=Build%20%28amd_release%29

ERROR: AWS API Call Precondition Failed
  | awc error: An error occurred (PreconditionFailed) when calling the PutObject operation: At least one of the pre-conditions you specified did not hold

@maxknv what was the precondition? can you PTAL?

This is the reason for the job error:
Timeout exceeded [7200], send SIGTERM to [2583] and give a chance for graceful termination

I have an issue to fix the timeout handling to make this error visible #85444

@azat
Copy link
Copy Markdown
Member Author

azat commented Aug 26, 2025

@maxknv thank you. Let's maybe increase timeout for LTO builds temporary (to i.e. 3h) ?

azat added a commit to azat/chdig that referenced this pull request Nov 27, 2025
@azat
Allow to pass SETTINGS to SQLView of arbitrary type
0395d28 
Otherwise you cannot change allow_experimental_analyzer in some
versions, it has been fixed only in [1]

  [1]: ClickHouse/ClickHouse#86109
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tuanpach tuanpach tuanpach approved these changes

Assignees

@tuanpach tuanpach

Labels

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Edit and run query - Bad get: has String, requested UInt64

4 participants

@azat @maxknv @tuanpach @robot-ch-test-poll4