Use native parameter approach by default#277
Merged
susodapop merged 27 commits intonative-query-params-stagingfrom Nov 10, 2023
Merged
Use native parameter approach by default#277susodapop merged 27 commits intonative-query-params-stagingfrom
susodapop merged 27 commits intonative-query-params-stagingfrom
Conversation
added 21 commits
November 9, 2023 12:45
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com> squash with two commits back Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
This commit updates the test fixtures so that we automatically run the NATIVE tests with both paramstyles. For parameter features that are new in version 3.0.0 (DbsqlParameter) we don't run with both parameter styles because there is no need for syntax backwards-compatibility when using the new DbsqlParameter class. i.e. Nobody should ever use DbsqlParameter with a ParamStyle.PYFORMAT query Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
We no longer need a separate 2 tests for doubles, as they are handled in the new _eq() function. Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
…ormer Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
- Tests that use DbsqlParam will not run under both paramstyles If a user is writing code with DbsqlParam then they *must* use the named param style - Likewise, if parameters are passed as a sequence then transform_paramstyle will not be called. - Adds tests for the inline ordinal parameter approach Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
transform_paramstyle now needs a dictionary of parameters since it doesn't use regex anymore Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
… not recognised by DBR. Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Without this change, these tests would use the default approach which is now native mode. Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
benc-db
reviewed
Nov 10, 2023
benc-db
reviewed
Nov 10, 2023
src/databricks/sql/client.py
Outdated
| "silent": Use inline parameters and don't log a warning | ||
| """ | ||
|
|
||
| if user_value is None: |
Collaborator
There was a problem hiding this comment.
declare as Option and set default in parameter?
benc-db
reviewed
Nov 10, 2023
added 2 commits
November 10, 2023 17:34
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
added 2 commits
November 10, 2023 17:38
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
susodapop
requested review from
andrefurlan-db,
arikfr and
yunbodeng-db
as code owners
benc-db
approved these changes
Nov 10, 2023
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
This PR updates the connector so that the native parameter approach is enabled by default. It modifies the semantics of
use_inline_paramsso that it now accepts three values:False(default) Native parameter approach will be used.True: Inline parameter approach will be used, and a warning will be issued"silent": Inline parameter approach will be used, and no warning will be logged.It also moves the warning message from the
execute()method (where a warning would be logged on every "offending" query execution) and into the client construtor. This way the warning is logged once. We'd like to be kind to our friends that examine their logs 😉