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

Rebase to 302 #5

Merged
merged 57 commits into from
Jan 31, 2024
Merged

Rebase to 302 #5

merged 57 commits into from
Jan 31, 2024

Conversation

matt-fleming
Copy link
Member

No description provided.

Jesse and others added 30 commits August 31, 2023 09:10
* Updated thrift definitions

Signed-off-by: nithinkdb <[email protected]>

* Tried with a different thrift installation

Signed-off-by: nithinkdb <[email protected]>

* Reverted TCLI to previous

Signed-off-by: nithinkdb <[email protected]>

* Reverted to older thrift

Signed-off-by: nithinkdb <[email protected]>

* Updated version again

Signed-off-by: nithinkdb <[email protected]>

* Upgraded thrift

Signed-off-by: nithinkdb <[email protected]>

* Final commit

Signed-off-by: nithinkdb <[email protected]>

---------

Signed-off-by: nithinkdb <[email protected]>
Signed-off-by: Jim Fulton <[email protected]>
Co-Authored-By: Jesse Whitehouse <[email protected]>
Signed-off-by: Jesse Whitehouse <[email protected]>
* Initial commit

Signed-off-by: nithinkdb <[email protected]>

* Added tsparkparam handling

Signed-off-by: nithinkdb <[email protected]>

* Added basic test

Signed-off-by: nithinkdb <[email protected]>

* Addressed comments

Signed-off-by: nithinkdb <[email protected]>

* Addressed missed comments

Signed-off-by: nithinkdb <[email protected]>

* Resolved comments

---------

Signed-off-by: nithinkdb <[email protected]>
…databricks#229)

* Updated thrift files and added check for protocol version

Signed-off-by: nithinkdb <[email protected]>

* Made error message more clear

Signed-off-by: nithinkdb <[email protected]>

* Changed name of fn

Signed-off-by: nithinkdb <[email protected]>

* Ran linter

Signed-off-by: nithinkdb <[email protected]>

* Update src/databricks/sql/client.py

Co-authored-by: Jesse <[email protected]>

---------

Signed-off-by: nithinkdb <[email protected]>
Co-authored-by: Jesse <[email protected]>
… version" (databricks#237)

Reverts databricks#229 as it causes all of our e2e tests to fail on some versions of DBR.

We'll reimplement the protocol version check in a follow-up.

This reverts commit 241e934.
* Stop skipping TableDDLTest and permanent skip HasIndexTest

We're now in the territory of features that aren't required for sqla2
compat as of pysql==3.0.0 but we may consider adding this in the future.

In this case, table comment reflection needs to be manually implemented.
Index reflection would require hooking into the compiler to reflect
the partition strategy.

test_suite.py::HasIndexTest_databricks+databricks::test_has_index[dialect] SKIPPED (Databricks does not support indexes.)
test_suite.py::HasIndexTest_databricks+databricks::test_has_index[inspector] SKIPPED (Databricks does not support indexes.)
test_suite.py::HasIndexTest_databricks+databricks::test_has_index_schema[dialect] SKIPPED (Databricks does not support indexes.)
test_suite.py::HasIndexTest_databricks+databricks::test_has_index_schema[inspector] SKIPPED (Databricks does not support indexes.)
test_suite.py::TableDDLTest_databricks+databricks::test_add_table_comment SKIPPED (Comment reflection is possible but not implemented in this dialect.)
test_suite.py::TableDDLTest_databricks+databricks::test_create_index_if_not_exists SKIPPED (Databricks does not support indexes.)
test_suite.py::TableDDLTest_databricks+databricks::test_create_table PASSED
test_suite.py::TableDDLTest_databricks+databricks::test_create_table_if_not_exists PASSED
test_suite.py::TableDDLTest_databricks+databricks::test_create_table_schema PASSED
test_suite.py::TableDDLTest_databricks+databricks::test_drop_index_if_exists SKIPPED (Databricks does not support indexes.)
test_suite.py::TableDDLTest_databricks+databricks::test_drop_table PASSED
test_suite.py::TableDDLTest_databricks+databricks::test_drop_table_comment SKIPPED (Comment reflection is possible but not implemented in this dialect.)
test_suite.py::TableDDLTest_databricks+databricks::test_drop_table_if_exists PASSED
test_suite.py::TableDDLTest_databricks+databricks::test_underscore_names PASSED

Signed-off-by: Jesse Whitehouse <[email protected]>

* Permanently skip QuotedNameArgumentTest with comments

The fixes to DESCRIBE TABLE and visit_xxx were necessary to get to the
point where I could even determine that these tests wouldn't pass.

But those changes are not currently tested in the dialect. If, in the
course of reviewing the remaining tests in the compliance suite, I find
that these visit_xxxx methods are not tested anywhere else then we should
extend test_suite.py with our own tests to confirm the behaviour for
ourselves.

Signed-off-by: Jesse Whitehouse <[email protected]>

* Move files from base.py to _ddl.py

The presence of this pytest.ini file is _required_ to establish pytest's
root_path

https://docs.pytest.org/en/7.1.x/reference/customize.html#finding-the-rootdir

Without it, the custom pytest plugin from SQLAlchemy can't read the contents
of setup.cfg which makes none of the tests runnable.

Signed-off-by: Jesse Whitehouse <[email protected]>

* Emit a warning for certain constructs

Signed-off-by: Jesse Whitehouse <[email protected]>

* Stop skipping RowFetchTest

Date type work fixed this test failure

Signed-off-by: Jesse Whitehouse <[email protected]>

* Revise infer_types logic to never infer a TINYINT

This allows these SQLAlchemy tests to pass:

