diff --git a/CHANGELOG.md b/CHANGELOG.md index f9304118..df3441a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Parsing a nested `Variant`, `Tuple`, `Nested`, or typed `JSON` column type whose element is an `Enum` with an escaped single quote in a value name no longer corrupts the escape sequence and fails while re-parsing the element type. Closes [#878](https://github.com/ClickHouse/clickhouse-connect/issues/878). - `None` nested inside an `Array` or `Tuple`, or inside a `Map` when `dict_parameter_format="map"`, now renders as the SQL `NULL` keyword instead of the `\N` sentinel used for top-level values. Top-level scalar `None` binds are unchanged. Closes [#879](https://github.com/ClickHouse/clickhouse-connect/issues/879). - Inserting empty bytes `b""` into a non-nullable `FixedString(N)` column now zero-pads to N bytes instead of raising `DataError`, matching the existing string and nullable-bytes write paths. Closes [#880](https://github.com/ClickHouse/clickhouse-connect/issues/880). +- Per-query and client settings that are not present in `system.settings` for the current user (including custom settings declared `CHANGEABLE_IN_READONLY` on a role) are now forwarded to ClickHouse instead of raising `ProgrammingError: Setting ... is unknown or readonly`. The client cannot discover those settings without extra privileges, so the server is treated as authoritative. Setting `invalid_setting_action` to `drop` still drops them, so a single settings dict stays portable across server versions. Known readonly settings still honor `invalid_setting_action`, and reserved HTTP request parameter names such as `query`, `user`, `default_format`, and the `param_` bound-parameter namespace still raise a client-side `ProgrammingError` because they are not settings. Closes [#530](https://github.com/ClickHouse/clickhouse-connect/issues/530). ## 1.6.0, 2026-07-23 diff --git a/clickhouse_connect/driver/_backend/httpcommon.py b/clickhouse_connect/driver/_backend/httpcommon.py index 5cab6880..223ac871 100644 --- a/clickhouse_connect/driver/_backend/httpcommon.py +++ b/clickhouse_connect/driver/_backend/httpcommon.py @@ -395,7 +395,13 @@ def plan_command_request( transport_settings: dict[str, str] | None, ) -> CommandRequestPlan: """Shape an already-bound command into an HTTP request plan.""" - params = dict(bind_params) + # Settings go in first so the structural request parameters below (bind params, external + # data metadata, query) win any name collision, matching plan_query_request. + params: dict[str, str] = {} + if runtime.database: + params["database"] = runtime.database + params.update(runtime.settings) + params.update(bind_params) headers: dict[str, Any] = {} payload: str | bytes | None = None form_files = None @@ -418,9 +424,6 @@ def plan_command_request( params["query"] = bound_cmd else: payload = bound_cmd - if runtime.database: - params["database"] = runtime.database - params.update(runtime.settings) headers = dict_copy(headers, transport_settings) method = "POST" if payload or form_files else "GET" return CommandRequestPlan(params, headers, method, payload=payload, form_files=form_files) diff --git a/clickhouse_connect/driver/asyncclient.py b/clickhouse_connect/driver/asyncclient.py index 25e3f1b9..094c6271 100644 --- a/clickhouse_connect/driver/asyncclient.py +++ b/clickhouse_connect/driver/asyncclient.py @@ -41,7 +41,13 @@ bind_query, use_form_encoding, # noqa: F401 (compatibility re-export) ) -from clickhouse_connect.driver.client import _INTERNAL_QUERY_FORMATS, Client, _apply_arrow_tz_policy +from clickhouse_connect.driver.client import ( + _HTTP_RESERVED_SETTING_NAMES, + _HTTP_RESERVED_SETTING_PREFIXES, + _INTERNAL_QUERY_FORMATS, + Client, + _apply_arrow_tz_policy, +) from clickhouse_connect.driver.common import ( StreamContext, coerce_bool, @@ -107,6 +113,8 @@ class AsyncClient(Client): "http_headers_progress_interval_ms", "enable_http_compression", } + _reserved_setting_names = set(_HTTP_RESERVED_SETTING_NAMES) + _reserved_setting_prefixes = _HTTP_RESERVED_SETTING_PREFIXES def __init__( self, diff --git a/clickhouse_connect/driver/client.py b/clickhouse_connect/driver/client.py index 42ab4e02..96f7304e 100644 --- a/clickhouse_connect/driver/client.py +++ b/clickhouse_connect/driver/client.py @@ -77,6 +77,15 @@ # user-configured global read formats such as set_default_formats("String", "bytes"). _INTERNAL_QUERY_FORMATS = {"String": "string"} +# Names the ClickHouse HTTP interface consumes as request parameters, not query settings. +# The subset already carried in valid_transport_settings (database, role, query_id, ...) is +# forwarded on purpose; these remaining names have no meaning as settings and would corrupt +# the request if placed in the query string, so they are rejected rather than forwarded. +_HTTP_RESERVED_SETTING_NAMES = frozenset({"query", "user", "password", "default_format", "stacktrace", "close_session"}) +# Prefixes the HTTP interface reserves. param_ is the bound-parameter namespace emitted by +# bind_query, so a setting named param_x would override an actual query parameter value. +_HTTP_RESERVED_SETTING_PREFIXES = ("param_",) + def _strip_utc_timezone_from_arrow(table: pyarrow.Table) -> pyarrow.Table: """Strip UTC timezone from timestamp columns in Arrow table. @@ -133,6 +142,12 @@ class Client(ABC): _initial_settings: dict[str, Any] | None = None valid_transport_settings: set[str] = set() optional_transport_settings: set[str] = set() + # Names and name prefixes the transport reserves for request parameters rather than query + # settings. They must never be forwarded as a setting even when unknown to system.settings, + # because a transport (e.g. HTTP query params) would treat them as something other than a + # setting and silently corrupt the request. + _reserved_setting_names: set[str] = set() + _reserved_setting_prefixes: tuple[str, ...] = () database = None max_error_message = 0 _tz_source: TzSource = "auto" @@ -244,7 +259,13 @@ def _apply_init_result(self, result: InitializationResult) -> None: def _validate_settings(self, settings: dict[str, Any] | None) -> dict[str, str]: """ - This strips any ClickHouse settings that are not recognized or are read only. + Filter and normalize ClickHouse settings before they are sent to the server. + + Settings known to be readonly on this server are handled according to the + common ``invalid_setting_action`` option. Settings that do not appear in + ``system.settings`` for the current user (for example custom settings made + ``CHANGEABLE_IN_READONLY`` on a role) are forwarded to ClickHouse so the + server can accept or reject them. :param settings: Dictionary of setting name and values :return: A filtered dictionary of settings with values rendered as strings """ @@ -285,16 +306,31 @@ def _validate_setting(self, key: str, value: Any, invalid_action: str) -> str | if setting_def and setting_def.value == str_value: if setting_def.readonly or (current_setting is not None and current_setting == setting_def.value): return None - if setting_def is None or setting_def.readonly: + if setting_def is None: + # Not present in system.settings for this user. May be a custom setting, + # including one made CHANGEABLE_IN_READONLY on a role, which the client cannot + # discover without extra privileges. Forward it and let ClickHouse accept or + # reject it. + if key in self.optional_transport_settings: + return None + if key in self._reserved_setting_names or key.startswith(self._reserved_setting_prefixes): + raise ProgrammingError(f"{key} is a reserved transport parameter and cannot be sent as a setting") from None + if invalid_action == "drop": + # Honor the caller's opt-in to strip settings the client cannot validate, + # which keeps a single settings dict portable across server versions. + logger.warning("Dropping setting %s not found in system.settings", key) + return None + return str_value + if setting_def.readonly: if key in self.optional_transport_settings: return None if invalid_action == "send": - logger.warning("Attempting to send unrecognized or readonly setting %s", key) + logger.warning("Attempting to send readonly setting %s", key) elif invalid_action == "drop": - logger.warning("Dropping unrecognized or readonly settings %s", key) + logger.warning("Dropping readonly setting %s", key) return None else: - raise ProgrammingError(f"Setting {key} is unknown or readonly") from None + raise ProgrammingError(f"Setting {key} is readonly") from None return str_value def _setting_status(self, key: str) -> SettingStatus: @@ -345,9 +381,11 @@ def _query_with_context(self, context: QueryContext) -> QueryResult: @abstractmethod def set_client_setting(self, key: str, value: Any) -> None: """ - Set a clickhouse setting for the client after initialization. If a setting is not recognized by ClickHouse, - or the setting is identified as "read_only", this call will either throw a Programming exception or attempt - to send the setting anyway based on the common setting 'invalid_setting_action' + Set a clickhouse setting for the client after initialization. Settings identified as read only on the + server honor the common setting 'invalid_setting_action', which can throw a ProgrammingError, drop the + setting, or send it anyway. Settings not present in system.settings for the current user (for example a + custom setting made CHANGEABLE_IN_READONLY on a role) are forwarded to ClickHouse, which accepts or + rejects them, unless 'invalid_setting_action' is 'drop', in which case they are dropped. :param key: ClickHouse setting name :param value: ClickHouse setting value """ diff --git a/clickhouse_connect/driver/httpclient.py b/clickhouse_connect/driver/httpclient.py index 3816a34d..6ce99828 100644 --- a/clickhouse_connect/driver/httpclient.py +++ b/clickhouse_connect/driver/httpclient.py @@ -23,6 +23,7 @@ from clickhouse_connect.driver.binding import ( use_form_encoding, # noqa: F401 (compatibility re-export) ) +from clickhouse_connect.driver.client import _HTTP_RESERVED_SETTING_NAMES, _HTTP_RESERVED_SETTING_PREFIXES from clickhouse_connect.driver.common import coerce_bool, coerce_int, dict_add, dict_copy from clickhouse_connect.driver.exceptions import ProgrammingError from clickhouse_connect.driver.httputil import ( @@ -56,6 +57,8 @@ class HttpClient(SyncBackendClient): "role", } optional_transport_settings = {"send_progress_in_http_headers", "http_headers_progress_interval_ms", "enable_http_compression"} + _reserved_setting_names = set(_HTTP_RESERVED_SETTING_NAMES) + _reserved_setting_prefixes = _HTTP_RESERVED_SETTING_PREFIXES _owns_pool_manager = False # R0917: too-many-positional-arguments diff --git a/docs/additional-options.mdx b/docs/additional-options.mdx index 9aa35898..795c612c 100644 --- a/docs/additional-options.mdx +++ b/docs/additional-options.mdx @@ -32,7 +32,7 @@ The following global settings are currently defined: | `autogenerate_session_id` | `True` | `True`, `False` | Generate a UUID session ID for each synchronous client unless a session ID is supplied. The async factory overrides this to `False` by default. | | `autogenerate_query_id` | `True` | `True`, `False` | Generate a UUID query ID for each request unless one is supplied. | | `dict_parameter_format` | `"json"` | `"json"`, `"map"` | Format Python dictionaries used in parameter binding as JSON or ClickHouse map literals. | -| `invalid_setting_action` | `"error"` | `"drop"`, `"send"`, `"error"` | Drop, send, or reject a setting that is unrecognized by the server or is readonly. `"error"` raises `ProgrammingError`. | +| `invalid_setting_action` | `"error"` | `"drop"`, `"send"`, `"error"` | Action for a setting the server reports as readonly. `drop` ignores it, `send` forwards it, `error` raises `ProgrammingError`. Settings absent from `system.settings` for the current user, such as one made `CHANGEABLE_IN_READONLY` on a role, are forwarded so the server can accept or reject them, unless the action is `drop`. | | `max_connection_age` | `600` | Any number of seconds | Maximum age for a reused HTTP keep-alive connection. Rotation helps distribute connections across nodes behind a load balancer. | | `product_name` | `""` | Any string | Product identifier added to client information. Use a value such as `"my-product/1.0"`. | | `readonly` | `0` | `0`, `1` | Compatibility hint for the `readonly` setting on very old ClickHouse servers. | diff --git a/tests/integration_tests/conftest.py b/tests/integration_tests/conftest.py index c42c2710..56eacf1c 100644 --- a/tests/integration_tests/conftest.py +++ b/tests/integration_tests/conftest.py @@ -167,6 +167,8 @@ def client_factory(client_mode, test_config, shared_loop): clients = [] def factory(**kwargs): + # Readonly users cannot set the writable defaults below, so let those tests opt out. + apply_test_settings = kwargs.pop("apply_test_settings", True) config = { "host": test_config.host, "port": test_config.port, @@ -179,28 +181,30 @@ def factory(**kwargs): if client_mode == "sync": client = create_client(**config) - if client.min_version("22.8"): - client.set_client_setting("database_replicated_enforce_synchronous_settings", 1) - if client.min_version("24.8") and (client.min_version("24.12") or not test_config.cloud): - client.set_client_setting("allow_experimental_json_type", 1) - client.set_client_setting("allow_experimental_dynamic_type", 1) - client.set_client_setting("allow_experimental_variant_type", 1) - if test_config.insert_quorum: - client.set_client_setting("insert_quorum", test_config.insert_quorum) - elif test_config.cloud: - client.set_client_setting("select_sequential_consistency", 1) + if apply_test_settings: + if client.min_version("22.8"): + client.set_client_setting("database_replicated_enforce_synchronous_settings", 1) + if client.min_version("24.8") and (client.min_version("24.12") or not test_config.cloud): + client.set_client_setting("allow_experimental_json_type", 1) + client.set_client_setting("allow_experimental_dynamic_type", 1) + client.set_client_setting("allow_experimental_variant_type", 1) + if test_config.insert_quorum: + client.set_client_setting("insert_quorum", test_config.insert_quorum) + elif test_config.cloud: + client.set_client_setting("select_sequential_consistency", 1) else: client = shared_loop.run_until_complete(get_async_client(**config)) - if client.min_version("22.8"): - client.set_client_setting("database_replicated_enforce_synchronous_settings", "1") - if client.min_version("24.8"): - client.set_client_setting("allow_experimental_json_type", "1") - client.set_client_setting("allow_experimental_dynamic_type", "1") - client.set_client_setting("allow_experimental_variant_type", "1") - if test_config.insert_quorum: - client.set_client_setting("insert_quorum", str(test_config.insert_quorum)) - elif test_config.cloud: - client.set_client_setting("select_sequential_consistency", "1") + if apply_test_settings: + if client.min_version("22.8"): + client.set_client_setting("database_replicated_enforce_synchronous_settings", "1") + if client.min_version("24.8"): + client.set_client_setting("allow_experimental_json_type", "1") + client.set_client_setting("allow_experimental_dynamic_type", "1") + client.set_client_setting("allow_experimental_variant_type", "1") + if test_config.insert_quorum: + client.set_client_setting("insert_quorum", str(test_config.insert_quorum)) + elif test_config.cloud: + client.set_client_setting("select_sequential_consistency", "1") clients.append(client) return client diff --git a/tests/integration_tests/test_client.py b/tests/integration_tests/test_client.py index e09840b1..c18811f1 100644 --- a/tests/integration_tests/test_client.py +++ b/tests/integration_tests/test_client.py @@ -496,6 +496,56 @@ def test_role_setting_works(param_client: Client, test_config: TestConfig, clien assert res.result_rows == [([role_limited],)] +def test_changeable_in_readonly_custom_setting( + param_client: Client, test_config: TestConfig, client_factory: Callable, client_mode: str, call +): + """Readonly user can set CHANGEABLE_IN_READONLY custom settings via settings= (issue #530).""" + if test_config.cloud: + pytest.skip("Skipping role test in cloud mode - cannot create custom users") + + # Docker test servers set custom_settings_prefixes=SQL_; skip otherwise. + try: + call(param_client.command, "SELECT 1 SETTINGS SQL_RO_probe_530='ok'") + except DatabaseError: + pytest.skip("Server does not allow SQL_ custom settings (need custom_settings_prefixes)") + + # Roles and users are server global, so the sync and async runs need distinct names to + # stay isolated when xdist runs them in parallel. + role = f"ch_connect_ro_role_530_{client_mode}" + user = f"ch_connect_ro_user_530_{client_mode}" + password = "R7m!pZt9qL#x" + setting = "SQL_RO_my_rls_key" + + try: + call(param_client.command, f"DROP USER IF EXISTS {user}") + call(param_client.command, f"DROP ROLE IF EXISTS {role}") + + call(param_client.command, f"CREATE ROLE {role}") + # CHANGEABLE_IN_READONLY lets a readonly user set the custom setting even though it is + # not visible in system.settings for that user, which is what previously tripped the + # client into rejecting it. This mirrors the row-policy getSetting use case in #530. + call(param_client.command, f"ALTER ROLE {role} SETTINGS {setting} CHANGEABLE_IN_READONLY") + call(param_client.command, f"CREATE USER {user} IDENTIFIED BY '{password}' DEFAULT ROLE {role} SETTINGS readonly = 1") + + # The readonly user cannot set the writable session defaults the factory normally applies, + # so opt out of them. + client = client_factory(username=user, password=password, apply_test_settings=False) + + # Inline SETTINGS clause already works; the settings= path must match. + inline = call(client.query, f"SELECT getSetting('{setting}') AS v SETTINGS {setting}='tenant_1'") + assert inline.result_rows == [("tenant_1",)] + + via_param = call(client.query, f"SELECT getSetting('{setting}') AS v", settings={setting: "tenant_1"}) + assert via_param.result_rows == [("tenant_1",)] + + # A different value takes effect per query, confirming the setting reaches the server. + other = call(client.query, f"SELECT getSetting('{setting}') AS v", settings={setting: "tenant_2"}) + assert other.result_rows == [("tenant_2",)] + finally: + call(param_client.command, f"DROP USER IF EXISTS {user}") + call(param_client.command, f"DROP ROLE IF EXISTS {role}") + + def test_query_id_autogeneration(param_client: Client, test_table_engine: str, call): """Test that query_id is auto-generated for query(), command(), and insert() methods""" result = call(param_client.query, "SELECT 1") diff --git a/tests/unit_tests/test_driver/test_backend_http.py b/tests/unit_tests/test_driver/test_backend_http.py index 641d5cc1..9045988f 100644 --- a/tests/unit_tests/test_driver/test_backend_http.py +++ b/tests/unit_tests/test_driver/test_backend_http.py @@ -221,7 +221,13 @@ def test_plain_command(self): assert result.form_files is None assert result.headers == {} assert result.params == {"param_id": "7", "database": "db1", "max_threads": "4"} - assert list(result.params) == ["param_id", "database", "max_threads"] + # Settings precede the structural params (bind params, external data, query), matching + # plan_query_request so a setting cannot override them. + assert list(result.params) == ["database", "max_threads", "param_id"] + + def test_bind_params_win_over_colliding_setting(self): + result = command_plan(bind_params={"param_id": "13"}, runtime=QueryRuntime(settings={"param_id": "79"})) + assert result.params["param_id"] == "13" def test_str_data_payload(self): result = command_plan("INSERT INTO t FORMAT CSV", data="1\n2\n") @@ -243,9 +249,14 @@ def test_external_data(self): assert result.payload is None assert result.params["query"] == "SELECT count() FROM f1" assert result.params["_f1_format"] == "CSV" - assert list(result.params) == ["_f1_format", "_f1_structure", "query", "database", "max_threads"] + assert list(result.params) == ["database", "max_threads", "_f1_format", "_f1_structure", "query"] assert result.method == "POST" + def test_external_data_params_win_over_colliding_setting(self): + external = make_external_data() + result = command_plan("SELECT count() FROM f1", external_data=external, runtime=QueryRuntime(settings={"_f1_format": "TSV"})) + assert result.params["_f1_format"] == "CSV" + def test_external_data_with_data_raises(self): with pytest.raises(ProgrammingError, match="external data"): command_plan(external_data=make_external_data(), data="1\n") diff --git a/tests/unit_tests/test_driver/test_validate_settings.py b/tests/unit_tests/test_driver/test_validate_settings.py new file mode 100644 index 00000000..42f5c3fa --- /dev/null +++ b/tests/unit_tests/test_driver/test_validate_settings.py @@ -0,0 +1,164 @@ +"""Unit tests for Client._validate_settings / _validate_setting. + +Covers readonly vs unknown (custom) settings, including the CHANGEABLE_IN_READONLY +case from issue #530 where custom settings are not present in system.settings for +the connecting user, plus the reserved transport parameter names that must never be +forwarded as settings. +""" + +from __future__ import annotations + +from unittest.mock import patch + +import pytest + +from clickhouse_connect import common +from clickhouse_connect.driver.exceptions import ProgrammingError +from clickhouse_connect.driver.httpclient import HttpClient +from clickhouse_connect.driver.models import SettingDef + + +@pytest.fixture +def client_with_settings(): + """Build HttpClients whose server settings are stubbed, closing them on teardown.""" + clients: list[HttpClient] = [] + + def factory(server_settings: dict[str, SettingDef]) -> HttpClient: + with patch.object(HttpClient, "_init_common_settings", autospec=True): + client = HttpClient( + interface="http", + host="localhost", + port=8123, + username="default", + password="", + database="default", + ) + client.server_settings = server_settings + clients.append(client) + return client + + yield factory + + for client in clients: + client.close() + + +@pytest.fixture(autouse=True) +def _restore_invalid_setting_action(): + original = common.get_setting("invalid_setting_action") + try: + yield + finally: + common.set_setting("invalid_setting_action", original) + + +def test_unknown_custom_setting_is_sent_by_default(client_with_settings): + """Custom settings absent from system.settings must be forwarded (issue #530).""" + client = client_with_settings({}) + common.set_setting("invalid_setting_action", "error") + + assert client._validate_settings({"SQL_RO_my_rls_key": "tenant_1"}) == {"SQL_RO_my_rls_key": "tenant_1"} + + +def test_unknown_custom_setting_is_sent_with_send_action(client_with_settings): + client = client_with_settings({}) + common.set_setting("invalid_setting_action", "send") + + assert client._validate_settings({"SQL_RO_my_rls_key": "tenant_1"}) == {"SQL_RO_my_rls_key": "tenant_1"} + + +def test_unknown_custom_setting_is_dropped_with_drop_action(client_with_settings, caplog): + """invalid_setting_action='drop' keeps a settings dict portable across server versions.""" + client = client_with_settings({}) + common.set_setting("invalid_setting_action", "drop") + + with caplog.at_level("WARNING"): + validated = client._validate_settings({"SQL_RO_my_rls_key": "tenant_1"}) + + assert validated == {} + assert any("SQL_RO_my_rls_key" in r.message for r in caplog.records) + + +def test_known_writable_setting_is_sent(client_with_settings): + client = client_with_settings({"max_threads": SettingDef("max_threads", "8", 0)}) + common.set_setting("invalid_setting_action", "error") + + assert client._validate_settings({"max_threads": "4"}) == {"max_threads": "4"} + + +def test_known_readonly_setting_raises_by_default(client_with_settings): + client = client_with_settings({"readonly": SettingDef("readonly", "1", 1)}) + common.set_setting("invalid_setting_action", "error") + + with pytest.raises(ProgrammingError, match="Setting readonly is readonly"): + client._validate_settings({"readonly": "0"}) + + +def test_known_readonly_matching_value_is_skipped(client_with_settings): + """Matching readonly values are not re-sent (issue #469 / #639 behavior).""" + client = client_with_settings({"readonly": SettingDef("readonly", "1", 1)}) + common.set_setting("invalid_setting_action", "error") + + assert client._validate_settings({"readonly": "1"}) == {} + + +def test_known_readonly_send_action_forwards_with_warning(client_with_settings, caplog): + client = client_with_settings({"max_memory_usage": SettingDef("max_memory_usage", "0", 1)}) + common.set_setting("invalid_setting_action", "send") + + with caplog.at_level("WARNING"): + validated = client._validate_settings({"max_memory_usage": "1000"}) + + assert validated == {"max_memory_usage": "1000"} + assert any("readonly setting max_memory_usage" in r.message for r in caplog.records) + + +def test_known_readonly_drop_action_drops(client_with_settings, caplog): + client = client_with_settings({"max_memory_usage": SettingDef("max_memory_usage", "0", 1)}) + common.set_setting("invalid_setting_action", "drop") + + with caplog.at_level("WARNING"): + validated = client._validate_settings({"max_memory_usage": "1000"}) + + assert validated == {} + assert any("Dropping readonly setting max_memory_usage" in r.message for r in caplog.records) + + +@pytest.mark.parametrize( + "reserved", + ["query", "user", "password", "default_format", "stacktrace", "close_session", "param_id", "param_"], +) +def test_reserved_transport_name_raises(client_with_settings, reserved): + """Reserved HTTP request parameters are never forwarded as settings, even when unknown. + + param_* is the bound-parameter namespace, so a setting there would override a real query parameter. + """ + client = client_with_settings({}) + common.set_setting("invalid_setting_action", "error") + + with pytest.raises(ProgrammingError, match="reserved transport parameter"): + client._validate_settings({reserved: "x"}) + + +@pytest.mark.parametrize("action", ["drop", "send"]) +def test_reserved_transport_name_raises_regardless_of_action(client_with_settings, action): + """A reserved name cannot be dropped or sent, since forwarding it would corrupt the request.""" + client = client_with_settings({}) + common.set_setting("invalid_setting_action", action) + + with pytest.raises(ProgrammingError, match="reserved transport parameter"): + client._validate_settings({"query": "SELECT 1"}) + + +def test_optional_transport_setting_unknown_is_dropped(client_with_settings): + client = client_with_settings({}) + common.set_setting("invalid_setting_action", "error") + + assert client._validate_settings({"enable_http_compression": "1"}) == {} + + +def test_transport_settings_always_pass_through(client_with_settings): + client = client_with_settings({}) + common.set_setting("invalid_setting_action", "error") + + assert client._validate_settings({"query_id": "q-530"}) == {"query_id": "q-530"}