Support query specific attributes for drivers #8266
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
grahamplata
force-pushed
the
gplata/plat-246-ch-templating
branch
from
c9a30c5 to
958c7f7
Compare
Member
Author
|
Updated test instance to support |
begelundmuller
requested changes
Nov 14, 2025
begelundmuller
requested changes
Nov 18, 2025
grahamplata
force-pushed
the
gplata/plat-246-ch-templating
branch
from
453ed05 to
ad8f4cd
Compare
begelundmuller
requested changes
Nov 20, 2025
begelundmuller
requested changes
Nov 21, 2025
grahamplata
force-pushed
the
gplata/plat-246-ch-templating
branch
from
4718a62 to
638585b
Compare
begelundmuller
requested changes
Nov 26, 2025
runtime/drivers/clickhouse/olap.go
Outdated
Comment on lines
133
to
167
| if c.supportSettings { | ||
| if c.config.QuerySettingsOverride != "" { | ||
| stmt.Query += "\n SETTINGS " + c.config.QuerySettingsOverride | ||
| } else { | ||
| stmt.Query += "\n SETTINGS cast_keep_nullable = 1, join_use_nulls = 1, session_timezone = 'UTC', prefer_global_in_and_join = 1, insert_distributed_sync = 1" | ||
| if c.config.QuerySettings != "" { | ||
| stmt.Query += ", " + c.config.QuerySettings | ||
| settings := map[string]any{ | ||
| "cast_keep_nullable": 1, | ||
| "insert_distributed_sync": 1, | ||
| "prefer_global_in_and_join": 1, | ||
| "session_timezone": "UTC", | ||
| "join_use_nulls": 1, | ||
| } | ||
|
|
||
| if c.supportSettings { | ||
| if len(stmt.QueryAttributes) > 0 { | ||
| for k, v := range stmt.QueryAttributes { | ||
| settings[k] = v | ||
| } | ||
| } | ||
|
|
||
| if c.config.QuerySettingsOverride != "" { | ||
| stmt.Query += "\n SETTINGS " + c.config.QuerySettingsOverride | ||
| } else { | ||
| stmt.Query += "\n SETTINGS cast_keep_nullable = 1, join_use_nulls = 1, session_timezone = 'UTC', prefer_global_in_and_join = 1, insert_distributed_sync = 1" | ||
| if c.config.QuerySettings != "" { | ||
| stmt.Query += ", " + c.config.QuerySettings | ||
| } | ||
| } | ||
| } | ||
|
|
||
| ctx = clickhouse.Context(ctx, clickhouse.WithSettings(settings)) |
Contributor
There was a problem hiding this comment.
- It checks
c.supportSettingsafter it has already been checked? - This no longer correctly honors
QuerySettingsOverride, which is supposed to override the default query settings. IfQuerySettingsOverride != "", it should not add the default query settings throughclickhouse.WithSettings. - Now that the default query settings are passed using
clickhouse.WithSettings, it should not also add them to the SQL query (as currently happens on line 160)
Contributor
There was a problem hiding this comment.
@grahamplata After your most recent push, it appears to no longer honor c.config.QuerySettings at all. And it also doesn't address bullet 2 – it still doesn't clear the built-in default settings when QuerySettingsOverride is set.
begelundmuller
approved these changes
Dec 11, 2025
k-anshul
pushed a commit
that referenced
this pull request
Dec 18, 2025
* add query attributes to MetricsViewSpec * add support for query attributes in OLAP driver and executor * add ClickHouse driver and executor tests * protos * add user claims handling in query attribute resolution * implement query attribute validation and add corresponding tests * output the additional query attributes in the query logging * add tests for query attributes * query attribute handling in ClickHouse queries * handle sql prefix * add custom_settings_prefixes to test clickhouse instance * test_ prefix * log query configuration for drivers * support for user attributes in executor * regen protos * self * update tests * review * update regex * query attributes * use context * ctx * dont parse * verify settings get added to query * review and tests * review * Review * Tests * Review --------- Co-authored-by: Benjamin Egelund-Müller <b@egelund-muller.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Support query specific attributes for ClickHouse. This enables end users to enrich queries and leverage our internal routing to our
row_filters.ClickHouse Docs: custom settings
Relates to: #8120 Custom user attributes support
After the above is merged I can leverage the security context for
{{ .user.specific }}attributesChecklist: