Skip to content
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

Disable non_native_boolean_check_constraint #120

Merged
merged 4 commits into from
Jul 12, 2023

Conversation

bkyryliuk
Copy link
Contributor

@bkyryliuk bkyryliuk commented May 9, 2023

Disable non_native_boolean_check_constraint as it is not supported by databricks.
It is true by default: https://github.com/zzzeek/sqlalchemy/blob/main/lib/sqlalchemy/engine/default.py#L162

Tested via monkeypatching out code

error msg:

File /opt/miniconda3/envs/dev/lib/python3.9/site-packages/databricks/sql/thrift_backend.py:484, in ThriftBackend._check_command_not_in_error_or_closed_state(self, op_handle, get_operations_resp)
    482 if get_operations_resp.operationState == ttypes.TOperationState.ERROR_STATE:
    483     if get_operations_resp.displayMessage:
--> 484         raise ServerOperationError(
    485             get_operations_resp.displayMessage,
    486             {
    487                 "operation-id": op_handle and op_handle.operationId.guid,
    488                 "diagnostic-info": get_operations_resp.diagnosticInfo,
    489             },
    490         )
    491     else:
    492         raise ServerOperationError(
    493             get_operations_resp.errorMessage,
    494             {
   (...)
    497             },
    498         )

DatabaseError: (databricks.sql.exc.ServerOperationError)```

Generated ddl
```CREATE TABLE bogdan.__bogdan_test_1 (
	booli BOOLEAN,
	CHECK (booli IN (0, 1))
-^^^
) USING DELTA```

@susodapop
Copy link
Contributor

The change LGTM. Please sign-off the commit as described in CONTRIBUTING.md

Then you just add a line to every git commit message:

Signed-off-by: Joe Smith <joe.smith@email.com>

Use your real name (sorry, no pseudonyms or anonymous contributions.)
If you set your user.name and user.email git configs, you can sign your commit automatically with git commit -s.

@bkyryliuk bkyryliuk force-pushed the bogdan/fix_boolean_constraints branch from 4cef2c4 to 3fac5ef Compare May 9, 2023 15:11
signed-off-by: Bogdan Kyryliuk <b.kyryliuk@gmail.com>
@bkyryliuk bkyryliuk force-pushed the bogdan/fix_boolean_constraints branch from 3fac5ef to 0672be3 Compare May 10, 2023 09:48
@bkyryliuk
Copy link
Contributor Author

The change LGTM. Please sign-off the commit as described in CONTRIBUTING.md

Then you just add a line to every git commit message:

Signed-off-by: Joe Smith <joe.smith@email.com>

Use your real name (sorry, no pseudonyms or anonymous contributions.)
If you set your user.name and user.email git configs, you can sign your commit automatically with git commit -s.

had a typo in the signature :) fixed it

Copy link
Contributor

@susodapop susodapop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All tests work for me. I need to push some changes to sync this back with main but it will merge today.

Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
@susodapop
Copy link
Contributor

Oh this is actually cool. I had to program something similar into our e2e tests when back-porting support for sqlalchemy==1.3.x. I wonder if I can remove those constraints now. I'm going to check that before I merge.

Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
@susodapop
Copy link
Contributor

Yessssss that worked.

@susodapop susodapop merged commit ec58144 into databricks:main Jul 12, 2023
13 of 14 checks passed
susodapop pushed a commit to unj1m/databricks-sql-python that referenced this pull request Sep 19, 2023
---------
Signed-off-by: Bogdan Kyryliuk <b.kyryliuk@gmail.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Co-authored-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants