-
Notifications
You must be signed in to change notification settings - Fork 111
[PECOBLR-330] Support for complex params #559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
3026b6f
to
37c89b8
Compare
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
@@ -429,7 +429,7 @@ def user_friendly_error_message(self, no_retry_reason, attempt, elapsed): | |||
# Taken from PyHive | |||
class ParamEscaper: | |||
_DATE_FORMAT = "%Y-%m-%d" | |||
_TIME_FORMAT = "%H:%M:%S.%f" | |||
_TIME_FORMAT = "%H:%M:%S.%f %z" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Timezone will not be there for TIMESTAMP_NTZ param.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, would like to know how we've accounted/tested for NTZ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is not there it will be an empty space, there is already a test suite that inserts NTZ and none NTZ and reads back to compare whether it is equal or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vikrantpuppala @shivam2680 There are already existing tests that insert NTZ and non NTZ values and reads back from table to ensure everything is working as expected -
def test_dbsqlparameter_single( |
@@ -429,7 +429,7 @@ def user_friendly_error_message(self, no_retry_reason, attempt, elapsed): | |||
# Taken from PyHive | |||
class ParamEscaper: | |||
_DATE_FORMAT = "%Y-%m-%d" | |||
_TIME_FORMAT = "%H:%M:%S.%f" | |||
_TIME_FORMAT = "%H:%M:%S.%f %z" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, would like to know how we've accounted/tested for NTZ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this not an opt-in feature, can't user disable this in favor of existing behavior?
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
@gopalldb We are just adding support for more types, don't think there is anything to opt in. If the users don't use complex types they won't need this |
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
Description
This PR introduces comprehensive support for complex parameter types (
ARRAY
andMAP
) in the Databricks SQL Python client.Key Changes
ArrayParameter
andMapParameter
dbsql_parameter_from_primitive
is updated to recognize and construct these parameter types, including for nested complex values.TSparkParamValueArgs
which is necessary to be passed as arguements for complex typesTesting
e2e
ARRAY<ARRAY<STRING>>, ARRAY<MAP<STRING,INTEGER>>, MAP<STRING,ARRAY<STRING>>
unit
IGNORE THE FAILING e2e Tests