Use native parameter approach by default (#277)

Jesse · web-flow · commit 9923c74e324f · 2023-11-10T18:35:24.000-05:00
Signed-off-by: Jesse Whitehouse &lt;jesse.whitehouse@databricks.com&gt;
diff --git a/src/databricks/sql/__init__.py b/src/databricks/sql/__init__.py
@@ -6,9 +6,7 @@
 apilevel = "2.0"
 threadsafety = 1  # Threads may share the module, but not connections.
 
-# Python extended format codes, e.g. ...WHERE name=%(name)s
-# Note that when we switch to ParameterApproach.NATIVE, paramstyle will be `named`
-paramstyle = "pyformat"
+paramstyle = "named"
 
 
 class DBAPITypeObject(object):
diff --git a/src/databricks/sql/client.py b/src/databricks/sql/client.py
@@ -63,24 +63,26 @@ def __init__(
                 Http Bearer access token, e.g. Databricks Personal Access Token.
                 Unless if you use auth_type=`databricks-oauth` you need to pass `access_token.
                 Examples:
+                        ```
                          connection = sql.connect(
                             server_hostname='dbc-12345.staging.cloud.databricks.com',
                             http_path='sql/protocolv1/o/6789/12abc567',
                             access_token='dabpi12345678'
                          )
+                        ```
             :param http_headers: An optional list of (k, v) pairs that will be set as Http headers on every request
             :param session_configuration: An optional dictionary of Spark session parameters. Defaults to None.
                 Execute the SQL command `SET -v` to get a full list of available commands.
             :param catalog: An optional initial catalog to use. Requires DBR version 9.0+
             :param schema: An optional initial schema to use. Requires DBR version 9.0+
 
         Other Parameters:
-            use_inline_params: `boolean`, optional (default is True)
+            use_inline_params: `boolean` | str, optional (default is False)
                 When True, parameterized calls to cursor.execute() will try to render parameter values inline with the
                 query text instead of using native bound parameters supported in DBR 14.1 and above. This connector will attempt to
-                sanitise parameterized inputs to prevent SQL injection. Before you can switch this to False, you must
-                update your queries to use the PEP-249 `named` paramstyle instead of the `pyformat` paramstyle used
-                in INLINE mode.
+                sanitise parameterized inputs to prevent SQL injection.  The inline parameter approach is maintained for
+                legacy purposes and will be deprecated in a future release. When this parameter is `True` you will see
+                a warning log message. To suppress this log message, set `use_inline_params="silent"`.
             auth_type: `str`, optional
                 `databricks-oauth` : to use oauth with fine-grained permission scopes, set to `databricks-oauth`.
                 This is currently in private preview for Databricks accounts on AWS.
@@ -128,6 +130,7 @@ def read(self) -> Optional[OAuthToken]:
                 own implementation of OAuthPersistence.
 
                 Examples:
+                ```
                         # for development only
                         from databricks.sql.experimental.oauth_persistence import DevOnlyFilePersistence
 
@@ -137,6 +140,7 @@ def read(self) -> Optional[OAuthToken]:
                             auth_type="databricks-oauth",
                             experimental_oauth_persistence=DevOnlyFilePersistence("~/dev-oauth.json")
                         )
+                ```
 
 
         """
@@ -223,8 +227,36 @@ def read(self) -> Optional[OAuthToken]:
         logger.info("Successfully opened session " + str(self.get_session_id_hex()))
         self._cursors = []  # type: List[Cursor]
 
-        self._suppress_inline_warning = "use_inline_params" in kwargs
-        self.use_inline_params = kwargs.get("use_inline_params", True)
+        self.use_inline_params = self._set_use_inline_params_with_warning(
+            kwargs.get("use_inline_params", False)
+        )
+
+    def _set_use_inline_params_with_warning(self, value: Union[bool, str]):
+        """Valid values are True, False, and "silent"
+
+        False: Use native parameters
+        True: Use inline parameters and log a warning
+        "silent": Use inline parameters and don't log a warning
+        """
+
+        if value is False:
+            return False
+
+        if value not in [True, "silent"]:
+            raise ValueError(
+                f"Invalid value for use_inline_params: {value}. "
+                + 'Valid values are True, False, and "silent"'
+            )
+
+        if value is True:
+            logger.warning(
+                "Parameterised queries executed with this client will use the inline parameter approach."
+                "This approach will be deprecated in a future release. Consider using native parameters."
+                "Learn more: https://github.com/databricks/databricks-sql-python/tree/main/docs/parameters.md"
+                'To suppress this warning, set use_inline_params="silent"'
+            )
+
+        return value
 
     def __enter__(self):
         return self
@@ -395,23 +427,7 @@ def _determine_parameter_approach(
         if params is None:
             return ParameterApproach.NONE
 
-        server_supports_native_approach = (
-            self.connection.server_parameterized_queries_enabled(
-                self.connection.protocol_version
-            )
-        )
-
         if self.connection.use_inline_params:
-            if (
-                server_supports_native_approach
-                and not self.connection._suppress_inline_warning
-            ):
-                logger.warning(
-                    "This query will be executed with inline parameters."
-                    "Consider using native parameters."
-                    "Learn more: https://github.com/databricks/databricks-sql-python/tree/main/docs/parameters.md"
-                    "To suppress this warning, pass use_inline_params=True when creating the connection."
-                )
             return ParameterApproach.INLINE
 
         else:
@@ -635,7 +651,7 @@ def execute(
 
         This behaviour is controlled by the `use_inline_params` argument passed when building a connection.
 
-        The syntax for these approaches is different:
+        The paramstyle for these approaches is different:
 
         If the connection was instantiated with use_inline_params=False, then parameters
         should be given in PEP-249 `named` paramstyle like :param_name
diff --git a/tests/e2e/test_parameterized_queries.py b/tests/e2e/test_parameterized_queries.py
@@ -271,11 +271,11 @@ def test_use_inline_by_default_with_warning(self, explicit_inline, caplog):
                     cursor.execute("SELECT %(p)s", parameters={"p": 1})
                     if explicit_inline:
                         assert (
-                            "Consider using native parameters." not in caplog.text
+                            "Consider using native parameters." in caplog.text
                         ), "Log message should be suppressed"
                     else:
                         assert (
-                            "Consider using native parameters." in caplog.text
+                            "Consider using native parameters." not in caplog.text
                         ), "Log message should not be supressed"
 
 