test_suite.py::FetchLimitOffsetTest_databricks+databricks::test_bound_limit PASSED
test_suite.py::FetchLimitOffsetTest_databricks+databricks::test_bound_limit_offset PASSED
test_suite.py::FetchLimitOffsetTest_databricks+databricks::test_expr_limit_simple_offset PASSED
test_suite.py::FetchLimitOffsetTest_databricks+databricks::test_simple_limit PASSED
test_suite.py::FetchLimitOffsetTest_databricks+databricks::test_simple_limit_expr_offset PASSED
test_suite.py::FetchLimitOffsetTest_databricks+databricks::test_simple_limit_offset[cases0] PASSED
test_suite.py::FetchLimitOffsetTest_databricks+databricks::test_simple_limit_offset[cases1] PASSED
test_suite.py::FetchLimitOffsetTest_databricks+databricks::test_simple_limit_offset[cases2] PASSED

This partially reverts the change introduced in databricks#246

Signed-off-by: Jesse Whitehouse <[email protected]>

* Stop skipping FetchLimitOffsetTest

I implemented our custom DatabricksStatementCompiler so we can override
the default rendering of unbounded LIMIT clauses from `LIMIT -1` to
`LIMIT ALL`

We also explicitly skip the FETCH clause tests since Databricks doesn't
support this syntax.

Blacked all source code here too.

Signed-off-by: Jesse Whitehouse <[email protected]>

* Stop skipping FutureTableDDLTest

Add meaningful skip markers for table comment reflection and indexes

Signed-off-by: Jesse Whitehouse <[email protected]>

* Stop skipping Identity column tests

This closes databricks#175

Signed-off-by: Jesse Whitehouse <[email protected]>

* Stop skipping HasTableTest

Adding the @reflection.cache decorator to has_table is necessary to pass
test_has_table_cache

Caching calls to has_table improves the efficiency of the connector

Signed-off-by: Jesse Whitehouse <[email protected]>

* Permanently skip LongNameBlowoutTest

Databricks constraint names are limited to 255 characters

Signed-off-by: Jesse Whitehouse <[email protected]>

* Stop skipping ExceptionTest

Black test_suite.py

Signed-off-by: Jesse Whitehouse <[email protected]>

* Permanently skip LastrowidTest

Signed-off-by: Jesse Whitehouse <[email protected]>

* Implement PRIMARY KEY and FOREIGN KEY reflection and enable tests

Signed-off-by: Jesse Whitehouse <[email protected]>

* Skip all IdentityColumnTest tests

Turns out that none of these can pass for the same reason that the
first two seemed un-runnable in db6f52b

Signed-off-by: Jesse Whitehouse <[email protected]>

---------

Signed-off-by: Jesse Whitehouse <[email protected]>
…ks#248)

* Put in some unit tests, will add e2e

Signed-off-by: nithinkdb <[email protected]>

* Added e2e test

Signed-off-by: nithinkdb <[email protected]>

* Linted

Signed-off-by: nithinkdb <[email protected]>

* re-bumped thrift files

Signed-off-by: nithinkdb <[email protected]>

* Changed structure to store protocol version as feature of connection

Signed-off-by: nithinkdb <[email protected]>

* Fixed parameters test

Signed-off-by: nithinkdb <[email protected]>

* Fixed comments

Signed-off-by: nithinkdb <[email protected]>

* Update src/databricks/sql/client.py

Co-authored-by: Jesse <[email protected]>
Signed-off-by: nithinkdb <[email protected]>

* Fixed comments

Signed-off-by: nithinkdb <[email protected]>

* Removed extra indent

Signed-off-by: nithinkdb <[email protected]>

---------

Signed-off-by: nithinkdb <[email protected]>
Co-authored-by: Jesse <[email protected]>
Jesse and others added 26 commits October 31, 2023 13:35
* Add OAuth M2M example

Signed-off-by: Jacky Hu <[email protected]>
Signed-off-by: Jesse Whitehouse <[email protected]>
(cherry picked from commit 8d85fa8)
…atabricks#274)

Remove Python 3.7 from the matrix for _all_ workflows

This was missed in databricks#270

Signed-off-by: Jesse Whitehouse <[email protected]>
pyarrow is currently compatible with Python 3.8 → Python 3.11

I also removed specifiers for when Python is 3.7 since this no longer
applies.

Signed-off-by: Jesse Whitehouse <[email protected]>
…#294)

Rename `dbapi` classmethod to `import_dbapi` as required by SQLAlchemy 2

Closes databricks#289

Signed-off-by: Jesse Whitehouse <[email protected]>
---------

Signed-off-by: Pieter Noordhuis <[email protected]>
Signed-off-by: Jesse Whitehouse <[email protected]>
Co-authored-by: Jesse Whitehouse <[email protected]>
Signed-off-by: Jessica <[email protected]>
Signed-off-by: Jesse Whitehouse <[email protected]>
Co-authored-by: Jesse Whitehouse <[email protected]>
Signed-off-by: Christophe Bornet <[email protected]>
Signed-off-by: Jesse Whitehouse <[email protected]>
Co-authored-by: Jesse Whitehouse <[email protected]>
…ks (databricks#330)

Signed-off-by: Ben Cassell <[email protected]>
Signed-off-by: Jesse Whitehouse <[email protected]>
Co-authored-by: Jesse Whitehouse <[email protected]>
Signed-off-by: Jesse Whitehouse <[email protected]>
Copy link

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 (git rebase -i main).

@matt-fleming matt-fleming merged commit 2091240 into main Jan 31, 2024
0 of 2 checks passed
@matt-fleming matt-fleming deleted the rebase-to-302 branch January 31, 2024 13:13
@matt-fleming matt-fleming mentioned this pull request Feb 9, 2024
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.

10 participants