From c9684df6a38be715d6e38888cfd5a1b9b1dc7392 Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Wed, 18 Oct 2023 16:43:29 -0400 Subject: [PATCH 01/23] WIP: moving files around (squash me please) Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 192 +++++++ src/databricks/sqlalchemy/test/_regression.py | 315 ++++++++++ .../sqlalchemy/test/_unsupported.py | 119 ++++ src/databricks/sqlalchemy/test/test_suite.py | 542 ------------------ 4 files changed, 626 insertions(+), 542 deletions(-) create mode 100644 src/databricks/sqlalchemy/test/_future.py create mode 100644 src/databricks/sqlalchemy/test/_regression.py create mode 100644 src/databricks/sqlalchemy/test/_unsupported.py diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py new file mode 100644 index 000000000..ecb61aec1 --- /dev/null +++ b/src/databricks/sqlalchemy/test/_future.py @@ -0,0 +1,192 @@ +import pytest +from sqlalchemy.testing.suite import ( + WeCanSetDefaultSchemaWEventsTest, + FutureWeCanSetDefaultSchemaWEventsTest, + SimpleUpdateDeleteTest, + RowCountTest, + NativeUUIDTest, + CollateTest, + TimeTZTest, + DateTimeTZTest, + LikeFunctionsTest, + JSONTest, + JSONLegacyStringCastIndexTest, + BizarroCharacterFKResolutionTest, + DifficultParametersTest, + IdentityReflectionTest, + IdentityColumnTest, + IdentityAutoincrementTest +) + + +@pytest.mark.reviewed +@pytest.mark.skip( + reason="Identity works. Test needs rewrite for Databricks. See comments in test_suite.py" +) +class IdentityColumnTest(IdentityColumnTest): + """The setup for these tests tries to create a table with a DELTA IDENTITY column but has two problems: + 1. It uses an Integer() type for the column. Whereas DELTA IDENTITY columns must be BIGINT. + 2. It tries to set the start == 42, which Databricks doesn't support + + I can get the tests to _run_ by patching the table fixture to use BigInteger(). But it asserts that the + identity of two rows are 42 and 43, which is not possible since they will be rows 1 and 2 instead. + + I'm satisified through manual testing that our implementation of visit_identity_column works but a better test is needed. + """ + + pass + + +@pytest.mark.reviewed +class IdentityAutoincrementTest(IdentityAutoincrementTest): + @pytest.mark.skip( + reason="Identity works. Test needs rewrite for Databricks. See comments in test_suite.py" + ) + def test_autoincrement_with_identity(self): + """This test has the same issue as IdentityColumnTest.test_select_all in that it creates a table with identity + using an Integer() rather than a BigInteger(). If I override this behaviour to use a BigInteger() instead, the + test passes. + """ + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Implementation deferred. See test_suite.py") +class BizarroCharacterFKResolutionTest(BizarroCharacterFKResolutionTest): + """Some of the combinations in this test pass. Others fail. Given the esoteric nature of these failures, + we have opted to defer implementing fixes to a later time, guided by customer feedback. Passage of + these tests is not an acceptance criteria for our dialect. + """ + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Implementation deferred. See test_suite.py") +class DifficultParametersTest(DifficultParametersTest): + """Some of the combinations in this test pass. Others fail. Given the esoteric nature of these failures, + we have opted to defer implementing fixes to a later time, guided by customer feedback. Passage of + these tests is not an acceptance criteria for our dialect. + """ + + +@pytest.mark.reviewed +@pytest.mark.skip( + reason="Identity reflection is not implemented in this dialect. See test_suite.py" +) +class IdentityReflectionTest(IdentityReflectionTest): + """It's not clear _how_ to implement this for SQLAlchemy. Columns created with GENERATED ALWAYS AS IDENTITY + are not specially demarked in the output of TGetColumnsResponse or DESCRIBE TABLE EXTENDED. + + We could theoretically parse this from the contents of `SHOW CREATE TABLE` but that feels like a hack. + """ + +@pytest.mark.reviewed +@pytest.mark.skip( + reason="Databricks dialect doesn't implement JSON column types. See test_suite.py" +) +class JSONTest(JSONTest): + """Databricks supports JSON path expressions in queries it's just not implemented in this dialect.""" + + pass + + +@pytest.mark.reviewed +@pytest.mark.skip( + reason="Databricks dialect doesn't implement JSON column types. See test_suite.py" +) +class JSONLegacyStringCastIndexTest(JSONLegacyStringCastIndexTest): + """Same comment applies as JSONTest""" + + pass + +@pytest.mark.reviewed +class LikeFunctionsTest(LikeFunctionsTest): + @pytest.mark.skip( + reason="Databricks dialect doesn't implement regexp features. See test_suite.py" + ) + def test_not_regexp_match(self): + """The defaul dialect doesn't implement _visit_regexp methods so we don't get them automatically.""" + pass + + @pytest.mark.skip( + reason="Databricks dialect doesn't implement regexp features. See test_suite.py" + ) + def test_regexp_match(self): + """The defaul dialect doesn't implement _visit_regexp methods so we don't get them automatically.""" + pass + + + +@pytest.mark.reviewed +@pytest.mark.skip( + reason="Datetime handling doesn't handle timezones well. Priority to fix." +) +class DateTimeTZTest(DateTimeTZTest): + """When I initially implemented DateTime type handling, I started using TIMESTAMP_NTZ because + that's the default behaviour of the DateTime() type and the other tests passed. I simply missed + this group of tests. Will need to modify the compilation and result_processor for our type override + so that we can pass both DateTimeTZTest and DateTimeTest. Currently, only DateTimeTest passes. + """ + + pass + +@pytest.mark.reviewed +@pytest.mark.skip( + reason="Databricks dialect does not implement timezone support for Timestamp() types. See test_suite.py" +) +class TimeTZTest(TimeTZTest): + """Similar to DateTimeTZTest, this should be possible for the dialect since we can override type compilation + and processing in _types.py. Implementation has been deferred. + """ + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks dialect does not implement COLLATE support") +class CollateTest(CollateTest): + """This is supported in Databricks. Not implemented here.""" + + +@pytest.mark.reviewed +@pytest.mark.skip( + reason="Databricks dialect doesn't implement UUID type. See test_suite.py" +) +class NativeUUIDTest(NativeUUIDTest): + """Type implementation will be straightforward. Since Databricks doesn't have a native UUID type we can use + a STRING field, create a custom TypeDecorator for sqlalchemy.types.Uuid and add it to the dialect's colspecs. + + Then mark requirements.uuid_data_type as open() so this test can run. + """ + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks dialect does not implement sane rowcount.") +class RowCountTest(RowCountTest): + pass + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks dialect does not implement sane rowcount.") +class SimpleUpdateDeleteTest(SimpleUpdateDeleteTest): + pass + + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Dialect doesn't implement provision.py See test_suite.py") +class WeCanSetDefaultSchemaWEventsTest(WeCanSetDefaultSchemaWEventsTest): + """provision.py allows us to define event listeners that emit DDL for things like setting up a test schema + or, in this case, changing the default schema for the connection after it's been built. This would override + the schema defined in the sqlalchemy connection string. This support is possible but is not implemented + in the dialect. Deferred for now. + """ + + pass + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Dialect doesn't implement provision.py See test_suite.py") +class FutureWeCanSetDefaultSchemaWEventsTest(FutureWeCanSetDefaultSchemaWEventsTest): + """provision.py allows us to define event listeners that emit DDL for things like setting up a test schema + or, in this case, changing the default schema for the connection after it's been built. This would override + the schema defined in the sqlalchemy connection string. This support is possible but is not implemented + in the dialect. Deferred for now. + """ + + pass diff --git a/src/databricks/sqlalchemy/test/_regression.py b/src/databricks/sqlalchemy/test/_regression.py new file mode 100644 index 000000000..c42590691 --- /dev/null +++ b/src/databricks/sqlalchemy/test/_regression.py @@ -0,0 +1,315 @@ +import pytest +from sqlalchemy.testing.suite import ( + TimeMicrosecondsTest, + TextTest, + StringTest, + DateTimeMicrosecondsTest, + TimestampMicrosecondsTest, + DateTimeCoercedToDateTimeTest, + TimeTest, + DateTimeTest, + DateTimeHistoricTest, + DateTest, + DateHistoricTest, + RowFetchTest, + CompositeKeyReflectionTest, + TrueDivTest, + ArgSignatureTest, + CompoundSelectTest, + DeprecatedCompoundSelectTest, + CastTypeDecoratorTest, + DistinctOnTest, + EscapingTest, + ExistsTest, + IntegerTest, + IsOrIsNotDistinctFromTest, + JoinTest, + OrderByLabelTest, + PingTest, + ReturningGuardsTest, + SameNamedSchemaTableTest, + UnicodeTextTest, + UnicodeVarcharTest, + TableNoColumnsTest, + PostCompileParamsTest, + BooleanTest, + ValuesExpressionTest, + UuidTest, + FetchLimitOffsetTest, + FutureTableDDLTest +) + +@pytest.mark.reviewed +class FutureTableDDLTest(FutureTableDDLTest): + @pytest.mark.skip( + reason="Comment reflection is possible but not implemented in this dialect." + ) + def test_add_table_comment(self): + """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" + pass + + @pytest.mark.skip( + reason="Comment reflection is possible but not implemented in this dialect." + ) + def test_drop_table_comment(self): + """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" + pass + + @pytest.mark.skip(reason="Databricks does not support indexes.") + def test_create_index_if_not_exists(self): + """We could use requirements.index_reflection and requirements.index_ddl_if_exists + here to disable this but prefer a more meaningful skip message + """ + pass + + @pytest.mark.skip(reason="Databricks does not support indexes.") + def test_drop_index_if_exists(self): + """We could use requirements.index_reflection and requirements.index_ddl_if_exists + here to disable this but prefer a more meaningful skip message + """ + pass + + +@pytest.mark.reviewed +class FetchLimitOffsetTest(FetchLimitOffsetTest): + @pytest.mark.flaky + @pytest.mark.skip( + reason="Insertion order on Databricks is not deterministic. See comment in test_suite.py." + ) + def test_limit_render_multiple_times(self): + """This test depends on the order that records are inserted into the table. It's passing criteria requires that + a record inserted with id=1 is the first record returned when no ORDER BY clause is specified. But Databricks occasionally + INSERTS in a different order, which makes this test seem to fail. The test is flaky, but the underlying functionality + (can multiple LIMIT clauses be rendered) is not broken. + + Unclear if this is a bug in Databricks, Delta, or some race-condition in the test itself. + """ + pass + + @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") + def test_bound_fetch_offset(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") + def test_fetch_offset_no_order(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") + def test_fetch_offset_nobinds(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") + def test_simple_fetch(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") + def test_simple_fetch_offset(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") + def test_simple_fetch_percent(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") + def test_simple_fetch_percent_ties(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") + def test_simple_fetch_ties(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") + def test_expr_fetch_offset(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") + def test_fetch_offset_percent(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") + def test_fetch_offset_percent_ties(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") + def test_fetch_offset_ties(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") + def test_fetch_offset_ties_exact_number(self): + pass + + +@pytest.mark.reviewed +class UuidTest(UuidTest): + @pytest.mark.skip(reason="Databricks doesn't support INSERT ... RETURNING syntax") + def test_uuid_returning(self): + pass + + +@pytest.mark.reviewed +class ValuesExpressionTest(ValuesExpressionTest): + pass + + +@pytest.mark.reviewed +class BooleanTest(BooleanTest): + pass + + +@pytest.mark.reviewed +class PostCompileParamsTest(PostCompileParamsTest): + pass + + +@pytest.mark.reviewed +class TimeMicrosecondsTest(TimeMicrosecondsTest): + pass + + +@pytest.mark.reviewed +class TextTest(TextTest): + pass + + +@pytest.mark.reviewed +class StringTest(StringTest): + pass + + +@pytest.mark.reviewed +class DateTimeMicrosecondsTest(DateTimeMicrosecondsTest): + pass + + +@pytest.mark.reviewed +class TimestampMicrosecondsTest(TimestampMicrosecondsTest): + pass + + +@pytest.mark.reviewed +class DateTimeCoercedToDateTimeTest(DateTimeCoercedToDateTimeTest): + pass + + +@pytest.mark.reviewed +class TimeTest(TimeTest): + pass + + +@pytest.mark.reviewed +class DateTimeTest(DateTimeTest): + pass + + +@pytest.mark.reviewed +class DateTimeHistoricTest(DateTimeHistoricTest): + pass + + +@pytest.mark.reviewed +class DateTest(DateTest): + pass + + +@pytest.mark.reviewed +class DateHistoricTest(DateHistoricTest): + pass + + +@pytest.mark.reviewed +class RowFetchTest(RowFetchTest): + pass + + +@pytest.mark.reviewed +class CompositeKeyReflectionTest(CompositeKeyReflectionTest): + pass + + +@pytest.mark.reviewed +class TrueDivTest(TrueDivTest): + pass + + +@pytest.mark.reviewed +class ArgSignatureTest(ArgSignatureTest): + pass + + +@pytest.mark.reviewed +class CompoundSelectTest(CompoundSelectTest): + pass + + +@pytest.mark.reviewed +class DeprecatedCompoundSelectTest(DeprecatedCompoundSelectTest): + pass + + +@pytest.mark.reviewed +class CastTypeDecoratorTest(CastTypeDecoratorTest): + pass + + +@pytest.mark.reviewed +class DistinctOnTest(DistinctOnTest): + pass + + +@pytest.mark.reviewed +class EscapingTest(EscapingTest): + pass + + +@pytest.mark.reviewed +class ExistsTest(ExistsTest): + pass + + +@pytest.mark.reviewed +class IntegerTest(IntegerTest): + pass + + +@pytest.mark.reviewed +class IsOrIsNotDistinctFromTest(IsOrIsNotDistinctFromTest): + pass + + +@pytest.mark.reviewed +class JoinTest(JoinTest): + pass + + +@pytest.mark.reviewed +class OrderByLabelTest(OrderByLabelTest): + pass + + +@pytest.mark.reviewed +class PingTest(PingTest): + pass + + +@pytest.mark.reviewed +class ReturningGuardsTest(ReturningGuardsTest): + pass + + +@pytest.mark.reviewed +class SameNamedSchemaTableTest(SameNamedSchemaTableTest): + pass + + +@pytest.mark.reviewed +class UnicodeTextTest(UnicodeTextTest): + pass + + +@pytest.mark.reviewed +class UnicodeVarcharTest(UnicodeVarcharTest): + pass + +@pytest.mark.reviewed +class TableNoColumnsTest(TableNoColumnsTest): + pass diff --git a/src/databricks/sqlalchemy/test/_unsupported.py b/src/databricks/sqlalchemy/test/_unsupported.py new file mode 100644 index 000000000..6f3378860 --- /dev/null +++ b/src/databricks/sqlalchemy/test/_unsupported.py @@ -0,0 +1,119 @@ +import pytest +from sqlalchemy.testing.suite import ( + SequenceTest, + SequenceCompilerTest, + ComputedColumnTest, + ComputedReflectionTest, + ReturningTest, + IsolationLevelTest, + AutocommitIsolationTest, + PercentSchemaNamesTest, + UnicodeSchemaTest, + ServerSideCursorsTest, + HasIndexTest, + HasSequenceTest, + HasSequenceTestEmpty, + LongNameBlowoutTest, + ExceptionTest +) + +@pytest.mark.reviewed +class ExceptionTest(ExceptionTest): + @pytest.mark.skip(reason="Databricks doesn't enforce primary key constraints.") + def test_integrity_error(self): + """Per Databricks documentation, primary and foreign key constraints are informational only + and are not enforced. + + https://docs.databricks.com/api/workspace/tableconstraints + """ + pass + +@pytest.mark.reviewed +class LongNameBlowoutTest(LongNameBlowoutTest): + """These tests all include assertions that the tested name > 255 characters""" + + @pytest.mark.skip( + reason="Databricks constraint names are limited to 255 characters" + ) + def test_long_convention_name(self): + pass + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks doesn't support SEQUENCE server defaults") +class HasSequenceTest(HasSequenceTest): + pass + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks doesn't support SEQUENCE server defaults") +class HasSequenceTestEmpty(HasSequenceTestEmpty): + pass + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks does not support indexes.") +class HasIndexTest(HasIndexTest): + pass + + +@pytest.mark.reviewed +@pytest.mark.skipped(reason="Databricks doesn't support unicode in symbol names") +class UnicodeSchemaTest(UnicodeSchemaTest): + pass + + + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks doesn't support server-side cursors.") +class ServerSideCursorsTest(ServerSideCursorsTest): + pass + + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks doesn't allow percent signs in identifiers") +class PercentSchemaNamesTest(PercentSchemaNamesTest): + pass + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks does not support transactions") +class IsolationLevelTest(IsolationLevelTest): + pass + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks does not support transactions") +class AutocommitIsolationTest(AutocommitIsolationTest): + pass + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks doesn't support INSERT ... RETURNING syntax") +class ReturningTest(ReturningTest): + pass + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks does not support sequences.") +class SequenceTest(SequenceTest): + pass + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks does not support sequences.") +class SequenceCompilerTest(SequenceCompilerTest): + pass + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks does not support computed / generated columns") +class ComputedColumnTest(ComputedColumnTest): + pass + + +@pytest.mark.reviewed +@pytest.mark.skip(reason="Databricks does not support computed / generated columns") +class ComputedReflectionTest(ComputedReflectionTest): + pass diff --git a/src/databricks/sqlalchemy/test/test_suite.py b/src/databricks/sqlalchemy/test/test_suite.py index ea141336b..d9bbda89a 100644 --- a/src/databricks/sqlalchemy/test/test_suite.py +++ b/src/databricks/sqlalchemy/test/test_suite.py @@ -30,11 +30,6 @@ class BinaryTest(BinaryTest): pass -@pytest.mark.reviewed -class BooleanTest(BooleanTest): - pass - - @pytest.mark.reviewed class NumericTest(NumericTest): @pytest.mark.skip(reason="Databricks doesn't support E notation for DECIMAL types") @@ -68,217 +63,15 @@ def test_float_custom_scale(self): pass -@pytest.mark.reviewed -class TimeMicrosecondsTest(TimeMicrosecondsTest): - pass - - -@pytest.mark.reviewed -class TextTest(TextTest): - pass - - -@pytest.mark.reviewed -class StringTest(StringTest): - pass - - -@pytest.mark.reviewed -class DateTimeMicrosecondsTest(DateTimeMicrosecondsTest): - pass - - -@pytest.mark.reviewed -class TimestampMicrosecondsTest(TimestampMicrosecondsTest): - pass - - -@pytest.mark.reviewed -class DateTimeCoercedToDateTimeTest(DateTimeCoercedToDateTimeTest): - pass - - -@pytest.mark.reviewed -class TimeTest(TimeTest): - pass - - -@pytest.mark.reviewed -class DateTimeTest(DateTimeTest): - pass - - -@pytest.mark.reviewed -class DateTimeHistoricTest(DateTimeHistoricTest): - pass - - -@pytest.mark.reviewed -class DateTest(DateTest): - pass - - -@pytest.mark.reviewed -class DateHistoricTest(DateHistoricTest): - pass - - -@pytest.mark.reviewed -class RowFetchTest(RowFetchTest): - pass - - -@pytest.mark.reviewed -class FetchLimitOffsetTest(FetchLimitOffsetTest): - @pytest.mark.flaky - @pytest.mark.skip( - reason="Insertion order on Databricks is not deterministic. See comment in test_suite.py." - ) - def test_limit_render_multiple_times(self): - """This test depends on the order that records are inserted into the table. It's passing criteria requires that - a record inserted with id=1 is the first record returned when no ORDER BY clause is specified. But Databricks occasionally - INSERTS in a different order, which makes this test seem to fail. The test is flaky, but the underlying functionality - (can multiple LIMIT clauses be rendered) is not broken. - - Unclear if this is a bug in Databricks, Delta, or some race-condition in the test itself. - """ - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_bound_fetch_offset(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_fetch_offset_no_order(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_fetch_offset_nobinds(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_simple_fetch(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_simple_fetch_offset(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_simple_fetch_percent(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_simple_fetch_percent_ties(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_simple_fetch_ties(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_expr_fetch_offset(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_fetch_offset_percent(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_fetch_offset_percent_ties(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_fetch_offset_ties(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_fetch_offset_ties_exact_number(self): - pass - - -@pytest.mark.reviewed -class FutureTableDDLTest(FutureTableDDLTest): - @pytest.mark.skip( - reason="Comment reflection is possible but not implemented in this dialect." - ) - def test_add_table_comment(self): - """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" - pass - - @pytest.mark.skip( - reason="Comment reflection is possible but not implemented in this dialect." - ) - def test_drop_table_comment(self): - """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" - pass - - @pytest.mark.skip(reason="Databricks does not support indexes.") - def test_create_index_if_not_exists(self): - """We could use requirements.index_reflection and requirements.index_ddl_if_exists - here to disable this but prefer a more meaningful skip message - """ - pass - @pytest.mark.skip(reason="Databricks does not support indexes.") - def test_drop_index_if_exists(self): - """We could use requirements.index_reflection and requirements.index_ddl_if_exists - here to disable this but prefer a more meaningful skip message - """ - pass -@pytest.mark.reviewed -@pytest.mark.skip( - reason="Identity works. Test needs rewrite for Databricks. See comments in test_suite.py" -) -class IdentityColumnTest(IdentityColumnTest): - """The setup for these tests tries to create a table with a DELTA IDENTITY column but has two problems: - 1. It uses an Integer() type for the column. Whereas DELTA IDENTITY columns must be BIGINT. - 2. It tries to set the start == 42, which Databricks doesn't support - I can get the tests to _run_ by patching the table fixture to use BigInteger(). But it asserts that the - identity of two rows are 42 and 43, which is not possible since they will be rows 1 and 2 instead. - I'm satisified through manual testing that our implementation of visit_identity_column works but a better test is needed. - """ - - pass - - -@pytest.mark.reviewed -class IdentityAutoincrementTest(IdentityAutoincrementTest): - @pytest.mark.skip( - reason="Identity works. Test needs rewrite for Databricks. See comments in test_suite.py" - ) - def test_autoincrement_with_identity(self): - """This test has the same issue as IdentityColumnTest.test_select_all in that it creates a table with identity - using an Integer() rather than a BigInteger(). If I override this behaviour to use a BigInteger() instead, the - test passes. - """ - - -@pytest.mark.reviewed -class LongNameBlowoutTest(LongNameBlowoutTest): - """These tests all include assertions that the tested name > 255 characters""" - @pytest.mark.skip( - reason="Databricks constraint names are limited to 255 characters" - ) - def test_long_convention_name(self): - pass -@pytest.mark.reviewed -class ExceptionTest(ExceptionTest): - @pytest.mark.skip(reason="Databricks doesn't enforce primary key constraints.") - def test_integrity_error(self): - """Per Databricks documentation, primary and foreign key constraints are informational only - and are not enforced. - https://docs.databricks.com/api/workspace/tableconstraints - """ - pass @pytest.mark.reviewed @@ -326,11 +119,6 @@ class LastrowidTest(LastrowidTest): pass -@pytest.mark.reviewed -class CompositeKeyReflectionTest(CompositeKeyReflectionTest): - pass - - @pytest.mark.reviewed class ComponentReflectionTestExtra(ComponentReflectionTestExtra): @pytest.mark.skip(reason="This dialect does not support check constraints") @@ -507,12 +295,6 @@ def test_drop_table_comment(self, connection): pass -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks does not support indexes.") -class HasIndexTest(HasIndexTest): - pass - - @pytest.mark.reviewed @pytest.mark.skip( reason="Databricks does not support spaces in table names. See comment in test_suite.py" @@ -525,121 +307,6 @@ class QuotedNameArgumentTest(QuotedNameArgumentTest): """ -@pytest.mark.reviewed -@pytest.mark.skip(reason="Implementation deferred. See test_suite.py") -class BizarroCharacterFKResolutionTest: - """Some of the combinations in this test pass. Others fail. Given the esoteric nature of these failures, - we have opted to defer implementing fixes to a later time, guided by customer feedback. Passage of - these tests is not an acceptance criteria for our dialect. - """ - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Implementation deferred. See test_suite.py") -class DifficultParametersTest: - """Some of the combinations in this test pass. Others fail. Given the esoteric nature of these failures, - we have opted to defer implementing fixes to a later time, guided by customer feedback. Passage of - these tests is not an acceptance criteria for our dialect. - """ - - -@pytest.mark.reviewed -@pytest.mark.skip( - reason="Identity reflection is not implemented in this dialect. See test_suite.py" -) -class IdentityReflectionTest(IdentityReflectionTest): - """It's not clear _how_ to implement this for SQLAlchemy. Columns created with GENERATED ALWAYS AS IDENTITY - are not specially demarked in the output of TGetColumnsResponse or DESCRIBE TABLE EXTENDED. - - We could theoretically parse this from the contents of `SHOW CREATE TABLE` but that feels like a hack. - """ - - -@pytest.mark.reviewed -class TrueDivTest(TrueDivTest): - pass - - -@pytest.mark.reviewed -class ArgSignatureTest(ArgSignatureTest): - pass - - -@pytest.mark.reviewed -class CompoundSelectTest(CompoundSelectTest): - pass - - -@pytest.mark.reviewed -class DeprecatedCompoundSelectTest(DeprecatedCompoundSelectTest): - pass - - -@pytest.mark.reviewed -class CastTypeDecoratorTest(CastTypeDecoratorTest): - pass - - -@pytest.mark.reviewed -class DistinctOnTest(DistinctOnTest): - pass - - -@pytest.mark.reviewed -class EscapingTest(EscapingTest): - pass - - -@pytest.mark.reviewed -class ExistsTest(ExistsTest): - pass - - -@pytest.mark.reviewed -class IntegerTest(IntegerTest): - pass - - -@pytest.mark.reviewed -class IsOrIsNotDistinctFromTest(IsOrIsNotDistinctFromTest): - pass - - -@pytest.mark.reviewed -class JoinTest(JoinTest): - pass - - -@pytest.mark.reviewed -class OrderByLabelTest(OrderByLabelTest): - pass - - -@pytest.mark.reviewed -class PingTest(PingTest): - pass - - -@pytest.mark.reviewed -class ReturningGuardsTest(ReturningGuardsTest): - pass - - -@pytest.mark.reviewed -class SameNamedSchemaTableTest(SameNamedSchemaTableTest): - pass - - -@pytest.mark.reviewed -class UnicodeTextTest(UnicodeTextTest): - pass - - -@pytest.mark.reviewed -class UnicodeVarcharTest(UnicodeVarcharTest): - pass - - @pytest.mark.reviewed @pytest.mark.skip( reason="pysql doesn't support binding of array parameters. See test_suite.py" @@ -651,70 +318,12 @@ class ArrayTest(ArrayTest): """ -@pytest.mark.reviewed -@pytest.mark.skip( - reason="Databricks dialect doesn't implement JSON column types. See test_suite.py" -) -class JSONTest(JSONTest): - """Databricks supports JSON path expressions in queries it's just not implemented in this dialect.""" - - pass - - -@pytest.mark.reviewed -@pytest.mark.skip( - reason="Databricks dialect doesn't implement JSON column types. See test_suite.py" -) -class JSONLegacyStringCastIndexTest(JSONLegacyStringCastIndexTest): - """Same comment applies as JSONTest""" - - pass - - @pytest.mark.reviewed @pytest.mark.skip(reason="Databricks doesn't support INSERT ... RETURNING syntax") class ReturningText(ReturningTest): pass -@pytest.mark.reviewed -class LikeFunctionsTest(LikeFunctionsTest): - @pytest.mark.skip( - reason="Databricks dialect doesn't implement regexp features. See test_suite.py" - ) - def test_not_regexp_match(self): - """The defaul dialect doesn't implement _visit_regexp methods so we don't get them automatically.""" - pass - - @pytest.mark.skip( - reason="Databricks dialect doesn't implement regexp features. See test_suite.py" - ) - def test_regexp_match(self): - """The defaul dialect doesn't implement _visit_regexp methods so we don't get them automatically.""" - pass - - -@pytest.mark.reviewed -class UuidTest(UuidTest): - @pytest.mark.skip(reason="Databricks doesn't support INSERT ... RETURNING syntax") - def test_uuid_returning(self): - pass - - -@pytest.mark.reviewed -@pytest.mark.skip( - reason="Datetime handling doesn't handle timezones well. Priority to fix." -) -class DateTimeTZTest(DateTimeTZTest): - """When I initially implemented DateTime type handling, I started using TIMESTAMP_NTZ because - that's the default behaviour of the DateTime() type and the other tests passed. I simply missed - this group of tests. Will need to modify the compilation and result_processor for our type override - so that we can pass both DateTimeTZTest and DateTimeTest. Currently, only DateTimeTest passes. - """ - - pass - - TUPLES_READ_AS_STRUCT_MSG = ( "Databricks interprets tuple-like IN markers as though they are structs." ) @@ -739,18 +348,6 @@ def test_empty_homogeneous_tuples_direct(self): pass -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks doesn't support SEQUENCE server defaults") -class HasSequenceTest(HasSequenceTest): - pass - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks doesn't support SEQUENCE server defaults") -class HasSequenceTestEmpty(HasSequenceTestEmpty): - pass - - @pytest.mark.reviewed class CTETest(CTETest): """During the teardown for this test block, it tries to drop a constraint that it never named which raises @@ -778,129 +375,6 @@ def test_delete_scalar_subq_round_trip(self): pass -@pytest.mark.reviewed -@pytest.mark.skip(reason="Dialect doesn't implement provision.py See test_suite.py") -class WeCanSetDefaultSchemaWEventsTest(WeCanSetDefaultSchemaWEventsTest): - """provision.py allows us to define event listeners that emit DDL for things like setting up a test schema - or, in this case, changing the default schema for the connection after it's been built. This would override - the schema defined in the sqlalchemy connection string. This support is possible but is not implemented - in the dialect. Deferred for now. - """ - - pass - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Dialect doesn't implement provision.py See test_suite.py") -class FutureWeCanSetDefaultSchemaWEventsTest(FutureWeCanSetDefaultSchemaWEventsTest): - """provision.py allows us to define event listeners that emit DDL for things like setting up a test schema - or, in this case, changing the default schema for the connection after it's been built. This would override - the schema defined in the sqlalchemy connection string. This support is possible but is not implemented - in the dialect. Deferred for now. - """ - - pass - - -@pytest.mark.reviewed -class ValuesExpressionTest(ValuesExpressionTest): - pass - - -@pytest.mark.reviewed -@pytest.mark.skipped(reason="Databricks doesn't support unicode in symbol names") -class UnicodeSchemaTest(UnicodeSchemaTest): - pass - - -@pytest.mark.reviewed -class TableNoColumnsTest(TableNoColumnsTest): - pass - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks doesn't support server-side cursors.") -class ServerSideCursorsTest(ServerSideCursorsTest): - pass - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks does not support sequences.") -class SequenceTest(SequenceTest): - pass - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks does not support sequences.") -class SequenceCompilerTest(SequenceCompilerTest): - pass - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks dialect does not implement sane rowcount.") -class RowCountTest(RowCountTest): - pass - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks dialect does not implement sane rowcount.") -class SimpleUpdateDeleteTest(SimpleUpdateDeleteTest): - pass - - -@pytest.mark.reviewed -class PostCompileParamsTest(PostCompileParamsTest): - pass - - -@pytest.mark.reviewed -@pytest.mark.skip( - reason="Databricks dialect doesn't implement UUID type. See test_suite.py" -) -class NativeUUIDTest(NativeUUIDTest): - """Type implementation will be straightforward. Since Databricks doesn't have a native UUID type we can use - a STRING field, create a custom TypeDecorator for sqlalchemy.types.Uuid and add it to the dialect's colspecs. - - Then mark requirements.uuid_data_type as open() so this test can run. - """ - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks doesn't allow percent signs in identifiers") -class PercentSchemaNamesTest(PercentSchemaNamesTest): - pass - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks does not support transactions") -class IsolationLevelTest(IsolationLevelTest): - pass - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks does not support transactions") -class AutocommitIsolationTest(AutocommitIsolationTest): - pass - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks dialect does not implement COLLATE support") -class CollateTest(CollateTest): - """This is supported in Databricks. Not implemented here.""" - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks does not support computed / generated columns") -class ComputedColumnTest(ComputedColumnTest): - pass - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks does not support computed / generated columns") -class ComputedReflectionTest(ComputedReflectionTest): - pass - - @pytest.mark.reviewed class NormalizedNameTest(NormalizedNameTest): @pytest.mark.skip(reason="Poor test design? See test_suite.py") @@ -915,19 +389,3 @@ def test_get_table_names(self): It's forcibly calling .upper() and .lower() on the same string and expecting them to be equal. """ pass - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks doesn't support INSERT ... RETURNING syntax") -class ReturningTest(ReturningTest): - pass - - -@pytest.mark.reviewed -@pytest.mark.skip( - reason="Databricks dialect does not implement timezone support for Timestamp() types. See test_suite.py" -) -class TimeTZTest(TimeTZTest): - """Similar to DateTimeTZTest, this should be possible for the dialect since we can override type compilation - and processing in _types.py. Implementation has been deferred. - """ From 84f32ce618d16cff7a7ec7cec0f452a0931719ce Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Thu, 19 Oct 2023 12:57:38 -0400 Subject: [PATCH 02/23] Split test_suite.py into _regression, _future, and _unsupported files Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 103 +++++++- src/databricks/sqlalchemy/test/_regression.py | 245 +++++++++++++++++- .../sqlalchemy/test/_unsupported.py | 29 ++- 3 files changed, 374 insertions(+), 3 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index ecb61aec1..028ec62a3 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -15,9 +15,110 @@ DifficultParametersTest, IdentityReflectionTest, IdentityColumnTest, - IdentityAutoincrementTest + IdentityAutoincrementTest, + CTETest, + NormalizedNameTest, + ExpandingBoundInTest, + LastrowidTest, + BinaryTest ) +@pytest.mark.reviewed +@pytest.mark.skip(reason="pysql doesn't support binding of BINARY type parameters") +class BinaryTest(BinaryTest): + pass + + + + +@pytest.mark.reviewed +@pytest.mark.skip( + reason="This dialect does not support implicit autoincrement. See comments in test_suite.py" +) +class LastrowidTest(LastrowidTest): + """SQLAlchemy docs describe that a column without an explicit Identity() may implicitly create one if autoincrement=True. + That is what this method tests. Databricks supports auto-incrementing IDENTITY columns but they must be explicitly + declared. This limitation is present in our dialect as well. Which means that SQLAlchemy's autoincrement setting of a column + is ignored. We emit a logging.WARN message if you try it. + + In the future we could handle this autoincrement by implicitly calling the visit_identity_column() method of our DDLCompiler + when autoincrement=True. There is an example of this in the Microsoft SQL Server dialect: MSSDDLCompiler.get_column_specification + + For now, if you need to create a SQLAlchemy column with an auto-incrementing identity, you must set this explicitly in your column + definition by passing an Identity() to the column constructor. + """ + + pass + + + +TUPLES_READ_AS_STRUCT_MSG = ( + "Databricks interprets tuple-like IN markers as though they are structs." +) + + +@pytest.mark.reviewed +class ExpandingBoundInTest(ExpandingBoundInTest): + @pytest.mark.skip(reason=TUPLES_READ_AS_STRUCT_MSG) + def test_empty_heterogeneous_tuples_bindparam(self): + pass + + @pytest.mark.skip(reason=TUPLES_READ_AS_STRUCT_MSG) + def test_empty_heterogeneous_tuples_direct(self): + pass + + @pytest.mark.skip(reason=TUPLES_READ_AS_STRUCT_MSG) + def test_empty_homogeneous_tuples_bindparam(self): + pass + + @pytest.mark.skip(reason=TUPLES_READ_AS_STRUCT_MSG) + def test_empty_homogeneous_tuples_direct(self): + pass + + +@pytest.mark.reviewed +class NormalizedNameTest(NormalizedNameTest): + @pytest.mark.skip(reason="Poor test design? See test_suite.py") + def test_get_table_names(self): + """I'm not clear how this test can ever pass given that it's assertion looks like this: + + ```python + eq_(tablenames[0].upper(), tablenames[0].lower()) + eq_(tablenames[1].upper(), tablenames[1].lower()) + ``` + + It's forcibly calling .upper() and .lower() on the same string and expecting them to be equal. + """ + pass + + +@pytest.mark.reviewed +class CTETest(CTETest): + """During the teardown for this test block, it tries to drop a constraint that it never named which raises + a compilation error. This could point to poor constraint reflection but our other constraint reflection + tests pass. Requires investigation. + """ + + @pytest.mark.skip( + reason="Databricks dialect doesn't implement multiple-table criteria within DELETE" + ) + def test_delete_from_round_trip(self): + """This may be supported by Databricks but has not been implemented here.""" + pass + + @pytest.mark.skip(reason="Databricks doesn't support recursive CTE") + def test_select_recursive_round_trip(self): + pass + + @pytest.mark.skip(reason="Unsupported by Databricks. See test_suite.py") + def test_delete_scalar_subq_round_trip(self): + """Error received is [UNSUPPORTED_SUBQUERY_EXPRESSION_CATEGORY.MUST_AGGREGATE_CORRELATED_SCALAR_SUBQUERY] + + This suggests a limitation of the platform. But a workaround may be possible if customers require it. + """ + pass + + @pytest.mark.reviewed @pytest.mark.skip( diff --git a/src/databricks/sqlalchemy/test/_regression.py b/src/databricks/sqlalchemy/test/_regression.py index c42590691..356b4fd5d 100644 --- a/src/databricks/sqlalchemy/test/_regression.py +++ b/src/databricks/sqlalchemy/test/_regression.py @@ -36,9 +36,252 @@ ValuesExpressionTest, UuidTest, FetchLimitOffsetTest, - FutureTableDDLTest + FutureTableDDLTest, + TableDDLTest, + ComponentReflectionTest, + InsertBehaviorTest, + ComponentReflectionTestExtra, + HasTableTest, + NumericTest ) +@pytest.mark.reviewed +class NumericTest(NumericTest): + @pytest.mark.skip(reason="Databricks doesn't support E notation for DECIMAL types") + def test_enotation_decimal(self): + """This test automatically runs if requirements.precision_numerics_enotation_large is open()""" + pass + + @pytest.mark.skip(reason="Databricks doesn't support E notation for DECIMAL types") + def test_enotation_decimal_large(self): + """This test automatically runs if requirements.precision_numerics_enotation_large is open()""" + pass + + @pytest.mark.skip( + reason="Without a specific CAST, Databricks doesn't return floats with same precision that was selected." + ) + def test_float_coerce_round_trip(self): + """ + This automatically runs if requirements.literal_float_coercion is open() + + Without additional work, Databricks returns 15.75629997253418 when you SELECT 15.7563. + This is a potential area where we could override the Float literal processor to add a CAST. + Will leave to a PM to decide if we should do so. + """ + pass + + @pytest.mark.skip( + reason="Databricks sometimes only returns six digits of precision for the generic Float type" + ) + def test_float_custom_scale(self): + """This test automatically runs if requirements.precision_generic_float_type is open()""" + pass + + + +@pytest.mark.reviewed +class HasTableTest(HasTableTest): + """Databricks does not support temporary tables.""" + + @pytest.mark.skip(reason="Databricks does not support temporary tables.") + def test_has_table_temp_table(self): + pass + + @pytest.mark.skip(reason="Strange test design. See comments in test_suite.py") + def test_has_table_temp_view(self): + """Databricks supports temporary views but this test depends on requirements.has_temp_table, which we + explicitly close so that we can run other tests in this group. See the comment under has_temp_table in + requirements.py for details. + + From what I can see, there is no way to run this test since it will fail during setup if we mark has_temp_table + open(). It _might_ be possible to hijack this behaviour by implementing temp_table_keyword_args in our own + provision.py. Doing so would mean creating a real table during this class setup instead of a temp table. Then + we could just skip the temp table tests but run the temp view tests. But this test fixture doesn't cleanup its + temp tables and has no hook to do so. + + It would be ideal for SQLAlchemy to define a separate requirements.has_temp_views. + """ + pass + + +@pytest.mark.reviewed +class ComponentReflectionTestExtra(ComponentReflectionTestExtra): + @pytest.mark.skip(reason="This dialect does not support check constraints") + def test_get_check_constraints(self): + pass + + @pytest.mark.skip(reason="Databricks does not support indexes.") + def test_reflect_covering_index(self): + pass + + @pytest.mark.skip(reason="Databricks does not support indexes.") + def test_reflect_expression_based_indexes(self): + pass + + @pytest.mark.skip( + reason="Databricks doesn't enforce String or VARCHAR length limitations." + ) + def test_varchar_reflection(self): + """Even if a user specifies String(52), Databricks won't enforce that limit.""" + pass + + @pytest.mark.skip( + reason="This dialect doesn't implement foreign key options checks." + ) + def test_get_foreign_key_options(self): + """It's not clear from the test code what the expected output is here. Further research required.""" + pass + + +@pytest.mark.reviewed +class InsertBehaviorTest(InsertBehaviorTest): + @pytest.mark.skip( + reason="Databricks dialect doesn't implement empty inserts. See test_suite.py" + ) + def test_empty_insert(self): + """Empty inserts are possible using DEFAULT VALUES on Databricks. To implement it, we need + to hook into the SQLCompiler to render a no-op column list. With SQLAlchemy's default implementation + the request fails with a syntax error + """ + pass + + @pytest.mark.skip( + reason="Databricks dialect doesn't implement empty inserts. See test_suite.py" + ) + def test_empty_insert_multiple(self): + """Empty inserts are possible using DEFAULT VALUES on Databricks. To implement it, we need + to hook into the SQLCompiler to render a no-op column list. With SQLAlchemy's default implementation + the request fails with a syntax error + """ + pass + + @pytest.mark.skip( + reason="Test setup relies on implicit autoincrement. See test_suite.py" + ) + def test_autoclose_on_insert(self): + """The setup for this test creates a column with implicit autoincrement enabled. + This dialect does not implement implicit autoincrement - users must declare Identity() explicitly. + """ + pass + + @pytest.mark.skip( + reason="Test setup relies on implicit autoincrement. See test_suite.py" + ) + def test_insert_from_select_autoinc(self): + """Implicit autoincrement is not implemented in this dialect.""" + pass + + @pytest.mark.skip( + reason="Test setup relies on implicit autoincrement. See test_suite.py" + ) + def test_insert_from_select_autoinc_no_rows(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support INSERT ... RETURNING syntax") + def test_autoclose_on_insert_implicit_returning(self): + pass + + + +@pytest.mark.reviewed +class ComponentReflectionTest(ComponentReflectionTest): + """This test requires two schemas be present in the target Databricks workspace: + - The schema set in --dburi + - A second schema named "test_schema" + + Note that test_get_multi_foreign keys is flaky because DBR does not guarantee the order of data returned in DESCRIBE TABLE EXTENDED + """ + + @pytest.mark.skip( + reason="Comment reflection is possible but not enabled in this dialect" + ) + def test_get_multi_table_comment(self): + """There are 84 permutations of this test that are skipped.""" + pass + + @pytest.mark.skip(reason="Databricks doesn't support UNIQUE constraints") + def test_get_multi_unique_constraints(self): + pass + + @pytest.mark.skip( + reason="This dialect doesn't support get_table_options. See comment in test_suite.py" + ) + def test_multi_get_table_options_tables(self): + """It's not clear what the expected ouput from this method would even _be_. Requires research.""" + pass + + @pytest.mark.skip("This dialect doesn't implement get_view_definition") + def test_get_view_definition(self): + pass + + @pytest.mark.skip(reason="This dialect doesn't implement get_view_definition") + def test_get_view_definition_does_not_exist(self): + pass + + @pytest.mark.skip(reason="Strange test design. See test_suite.py") + def test_get_temp_view_names(self): + """While Databricks supports temporary views, this test creates a temp view aimed at a temp table. + Databricks doesn't support temp tables. So the test can never pass. + """ + pass + + @pytest.mark.skip("This dialect doesn't implement get_multi_pk_constraint") + def test_get_multi_pk_constraint(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support temp tables.") + def test_get_temp_table_columns(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support temp tables.") + def test_get_temp_table_indexes(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support temp tables.") + def test_get_temp_table_names(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support temp tables.") + def test_get_temp_table_unique_constraints(self): + pass + + @pytest.mark.skip(reason="Databricks doesn't support temp tables.") + def test_reflect_table_temp_table(self): + pass + + +@pytest.mark.reviewed +class TableDDLTest(TableDDLTest): + @pytest.mark.skip(reason="Databricks does not support indexes.") + def test_create_index_if_not_exists(self, connection): + """We could use requirements.index_reflection and requirements.index_ddl_if_exists + here to disable this but prefer a more meaningful skip message + """ + pass + + @pytest.mark.skip(reason="Databricks does not support indexes.") + def test_drop_index_if_exists(self, connection): + """We could use requirements.index_reflection and requirements.index_ddl_if_exists + here to disable this but prefer a more meaningful skip message + """ + pass + + @pytest.mark.skip( + reason="Comment reflection is possible but not implemented in this dialect." + ) + def test_add_table_comment(self, connection): + """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" + pass + + @pytest.mark.skip( + reason="Comment reflection is possible but not implemented in this dialect." + ) + def test_drop_table_comment(self, connection): + """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" + pass + + + @pytest.mark.reviewed class FutureTableDDLTest(FutureTableDDLTest): @pytest.mark.skip( diff --git a/src/databricks/sqlalchemy/test/_unsupported.py b/src/databricks/sqlalchemy/test/_unsupported.py index 6f3378860..96bc3d550 100644 --- a/src/databricks/sqlalchemy/test/_unsupported.py +++ b/src/databricks/sqlalchemy/test/_unsupported.py @@ -14,9 +14,36 @@ HasSequenceTest, HasSequenceTestEmpty, LongNameBlowoutTest, - ExceptionTest + ExceptionTest, + ArrayTest, + QuotedNameArgumentTest ) +@pytest.mark.reviewed +@pytest.mark.skip( + reason="Databricks does not support spaces in table names. See comment in test_suite.py" +) +class QuotedNameArgumentTest(QuotedNameArgumentTest): + """These tests are challenging. The whole test setup depends on a table with a name like `quote ' one` + which will never work on Databricks because table names can't contains spaces. But QuotedNamedArgumentTest + also checks the behaviour of DDL identifier preparation process. We need to override some of IdentifierPreparer + methods because these are the ultimate control for whether or not CHECK and UNIQUE constraints are emitted. + """ + + + +@pytest.mark.reviewed +@pytest.mark.skip( + reason="pysql doesn't support binding of array parameters. See test_suite.py" +) +class ArrayTest(ArrayTest): + """While Databricks supports ARRAY types, DBR cannot handle bound parameters of this type. + This makes them unusable to SQLAlchemy without some workaround. Potentially we could inline + the values of these parameters (which risks sql injection). + """ + + + @pytest.mark.reviewed class ExceptionTest(ExceptionTest): @pytest.mark.skip(reason="Databricks doesn't enforce primary key constraints.") From 7755387d16aafc60e2c33c8638a93ac410787036 Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Thu, 19 Oct 2023 13:15:55 -0400 Subject: [PATCH 03/23] Start enumerating skip reasons and update test_suite.py to import all these tests Signed-off-by: Jesse Whitehouse --- .../sqlalchemy/test/_unsupported.py | 68 ++-- src/databricks/sqlalchemy/test/test_suite.py | 343 +----------------- 2 files changed, 43 insertions(+), 368 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_unsupported.py b/src/databricks/sqlalchemy/test/_unsupported.py index 96bc3d550..038eeea3e 100644 --- a/src/databricks/sqlalchemy/test/_unsupported.py +++ b/src/databricks/sqlalchemy/test/_unsupported.py @@ -19,10 +19,27 @@ QuotedNameArgumentTest ) +from enum import Enum +class SkipReason(Enum): + + TABLE_NAMES_NO_SPACE = "spaces in table names" + PK_FK_NO_ENFORCE = "enforcing primary or foreign key restraints" + IDENTIFIER_LENGTH = "identifiers > 255 characters" + SEQUENCES = "SQL SEQUENCES" + INDEXES = "SQL INDEXes" + SYMBOL_CHARSET = "symbols expected by test" + CURSORS = "server-side cursors" + TRANSACTIONS = "transactions" + RETURNING = "INSERT ... RETURNING syntax" + GENERATED_COLUMNS = "computed / generated columns" + + +def render_skip_reason(rsn: SkipReason, setup_error=False) -> str: + prefix = "[BADSETUP]" if setup_error else "" + return f"{prefix}{rsn.name}: Databricks does not support {rsn.value}." + @pytest.mark.reviewed -@pytest.mark.skip( - reason="Databricks does not support spaces in table names. See comment in test_suite.py" -) +@pytest.mark.skip(reason=render_skip_reason(SkipReason.TABLE_NAMES_NO_SPACE, True)) class QuotedNameArgumentTest(QuotedNameArgumentTest): """These tests are challenging. The whole test setup depends on a table with a name like `quote ' one` which will never work on Databricks because table names can't contains spaces. But QuotedNamedArgumentTest @@ -45,47 +62,42 @@ class ArrayTest(ArrayTest): @pytest.mark.reviewed +@pytest.mark.skip(reason=render_skip_reason(SkipReason.PK_FK_NO_ENFORCE)) class ExceptionTest(ExceptionTest): - @pytest.mark.skip(reason="Databricks doesn't enforce primary key constraints.") - def test_integrity_error(self): - """Per Databricks documentation, primary and foreign key constraints are informational only + """Per Databricks documentation, primary and foreign key constraints are informational only and are not enforced. - + https://docs.databricks.com/api/workspace/tableconstraints - """ - pass + """ + pass + @pytest.mark.reviewed +@pytest.mark.skip(reason=render_skip_reason(SkipReason.IDENTIFIER_LENGTH)) class LongNameBlowoutTest(LongNameBlowoutTest): """These tests all include assertions that the tested name > 255 characters""" - @pytest.mark.skip( - reason="Databricks constraint names are limited to 255 characters" - ) - def test_long_convention_name(self): - pass - @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks doesn't support SEQUENCE server defaults") +@pytest.mark.skip(reason=render_skip_reason(SkipReason.SEQUENCES)) class HasSequenceTest(HasSequenceTest): pass @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks doesn't support SEQUENCE server defaults") +@pytest.mark.skip(reason=render_skip_reason(SkipReason.SEQUENCES)) class HasSequenceTestEmpty(HasSequenceTestEmpty): pass @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks does not support indexes.") +@pytest.mark.skip(reason=render_skip_reason(SkipReason.INDEXES)) class HasIndexTest(HasIndexTest): pass @pytest.mark.reviewed -@pytest.mark.skipped(reason="Databricks doesn't support unicode in symbol names") +@pytest.mark.skip(reason=render_skip_reason(SkipReason.SYMBOL_CHARSET)) class UnicodeSchemaTest(UnicodeSchemaTest): pass @@ -93,54 +105,54 @@ class UnicodeSchemaTest(UnicodeSchemaTest): @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks doesn't support server-side cursors.") +@pytest.mark.skip(reason=render_skip_reason(SkipReason.CURSORS)) class ServerSideCursorsTest(ServerSideCursorsTest): pass @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks doesn't allow percent signs in identifiers") +@pytest.mark.skip(reason=render_skip_reason(SkipReason.SYMBOL_CHARSET)) class PercentSchemaNamesTest(PercentSchemaNamesTest): pass @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks does not support transactions") +@pytest.mark.skip(reason=render_skip_reason(SkipReason.TRANSACTIONS)) class IsolationLevelTest(IsolationLevelTest): pass @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks does not support transactions") +@pytest.mark.skip(reason=render_skip_reason(SkipReason.TRANSACTIONS)) class AutocommitIsolationTest(AutocommitIsolationTest): pass @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks doesn't support INSERT ... RETURNING syntax") +@pytest.mark.skip(reason=render_skip_reason(SkipReason.RETURNING)) class ReturningTest(ReturningTest): pass @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks does not support sequences.") +@pytest.mark.skip(reason=render_skip_reason(SkipReason.SEQUENCES)) class SequenceTest(SequenceTest): pass @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks does not support sequences.") +@pytest.mark.skip(reason=render_skip_reason(SkipReason.SEQUENCES)) class SequenceCompilerTest(SequenceCompilerTest): pass @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks does not support computed / generated columns") +@pytest.mark.skip(reason=render_skip_reason(SkipReason.GENERATED_COLUMNS)) class ComputedColumnTest(ComputedColumnTest): pass @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks does not support computed / generated columns") +@pytest.mark.skip(reason=render_skip_reason(SkipReason.GENERATED_COLUMNS)) class ComputedReflectionTest(ComputedReflectionTest): pass diff --git a/src/databricks/sqlalchemy/test/test_suite.py b/src/databricks/sqlalchemy/test/test_suite.py index d9bbda89a..76f6f4e5e 100644 --- a/src/databricks/sqlalchemy/test/test_suite.py +++ b/src/databricks/sqlalchemy/test/test_suite.py @@ -1,6 +1,8 @@ # type: ignore from sqlalchemy.testing.suite import * -import pytest +from databricks.sqlalchemy.test._regression import * +from databricks.sqlalchemy.test._future import * +from databricks.sqlalchemy.test._unsupported import * # Test definitions are found here: # https://github.com/sqlalchemy/sqlalchemy/tree/main/lib/sqlalchemy/testing/suite @@ -24,43 +26,12 @@ # See further: https://github.com/sqlalchemy/sqlalchemy/blob/rel_1_4_48/README.dialects.rst -@pytest.mark.reviewed -@pytest.mark.skip(reason="pysql doesn't support binding of BINARY type parameters") -class BinaryTest(BinaryTest): - pass -@pytest.mark.reviewed -class NumericTest(NumericTest): - @pytest.mark.skip(reason="Databricks doesn't support E notation for DECIMAL types") - def test_enotation_decimal(self): - """This test automatically runs if requirements.precision_numerics_enotation_large is open()""" - pass - @pytest.mark.skip(reason="Databricks doesn't support E notation for DECIMAL types") - def test_enotation_decimal_large(self): - """This test automatically runs if requirements.precision_numerics_enotation_large is open()""" - pass - @pytest.mark.skip( - reason="Without a specific CAST, Databricks doesn't return floats with same precision that was selected." - ) - def test_float_coerce_round_trip(self): - """ - This automatically runs if requirements.literal_float_coercion is open() - Without additional work, Databricks returns 15.75629997253418 when you SELECT 15.7563. - This is a potential area where we could override the Float literal processor to add a CAST. - Will leave to a PM to decide if we should do so. - """ - pass - @pytest.mark.skip( - reason="Databricks sometimes only returns six digits of precision for the generic Float type" - ) - def test_float_custom_scale(self): - """This test automatically runs if requirements.precision_generic_float_type is open()""" - pass @@ -74,318 +45,10 @@ def test_float_custom_scale(self): -@pytest.mark.reviewed -class HasTableTest(HasTableTest): - """Databricks does not support temporary tables.""" - @pytest.mark.skip(reason="Databricks does not support temporary tables.") - def test_has_table_temp_table(self): - pass - @pytest.mark.skip(reason="Strange test design. See comments in test_suite.py") - def test_has_table_temp_view(self): - """Databricks supports temporary views but this test depends on requirements.has_temp_table, which we - explicitly close so that we can run other tests in this group. See the comment under has_temp_table in - requirements.py for details. - From what I can see, there is no way to run this test since it will fail during setup if we mark has_temp_table - open(). It _might_ be possible to hijack this behaviour by implementing temp_table_keyword_args in our own - provision.py. Doing so would mean creating a real table during this class setup instead of a temp table. Then - we could just skip the temp table tests but run the temp view tests. But this test fixture doesn't cleanup its - temp tables and has no hook to do so. - It would be ideal for SQLAlchemy to define a separate requirements.has_temp_views. - """ - pass -@pytest.mark.reviewed -@pytest.mark.skip( - reason="This dialect does not support implicit autoincrement. See comments in test_suite.py" -) -class LastrowidTest(LastrowidTest): - """SQLAlchemy docs describe that a column without an explicit Identity() may implicitly create one if autoincrement=True. - That is what this method tests. Databricks supports auto-incrementing IDENTITY columns but they must be explicitly - declared. This limitation is present in our dialect as well. Which means that SQLAlchemy's autoincrement setting of a column - is ignored. We emit a logging.WARN message if you try it. - In the future we could handle this autoincrement by implicitly calling the visit_identity_column() method of our DDLCompiler - when autoincrement=True. There is an example of this in the Microsoft SQL Server dialect: MSSDDLCompiler.get_column_specification - - For now, if you need to create a SQLAlchemy column with an auto-incrementing identity, you must set this explicitly in your column - definition by passing an Identity() to the column constructor. - """ - - pass - - -@pytest.mark.reviewed -class ComponentReflectionTestExtra(ComponentReflectionTestExtra): - @pytest.mark.skip(reason="This dialect does not support check constraints") - def test_get_check_constraints(self): - pass - - @pytest.mark.skip(reason="Databricks does not support indexes.") - def test_reflect_covering_index(self): - pass - - @pytest.mark.skip(reason="Databricks does not support indexes.") - def test_reflect_expression_based_indexes(self): - pass - - @pytest.mark.skip( - reason="Databricks doesn't enforce String or VARCHAR length limitations." - ) - def test_varchar_reflection(self): - """Even if a user specifies String(52), Databricks won't enforce that limit.""" - pass - - @pytest.mark.skip( - reason="This dialect doesn't implement foreign key options checks." - ) - def test_get_foreign_key_options(self): - """It's not clear from the test code what the expected output is here. Further research required.""" - pass - - -@pytest.mark.reviewed -class InsertBehaviorTest(InsertBehaviorTest): - @pytest.mark.skip( - reason="Databricks dialect doesn't implement empty inserts. See test_suite.py" - ) - def test_empty_insert(self): - """Empty inserts are possible using DEFAULT VALUES on Databricks. To implement it, we need - to hook into the SQLCompiler to render a no-op column list. With SQLAlchemy's default implementation - the request fails with a syntax error - """ - pass - - @pytest.mark.skip( - reason="Databricks dialect doesn't implement empty inserts. See test_suite.py" - ) - def test_empty_insert_multiple(self): - """Empty inserts are possible using DEFAULT VALUES on Databricks. To implement it, we need - to hook into the SQLCompiler to render a no-op column list. With SQLAlchemy's default implementation - the request fails with a syntax error - """ - pass - - @pytest.mark.skip( - reason="Test setup relies on implicit autoincrement. See test_suite.py" - ) - def test_autoclose_on_insert(self): - """The setup for this test creates a column with implicit autoincrement enabled. - This dialect does not implement implicit autoincrement - users must declare Identity() explicitly. - """ - pass - - @pytest.mark.skip( - reason="Test setup relies on implicit autoincrement. See test_suite.py" - ) - def test_insert_from_select_autoinc(self): - """Implicit autoincrement is not implemented in this dialect.""" - pass - - @pytest.mark.skip( - reason="Test setup relies on implicit autoincrement. See test_suite.py" - ) - def test_insert_from_select_autoinc_no_rows(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support INSERT ... RETURNING syntax") - def test_autoclose_on_insert_implicit_returning(self): - pass - - -@pytest.mark.reviewed -class ComponentReflectionTest(ComponentReflectionTest): - """This test requires two schemas be present in the target Databricks workspace: - - The schema set in --dburi - - A second schema named "test_schema" - - Note that test_get_multi_foreign keys is flaky because DBR does not guarantee the order of data returned in DESCRIBE TABLE EXTENDED - """ - - @pytest.mark.skip( - reason="Comment reflection is possible but not enabled in this dialect" - ) - def test_get_multi_table_comment(self): - """There are 84 permutations of this test that are skipped.""" - pass - - @pytest.mark.skip(reason="Databricks doesn't support UNIQUE constraints") - def test_get_multi_unique_constraints(self): - pass - - @pytest.mark.skip( - reason="This dialect doesn't support get_table_options. See comment in test_suite.py" - ) - def test_multi_get_table_options_tables(self): - """It's not clear what the expected ouput from this method would even _be_. Requires research.""" - pass - - @pytest.mark.skip("This dialect doesn't implement get_view_definition") - def test_get_view_definition(self): - pass - - @pytest.mark.skip(reason="This dialect doesn't implement get_view_definition") - def test_get_view_definition_does_not_exist(self): - pass - - @pytest.mark.skip(reason="Strange test design. See test_suite.py") - def test_get_temp_view_names(self): - """While Databricks supports temporary views, this test creates a temp view aimed at a temp table. - Databricks doesn't support temp tables. So the test can never pass. - """ - pass - - @pytest.mark.skip("This dialect doesn't implement get_multi_pk_constraint") - def test_get_multi_pk_constraint(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support temp tables.") - def test_get_temp_table_columns(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support temp tables.") - def test_get_temp_table_indexes(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support temp tables.") - def test_get_temp_table_names(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support temp tables.") - def test_get_temp_table_unique_constraints(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support temp tables.") - def test_reflect_table_temp_table(self): - pass - - -@pytest.mark.reviewed -class TableDDLTest(TableDDLTest): - @pytest.mark.skip(reason="Databricks does not support indexes.") - def test_create_index_if_not_exists(self, connection): - """We could use requirements.index_reflection and requirements.index_ddl_if_exists - here to disable this but prefer a more meaningful skip message - """ - pass - - @pytest.mark.skip(reason="Databricks does not support indexes.") - def test_drop_index_if_exists(self, connection): - """We could use requirements.index_reflection and requirements.index_ddl_if_exists - here to disable this but prefer a more meaningful skip message - """ - pass - - @pytest.mark.skip( - reason="Comment reflection is possible but not implemented in this dialect." - ) - def test_add_table_comment(self, connection): - """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" - pass - - @pytest.mark.skip( - reason="Comment reflection is possible but not implemented in this dialect." - ) - def test_drop_table_comment(self, connection): - """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" - pass - - -@pytest.mark.reviewed -@pytest.mark.skip( - reason="Databricks does not support spaces in table names. See comment in test_suite.py" -) -class QuotedNameArgumentTest(QuotedNameArgumentTest): - """These tests are challenging. The whole test setup depends on a table with a name like `quote ' one` - which will never work on Databricks because table names can't contains spaces. But QuotedNamedArgumentTest - also checks the behaviour of DDL identifier preparation process. We need to override some of IdentifierPreparer - methods because these are the ultimate control for whether or not CHECK and UNIQUE constraints are emitted. - """ - - -@pytest.mark.reviewed -@pytest.mark.skip( - reason="pysql doesn't support binding of array parameters. See test_suite.py" -) -class ArrayTest(ArrayTest): - """While Databricks supports ARRAY types, DBR cannot handle bound parameters of this type. - This makes them unusable to SQLAlchemy without some workaround. Potentially we could inline - the values of these parameters (which risks sql injection). - """ - - -@pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks doesn't support INSERT ... RETURNING syntax") -class ReturningText(ReturningTest): - pass - - -TUPLES_READ_AS_STRUCT_MSG = ( - "Databricks interprets tuple-like IN markers as though they are structs." -) - - -@pytest.mark.reviewed -class ExpandingBoundInTest(ExpandingBoundInTest): - @pytest.mark.skip(reason=TUPLES_READ_AS_STRUCT_MSG) - def test_empty_heterogeneous_tuples_bindparam(self): - pass - - @pytest.mark.skip(reason=TUPLES_READ_AS_STRUCT_MSG) - def test_empty_heterogeneous_tuples_direct(self): - pass - - @pytest.mark.skip(reason=TUPLES_READ_AS_STRUCT_MSG) - def test_empty_homogeneous_tuples_bindparam(self): - pass - - @pytest.mark.skip(reason=TUPLES_READ_AS_STRUCT_MSG) - def test_empty_homogeneous_tuples_direct(self): - pass - - -@pytest.mark.reviewed -class CTETest(CTETest): - """During the teardown for this test block, it tries to drop a constraint that it never named which raises - a compilation error. This could point to poor constraint reflection but our other constraint reflection - tests pass. Requires investigation. - """ - - @pytest.mark.skip( - reason="Databricks dialect doesn't implement multiple-table criteria within DELETE" - ) - def test_delete_from_round_trip(self): - """This may be supported by Databricks but has not been implemented here.""" - pass - - @pytest.mark.skip(reason="Databricks doesn't support recursive CTE") - def test_select_recursive_round_trip(self): - pass - - @pytest.mark.skip(reason="Unsupported by Databricks. See test_suite.py") - def test_delete_scalar_subq_round_trip(self): - """Error received is [UNSUPPORTED_SUBQUERY_EXPRESSION_CATEGORY.MUST_AGGREGATE_CORRELATED_SCALAR_SUBQUERY] - - This suggests a limitation of the platform. But a workaround may be possible if customers require it. - """ - pass - - -@pytest.mark.reviewed -class NormalizedNameTest(NormalizedNameTest): - @pytest.mark.skip(reason="Poor test design? See test_suite.py") - def test_get_table_names(self): - """I'm not clear how this test can ever pass given that it's assertion looks like this: - - ```python - eq_(tablenames[0].upper(), tablenames[0].lower()) - eq_(tablenames[1].upper(), tablenames[1].lower()) - ``` - - It's forcibly calling .upper() and .lower() on the same string and expecting them to be equal. - """ - pass From b0ee53dba912fdbfc74af59f6b807475234a3b62 Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Thu, 19 Oct 2023 14:15:58 -0400 Subject: [PATCH 04/23] Enumerate some FutureFeatures and 50% distributing tests between files Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 162 +++++++-- src/databricks/sqlalchemy/test/_regression.py | 319 +---------------- .../sqlalchemy/test/_unsupported.py | 329 ++++++++++++++++-- src/databricks/sqlalchemy/test/test_suite.py | 2 +- 4 files changed, 445 insertions(+), 367 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index 028ec62a3..4f06f4f99 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -18,60 +18,62 @@ IdentityAutoincrementTest, CTETest, NormalizedNameTest, - ExpandingBoundInTest, - LastrowidTest, - BinaryTest + BinaryTest, + ArrayTest, ) -@pytest.mark.reviewed -@pytest.mark.skip(reason="pysql doesn't support binding of BINARY type parameters") -class BinaryTest(BinaryTest): - pass - - +from databricks.sqlalchemy.test._unsupported import ( + FutureTableDDLTest, + TableDDLTest, + ComponentReflectionTest, + ComponentReflectionTestExtra, + InsertBehaviorTest, +) +from databricks.sqlalchemy.test._regression import ExpandingBoundInTest -@pytest.mark.reviewed -@pytest.mark.skip( - reason="This dialect does not support implicit autoincrement. See comments in test_suite.py" -) -class LastrowidTest(LastrowidTest): - """SQLAlchemy docs describe that a column without an explicit Identity() may implicitly create one if autoincrement=True. - That is what this method tests. Databricks supports auto-incrementing IDENTITY columns but they must be explicitly - declared. This limitation is present in our dialect as well. Which means that SQLAlchemy's autoincrement setting of a column - is ignored. We emit a logging.WARN message if you try it. +from enum import Enum - In the future we could handle this autoincrement by implicitly calling the visit_identity_column() method of our DDLCompiler - when autoincrement=True. There is an example of this in the Microsoft SQL Server dialect: MSSDDLCompiler.get_column_specification - For now, if you need to create a SQLAlchemy column with an auto-incrementing identity, you must set this explicitly in your column - definition by passing an Identity() to the column constructor. - """ +class FutureFeature(Enum): + TBL_COMMENTS = "table comment reflection" + VIEW_DEF = "get_view_definition method" + TBL_OPTS = "get_table_options method" + MULTI_PK = "get_multi_pk_constraint method" + CHECK = "CHECK constraint handling" + FK_OPTS = "foreign key option checking" + EMPTY_INSERT = "empty INSERT support" + ARRAY = "ARRAY type handling" + BINARY = "BINARY type handling" + TUPLE_LITERAL = "tuple-like IN markers completely" - pass +def render_future_feature(rsn: FutureFeature, extra=False) -> str: + postfix = " More detail in _future.py" if extra else "" + return f"[FUTURE][{rsn.name}]: This dialect doesn't implement {rsn.value}.{postfix}" -TUPLES_READ_AS_STRUCT_MSG = ( - "Databricks interprets tuple-like IN markers as though they are structs." -) +@pytest.mark.reviewed +@pytest.mark.skip(render_future_feature(FutureFeature.BINARY)) +class BinaryTest(BinaryTest): + pass @pytest.mark.reviewed class ExpandingBoundInTest(ExpandingBoundInTest): - @pytest.mark.skip(reason=TUPLES_READ_AS_STRUCT_MSG) + @pytest.mark.skip(render_future_feature(FutureFeature.TUPLE_LITERAL)) def test_empty_heterogeneous_tuples_bindparam(self): pass - @pytest.mark.skip(reason=TUPLES_READ_AS_STRUCT_MSG) + @pytest.mark.skip(render_future_feature(FutureFeature.TUPLE_LITERAL)) def test_empty_heterogeneous_tuples_direct(self): pass - @pytest.mark.skip(reason=TUPLES_READ_AS_STRUCT_MSG) + @pytest.mark.skip(render_future_feature(FutureFeature.TUPLE_LITERAL)) def test_empty_homogeneous_tuples_bindparam(self): pass - @pytest.mark.skip(reason=TUPLES_READ_AS_STRUCT_MSG) + @pytest.mark.skip(render_future_feature(FutureFeature.TUPLE_LITERAL)) def test_empty_homogeneous_tuples_direct(self): pass @@ -119,7 +121,6 @@ def test_delete_scalar_subq_round_trip(self): pass - @pytest.mark.reviewed @pytest.mark.skip( reason="Identity works. Test needs rewrite for Databricks. See comments in test_suite.py" @@ -149,6 +150,7 @@ def test_autoincrement_with_identity(self): test passes. """ + @pytest.mark.reviewed @pytest.mark.skip(reason="Implementation deferred. See test_suite.py") class BizarroCharacterFKResolutionTest(BizarroCharacterFKResolutionTest): @@ -178,6 +180,7 @@ class IdentityReflectionTest(IdentityReflectionTest): We could theoretically parse this from the contents of `SHOW CREATE TABLE` but that feels like a hack. """ + @pytest.mark.reviewed @pytest.mark.skip( reason="Databricks dialect doesn't implement JSON column types. See test_suite.py" @@ -197,6 +200,7 @@ class JSONLegacyStringCastIndexTest(JSONLegacyStringCastIndexTest): pass + @pytest.mark.reviewed class LikeFunctionsTest(LikeFunctionsTest): @pytest.mark.skip( @@ -214,7 +218,6 @@ def test_regexp_match(self): pass - @pytest.mark.reviewed @pytest.mark.skip( reason="Datetime handling doesn't handle timezones well. Priority to fix." @@ -228,6 +231,7 @@ class DateTimeTZTest(DateTimeTZTest): pass + @pytest.mark.reviewed @pytest.mark.skip( reason="Databricks dialect does not implement timezone support for Timestamp() types. See test_suite.py" @@ -268,7 +272,6 @@ class SimpleUpdateDeleteTest(SimpleUpdateDeleteTest): pass - @pytest.mark.reviewed @pytest.mark.skip(reason="Dialect doesn't implement provision.py See test_suite.py") class WeCanSetDefaultSchemaWEventsTest(WeCanSetDefaultSchemaWEventsTest): @@ -291,3 +294,94 @@ class FutureWeCanSetDefaultSchemaWEventsTest(FutureWeCanSetDefaultSchemaWEventsT """ pass + + +@pytest.mark.reviewed +class FutureTableDDLTest(FutureTableDDLTest): + @pytest.mark.skip(reason=render_future_feature(FutureFeature.TBL_COMMENTS)) + def test_add_table_comment(self): + """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" + pass + + @pytest.mark.skip(reason=render_future_feature(FutureFeature.TBL_COMMENTS)) + def test_drop_table_comment(self): + """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" + pass + + +@pytest.mark.reviewed +class TableDDLTest(TableDDLTest): + @pytest.mark.skip(reason=render_future_feature(FutureFeature.TBL_COMMENTS)) + def test_add_table_comment(self, connection): + """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" + pass + + @pytest.mark.skip(reason=render_future_feature(FutureFeature.TBL_COMMENTS)) + def test_drop_table_comment(self, connection): + """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" + pass + + +@pytest.mark.reviewed +class ComponentReflectionTest(ComponentReflectionTest): + @pytest.mark.skip(reason=render_future_feature(FutureFeature.TBL_COMMENTS)) + def test_get_multi_table_comment(self): + """There are 84 permutations of this test that are skipped.""" + pass + + @pytest.mark.skip(reason=render_future_feature(FutureFeature.TBL_OPTS, True)) + def test_multi_get_table_options_tables(self): + """It's not clear what the expected ouput from this method would even _be_. Requires research.""" + pass + + @pytest.mark.skip(render_future_feature(FutureFeature.VIEW_DEF)) + def test_get_view_definition(self): + pass + + @pytest.mark.skip(render_future_feature(FutureFeature.VIEW_DEF)) + def test_get_view_definition_does_not_exist(self): + pass + + @pytest.mark.skip(render_future_feature(FutureFeature.MULTI_PK)) + def test_get_multi_pk_constraint(self): + pass + + +@pytest.mark.reviewed +class ComponentReflectionTestExtra(ComponentReflectionTestExtra): + @pytest.mark.skip(render_future_feature(FutureFeature.CHECK)) + def test_get_check_constraints(self): + pass + + @pytest.mark.skip(render_future_feature(FutureFeature.FK_OPTS)) + def test_get_foreign_key_options(self): + """It's not clear from the test code what the expected output is here. Further research required.""" + pass + + +@pytest.mark.reviewed +class InsertBehaviorTest(InsertBehaviorTest): + @pytest.mark.skip(render_future_feature(FutureFeature.EMPTY_INSERT, True)) + def test_empty_insert(self): + """Empty inserts are possible using DEFAULT VALUES on Databricks. To implement it, we need + to hook into the SQLCompiler to render a no-op column list. With SQLAlchemy's default implementation + the request fails with a syntax error + """ + pass + + @pytest.mark.skip(render_future_feature(FutureFeature.EMPTY_INSERT, True)) + def test_empty_insert_multiple(self): + """Empty inserts are possible using DEFAULT VALUES on Databricks. To implement it, we need + to hook into the SQLCompiler to render a no-op column list. With SQLAlchemy's default implementation + the request fails with a syntax error + """ + pass + + +@pytest.mark.reviewed +@pytest.mark.skip(render_future_feature(FutureFeature.ARRAY)) +class ArrayTest(ArrayTest): + """While Databricks supports ARRAY types, DBR cannot handle bound parameters of this type. + This makes them unusable to SQLAlchemy without some workaround. Potentially we could inline + the values of these parameters (which risks sql injection). + """ diff --git a/src/databricks/sqlalchemy/test/_regression.py b/src/databricks/sqlalchemy/test/_regression.py index 356b4fd5d..b56929de0 100644 --- a/src/databricks/sqlalchemy/test/_regression.py +++ b/src/databricks/sqlalchemy/test/_regression.py @@ -42,145 +42,29 @@ InsertBehaviorTest, ComponentReflectionTestExtra, HasTableTest, - NumericTest + NumericTest, + ExpandingBoundInTest ) @pytest.mark.reviewed class NumericTest(NumericTest): - @pytest.mark.skip(reason="Databricks doesn't support E notation for DECIMAL types") - def test_enotation_decimal(self): - """This test automatically runs if requirements.precision_numerics_enotation_large is open()""" - pass - - @pytest.mark.skip(reason="Databricks doesn't support E notation for DECIMAL types") - def test_enotation_decimal_large(self): - """This test automatically runs if requirements.precision_numerics_enotation_large is open()""" - pass - - @pytest.mark.skip( - reason="Without a specific CAST, Databricks doesn't return floats with same precision that was selected." - ) - def test_float_coerce_round_trip(self): - """ - This automatically runs if requirements.literal_float_coercion is open() - - Without additional work, Databricks returns 15.75629997253418 when you SELECT 15.7563. - This is a potential area where we could override the Float literal processor to add a CAST. - Will leave to a PM to decide if we should do so. - """ - pass - - @pytest.mark.skip( - reason="Databricks sometimes only returns six digits of precision for the generic Float type" - ) - def test_float_custom_scale(self): - """This test automatically runs if requirements.precision_generic_float_type is open()""" - pass + pass @pytest.mark.reviewed class HasTableTest(HasTableTest): - """Databricks does not support temporary tables.""" - - @pytest.mark.skip(reason="Databricks does not support temporary tables.") - def test_has_table_temp_table(self): - pass - - @pytest.mark.skip(reason="Strange test design. See comments in test_suite.py") - def test_has_table_temp_view(self): - """Databricks supports temporary views but this test depends on requirements.has_temp_table, which we - explicitly close so that we can run other tests in this group. See the comment under has_temp_table in - requirements.py for details. - - From what I can see, there is no way to run this test since it will fail during setup if we mark has_temp_table - open(). It _might_ be possible to hijack this behaviour by implementing temp_table_keyword_args in our own - provision.py. Doing so would mean creating a real table during this class setup instead of a temp table. Then - we could just skip the temp table tests but run the temp view tests. But this test fixture doesn't cleanup its - temp tables and has no hook to do so. - - It would be ideal for SQLAlchemy to define a separate requirements.has_temp_views. - """ - pass + pass @pytest.mark.reviewed class ComponentReflectionTestExtra(ComponentReflectionTestExtra): - @pytest.mark.skip(reason="This dialect does not support check constraints") - def test_get_check_constraints(self): - pass - - @pytest.mark.skip(reason="Databricks does not support indexes.") - def test_reflect_covering_index(self): - pass - - @pytest.mark.skip(reason="Databricks does not support indexes.") - def test_reflect_expression_based_indexes(self): - pass - - @pytest.mark.skip( - reason="Databricks doesn't enforce String or VARCHAR length limitations." - ) - def test_varchar_reflection(self): - """Even if a user specifies String(52), Databricks won't enforce that limit.""" - pass - - @pytest.mark.skip( - reason="This dialect doesn't implement foreign key options checks." - ) - def test_get_foreign_key_options(self): - """It's not clear from the test code what the expected output is here. Further research required.""" - pass + pass @pytest.mark.reviewed class InsertBehaviorTest(InsertBehaviorTest): - @pytest.mark.skip( - reason="Databricks dialect doesn't implement empty inserts. See test_suite.py" - ) - def test_empty_insert(self): - """Empty inserts are possible using DEFAULT VALUES on Databricks. To implement it, we need - to hook into the SQLCompiler to render a no-op column list. With SQLAlchemy's default implementation - the request fails with a syntax error - """ - pass - - @pytest.mark.skip( - reason="Databricks dialect doesn't implement empty inserts. See test_suite.py" - ) - def test_empty_insert_multiple(self): - """Empty inserts are possible using DEFAULT VALUES on Databricks. To implement it, we need - to hook into the SQLCompiler to render a no-op column list. With SQLAlchemy's default implementation - the request fails with a syntax error - """ - pass - - @pytest.mark.skip( - reason="Test setup relies on implicit autoincrement. See test_suite.py" - ) - def test_autoclose_on_insert(self): - """The setup for this test creates a column with implicit autoincrement enabled. - This dialect does not implement implicit autoincrement - users must declare Identity() explicitly. - """ - pass - - @pytest.mark.skip( - reason="Test setup relies on implicit autoincrement. See test_suite.py" - ) - def test_insert_from_select_autoinc(self): - """Implicit autoincrement is not implemented in this dialect.""" - pass - - @pytest.mark.skip( - reason="Test setup relies on implicit autoincrement. See test_suite.py" - ) - def test_insert_from_select_autoinc_no_rows(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support INSERT ... RETURNING syntax") - def test_autoclose_on_insert_implicit_returning(self): - pass - + pass @pytest.mark.reviewed @@ -191,202 +75,26 @@ class ComponentReflectionTest(ComponentReflectionTest): Note that test_get_multi_foreign keys is flaky because DBR does not guarantee the order of data returned in DESCRIBE TABLE EXTENDED """ - - @pytest.mark.skip( - reason="Comment reflection is possible but not enabled in this dialect" - ) - def test_get_multi_table_comment(self): - """There are 84 permutations of this test that are skipped.""" - pass - - @pytest.mark.skip(reason="Databricks doesn't support UNIQUE constraints") - def test_get_multi_unique_constraints(self): - pass - - @pytest.mark.skip( - reason="This dialect doesn't support get_table_options. See comment in test_suite.py" - ) - def test_multi_get_table_options_tables(self): - """It's not clear what the expected ouput from this method would even _be_. Requires research.""" - pass - - @pytest.mark.skip("This dialect doesn't implement get_view_definition") - def test_get_view_definition(self): - pass - - @pytest.mark.skip(reason="This dialect doesn't implement get_view_definition") - def test_get_view_definition_does_not_exist(self): - pass - - @pytest.mark.skip(reason="Strange test design. See test_suite.py") - def test_get_temp_view_names(self): - """While Databricks supports temporary views, this test creates a temp view aimed at a temp table. - Databricks doesn't support temp tables. So the test can never pass. - """ - pass - - @pytest.mark.skip("This dialect doesn't implement get_multi_pk_constraint") - def test_get_multi_pk_constraint(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support temp tables.") - def test_get_temp_table_columns(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support temp tables.") - def test_get_temp_table_indexes(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support temp tables.") - def test_get_temp_table_names(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support temp tables.") - def test_get_temp_table_unique_constraints(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support temp tables.") - def test_reflect_table_temp_table(self): - pass - + pass @pytest.mark.reviewed class TableDDLTest(TableDDLTest): - @pytest.mark.skip(reason="Databricks does not support indexes.") - def test_create_index_if_not_exists(self, connection): - """We could use requirements.index_reflection and requirements.index_ddl_if_exists - here to disable this but prefer a more meaningful skip message - """ - pass - - @pytest.mark.skip(reason="Databricks does not support indexes.") - def test_drop_index_if_exists(self, connection): - """We could use requirements.index_reflection and requirements.index_ddl_if_exists - here to disable this but prefer a more meaningful skip message - """ - pass - - @pytest.mark.skip( - reason="Comment reflection is possible but not implemented in this dialect." - ) - def test_add_table_comment(self, connection): - """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" - pass - - @pytest.mark.skip( - reason="Comment reflection is possible but not implemented in this dialect." - ) - def test_drop_table_comment(self, connection): - """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" - pass - + pass @pytest.mark.reviewed class FutureTableDDLTest(FutureTableDDLTest): - @pytest.mark.skip( - reason="Comment reflection is possible but not implemented in this dialect." - ) - def test_add_table_comment(self): - """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" - pass - - @pytest.mark.skip( - reason="Comment reflection is possible but not implemented in this dialect." - ) - def test_drop_table_comment(self): - """We could use requirements.comment_reflection here to disable this but prefer a more meaningful skip message""" - pass - - @pytest.mark.skip(reason="Databricks does not support indexes.") - def test_create_index_if_not_exists(self): - """We could use requirements.index_reflection and requirements.index_ddl_if_exists - here to disable this but prefer a more meaningful skip message - """ - pass - - @pytest.mark.skip(reason="Databricks does not support indexes.") - def test_drop_index_if_exists(self): - """We could use requirements.index_reflection and requirements.index_ddl_if_exists - here to disable this but prefer a more meaningful skip message - """ - pass + pass @pytest.mark.reviewed class FetchLimitOffsetTest(FetchLimitOffsetTest): - @pytest.mark.flaky - @pytest.mark.skip( - reason="Insertion order on Databricks is not deterministic. See comment in test_suite.py." - ) - def test_limit_render_multiple_times(self): - """This test depends on the order that records are inserted into the table. It's passing criteria requires that - a record inserted with id=1 is the first record returned when no ORDER BY clause is specified. But Databricks occasionally - INSERTS in a different order, which makes this test seem to fail. The test is flaky, but the underlying functionality - (can multiple LIMIT clauses be rendered) is not broken. - - Unclear if this is a bug in Databricks, Delta, or some race-condition in the test itself. - """ - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_bound_fetch_offset(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_fetch_offset_no_order(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_fetch_offset_nobinds(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_simple_fetch(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_simple_fetch_offset(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_simple_fetch_percent(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_simple_fetch_percent_ties(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_simple_fetch_ties(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_expr_fetch_offset(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_fetch_offset_percent(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_fetch_offset_percent_ties(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_fetch_offset_ties(self): - pass - - @pytest.mark.skip(reason="Databricks doesn't support FETCH clauses") - def test_fetch_offset_ties_exact_number(self): - pass + pass @pytest.mark.reviewed class UuidTest(UuidTest): - @pytest.mark.skip(reason="Databricks doesn't support INSERT ... RETURNING syntax") - def test_uuid_returning(self): - pass + pass @pytest.mark.reviewed @@ -556,3 +264,8 @@ class UnicodeVarcharTest(UnicodeVarcharTest): @pytest.mark.reviewed class TableNoColumnsTest(TableNoColumnsTest): pass + + +@pytest.mark.reviewed +class ExpandingBoundInTest(ExpandingBoundInTest): + pass \ No newline at end of file diff --git a/src/databricks/sqlalchemy/test/_unsupported.py b/src/databricks/sqlalchemy/test/_unsupported.py index 038eeea3e..1dff0a491 100644 --- a/src/databricks/sqlalchemy/test/_unsupported.py +++ b/src/databricks/sqlalchemy/test/_unsupported.py @@ -1,4 +1,7 @@ import pytest +from typing import Union + +# These are test suites that are fully skipped with a SkipReason from sqlalchemy.testing.suite import ( SequenceTest, SequenceCompilerTest, @@ -15,15 +18,27 @@ HasSequenceTestEmpty, LongNameBlowoutTest, ExceptionTest, - ArrayTest, - QuotedNameArgumentTest + QuotedNameArgumentTest, + LastrowidTest +) + +from databricks.sqlalchemy.test._regression import ( + FetchLimitOffsetTest, + UuidTest, + FutureTableDDLTest, + TableDDLTest, + ComponentReflectionTest, + NumericTest, + HasTableTest, + ComponentReflectionTestExtra, + InsertBehaviorTest ) from enum import Enum -class SkipReason(Enum): - TABLE_NAMES_NO_SPACE = "spaces in table names" - PK_FK_NO_ENFORCE = "enforcing primary or foreign key restraints" + +class SkipReason(Enum): + ENFORCE_KEYS = "enforcing primary or foreign key restraints" IDENTIFIER_LENGTH = "identifiers > 255 characters" SEQUENCES = "SQL SEQUENCES" INDEXES = "SQL INDEXes" @@ -31,15 +46,24 @@ class SkipReason(Enum): CURSORS = "server-side cursors" TRANSACTIONS = "transactions" RETURNING = "INSERT ... RETURNING syntax" - GENERATED_COLUMNS = "computed / generated columns" - - -def render_skip_reason(rsn: SkipReason, setup_error=False) -> str: + GENERATED_COLUMNS = "computed / generated columns" + FETCH = "fetch clauses" + IMPLICIT_ORDER = "deterministic return order if ORDER BY is not present" + UNIQUE = "UNIQUE constraints" + TEMP_TBL = "temporary tables" + DECIMAL_FEAT = "required decimal features" + IMPL_FLOAT_PREC = "required implicit float precision" + STRING_FEAT = "required STRING type features" + AUTO_INC = "implicit AUTO_INCREMENT" + +def render_skip_reason(rsn: SkipReason, setup_error=False, extra=False) -> str: prefix = "[BADSETUP]" if setup_error else "" - return f"{prefix}{rsn.name}: Databricks does not support {rsn.value}." + postfix = " More detail in _unsupported.py" if extra else "" + return f"[UNSUPPORTED]{prefix}[{rsn.name}]: Databricks does not support {rsn.value}.{postfix}" + @pytest.mark.reviewed -@pytest.mark.skip(reason=render_skip_reason(SkipReason.TABLE_NAMES_NO_SPACE, True)) +@pytest.mark.skip(reason=render_skip_reason(SkipReason.SYMBOL_CHARSET, True, True)) class QuotedNameArgumentTest(QuotedNameArgumentTest): """These tests are challenging. The whole test setup depends on a table with a name like `quote ' one` which will never work on Databricks because table names can't contains spaces. But QuotedNamedArgumentTest @@ -49,26 +73,16 @@ class QuotedNameArgumentTest(QuotedNameArgumentTest): -@pytest.mark.reviewed -@pytest.mark.skip( - reason="pysql doesn't support binding of array parameters. See test_suite.py" -) -class ArrayTest(ArrayTest): - """While Databricks supports ARRAY types, DBR cannot handle bound parameters of this type. - This makes them unusable to SQLAlchemy without some workaround. Potentially we could inline - the values of these parameters (which risks sql injection). - """ - - @pytest.mark.reviewed -@pytest.mark.skip(reason=render_skip_reason(SkipReason.PK_FK_NO_ENFORCE)) +@pytest.mark.skip(reason=render_skip_reason(SkipReason.ENFORCE_KEYS)) class ExceptionTest(ExceptionTest): """Per Databricks documentation, primary and foreign key constraints are informational only - and are not enforced. - - https://docs.databricks.com/api/workspace/tableconstraints + and are not enforced. + + https://docs.databricks.com/api/workspace/tableconstraints """ + pass @@ -76,6 +90,7 @@ class ExceptionTest(ExceptionTest): @pytest.mark.skip(reason=render_skip_reason(SkipReason.IDENTIFIER_LENGTH)) class LongNameBlowoutTest(LongNameBlowoutTest): """These tests all include assertions that the tested name > 255 characters""" + pass @pytest.mark.reviewed @@ -102,15 +117,12 @@ class UnicodeSchemaTest(UnicodeSchemaTest): pass - - @pytest.mark.reviewed @pytest.mark.skip(reason=render_skip_reason(SkipReason.CURSORS)) class ServerSideCursorsTest(ServerSideCursorsTest): pass - @pytest.mark.reviewed @pytest.mark.skip(reason=render_skip_reason(SkipReason.SYMBOL_CHARSET)) class PercentSchemaNamesTest(PercentSchemaNamesTest): @@ -146,6 +158,7 @@ class SequenceTest(SequenceTest): class SequenceCompilerTest(SequenceCompilerTest): pass + @pytest.mark.reviewed @pytest.mark.skip(reason=render_skip_reason(SkipReason.GENERATED_COLUMNS)) class ComputedColumnTest(ComputedColumnTest): @@ -156,3 +169,261 @@ class ComputedColumnTest(ComputedColumnTest): @pytest.mark.skip(reason=render_skip_reason(SkipReason.GENERATED_COLUMNS)) class ComputedReflectionTest(ComputedReflectionTest): pass + + +class FetchLimitOffsetTest(FetchLimitOffsetTest): + @pytest.mark.flaky + @pytest.mark.skip(reason=render_skip_reason(SkipReason.IMPLICIT_ORDER, extra=True)) + def test_limit_render_multiple_times(self): + """This test depends on the order that records are inserted into the table. It's passing criteria requires that + a record inserted with id=1 is the first record returned when no ORDER BY clause is specified. But Databricks occasionally + INSERTS in a different order, which makes this test seem to fail. The test is flaky, but the underlying functionality + (can multiple LIMIT clauses be rendered) is not broken. + + Unclear if this is a bug in Databricks, Delta, or some race-condition in the test itself. + """ + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.FETCH)) + def test_bound_fetch_offset(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.FETCH)) + def test_fetch_offset_no_order(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.FETCH)) + def test_fetch_offset_nobinds(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.FETCH)) + def test_simple_fetch(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.FETCH)) + def test_simple_fetch_offset(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.FETCH)) + def test_simple_fetch_percent(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.FETCH)) + def test_simple_fetch_percent_ties(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.FETCH)) + def test_simple_fetch_ties(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.FETCH)) + def test_expr_fetch_offset(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.FETCH)) + def test_fetch_offset_percent(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.FETCH)) + def test_fetch_offset_percent_ties(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.FETCH)) + def test_fetch_offset_ties(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.FETCH)) + def test_fetch_offset_ties_exact_number(self): + pass + + +class UuidTest(UuidTest): + @pytest.mark.skip(reason=render_skip_reason(SkipReason.RETURNING)) + def test_uuid_returning(self): + pass + + +@pytest.mark.reviewed +class FutureTableDDLTest(FutureTableDDLTest): + + @pytest.mark.skip(render_skip_reason(SkipReason.INDEXES)) + def test_create_index_if_not_exists(self): + """We could use requirements.index_reflection and requirements.index_ddl_if_exists + here to disable this but prefer a more meaningful skip message + """ + pass + + @pytest.mark.skip(render_skip_reason(SkipReason.INDEXES)) + def test_drop_index_if_exists(self): + """We could use requirements.index_reflection and requirements.index_ddl_if_exists + here to disable this but prefer a more meaningful skip message + """ + pass + + +@pytest.mark.reviewed +class TableDDLTest(TableDDLTest): + @pytest.mark.skip(reason=render_skip_reason(SkipReason.INDEXES)) + def test_create_index_if_not_exists(self, connection): + """We could use requirements.index_reflection and requirements.index_ddl_if_exists + here to disable this but prefer a more meaningful skip message + """ + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.INDEXES)) + def test_drop_index_if_exists(self, connection): + """We could use requirements.index_reflection and requirements.index_ddl_if_exists + here to disable this but prefer a more meaningful skip message + """ + pass + + +@pytest.mark.reviewed +class ComponentReflectionTest(ComponentReflectionTest): + """This test requires two schemas be present in the target Databricks workspace: + - The schema set in --dburi + - A second schema named "test_schema" + + Note that test_get_multi_foreign keys is flaky because DBR does not guarantee the order of data returned in DESCRIBE TABLE EXTENDED + """ + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.UNIQUE)) + def test_get_multi_unique_constraints(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.TEMP_TBL, True, True)) + def test_get_temp_view_names(self): + """While Databricks supports temporary views, this test creates a temp view aimed at a temp table. + Databricks doesn't support temp tables. So the test can never pass. + """ + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.TEMP_TBL)) + def test_get_temp_table_columns(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.TEMP_TBL)) + def test_get_temp_table_indexes(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.TEMP_TBL)) + def test_get_temp_table_names(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.TEMP_TBL)) + def test_get_temp_table_unique_constraints(self): + pass + + @pytest.mark.skip(reason=render_skip_reason(SkipReason.TEMP_TBL)) + def test_reflect_table_temp_table(self): + pass + + +@pytest.mark.reviewed +class NumericTest(NumericTest): + @pytest.mark.skip(render_skip_reason(SkipReason.DECIMAL_FEAT)) + def test_enotation_decimal(self): + """This test automatically runs if requirements.precision_numerics_enotation_large is open()""" + pass + + @pytest.mark.skip(render_skip_reason(SkipReason.DECIMAL_FEAT)) + def test_enotation_decimal_large(self): + """This test automatically runs if requirements.precision_numerics_enotation_large is open()""" + pass + + @pytest.mark.skip(render_skip_reason(SkipReason.IMPL_FLOAT_PREC, extra=True)) + def test_float_coerce_round_trip(self): + """ + This automatically runs if requirements.literal_float_coercion is open() + + Without additional work, Databricks returns 15.75629997253418 when you SELECT 15.7563. + This is a potential area where we could override the Float literal processor to add a CAST. + Will leave to a PM to decide if we should do so. + """ + pass + + @pytest.mark.skip(render_skip_reason(SkipReason.IMPL_FLOAT_PREC, extra=True)) + def test_float_custom_scale(self): + """This test automatically runs if requirements.precision_generic_float_type is open()""" + pass + +@pytest.mark.reviewed +class HasTableTest(HasTableTest): + """Databricks does not support temporary tables.""" + + @pytest.mark.skip(render_skip_reason(SkipReason.TEMP_TBL)) + def test_has_table_temp_table(self): + pass + + @pytest.mark.skip(render_skip_reason(SkipReason.TEMP_TBL,True,True)) + def test_has_table_temp_view(self): + """Databricks supports temporary views but this test depends on requirements.has_temp_table, which we + explicitly close so that we can run other tests in this group. See the comment under has_temp_table in + requirements.py for details. + + From what I can see, there is no way to run this test since it will fail during setup if we mark has_temp_table + open(). It _might_ be possible to hijack this behaviour by implementing temp_table_keyword_args in our own + provision.py. Doing so would mean creating a real table during this class setup instead of a temp table. Then + we could just skip the temp table tests but run the temp view tests. But this test fixture doesn't cleanup its + temp tables and has no hook to do so. + + It would be ideal for SQLAlchemy to define a separate requirements.has_temp_views. + """ + pass + +@pytest.mark.reviewed +class ComponentReflectionTestExtra(ComponentReflectionTestExtra): + + @pytest.mark.skip(render_skip_reason(SkipReason.INDEXES)) + def test_reflect_covering_index(self): + pass + + @pytest.mark.skip(render_skip_reason(SkipReason.INDEXES)) + def test_reflect_expression_based_indexes(self): + pass + + @pytest.mark.skip(render_skip_reason(SkipReason.STRING_FEAT, extra=True)) + def test_varchar_reflection(self): + """Databricks doesn't enforce string length limitations like STRING(255). """ + pass + + +@pytest.mark.reviewed +class InsertBehaviorTest(InsertBehaviorTest): + + @pytest.mark.skip(render_skip_reason(SkipReason.AUTO_INC, True, True)) + def test_autoclose_on_insert(self): + """The setup for this test creates a column with implicit autoincrement enabled. + This dialect does not implement implicit autoincrement - users must declare Identity() explicitly. + """ + pass + + @pytest.mark.skip(render_skip_reason(SkipReason.AUTO_INC, True, True)) + def test_insert_from_select_autoinc(self): + """Implicit autoincrement is not implemented in this dialect.""" + pass + + @pytest.mark.skip(render_skip_reason(SkipReason.AUTO_INC, True, True)) + def test_insert_from_select_autoinc_no_rows(self): + pass + + @pytest.mark.skip(render_skip_reason(SkipReason.RETURNING)) + def test_autoclose_on_insert_implicit_returning(self): + pass + +@pytest.mark.reviewed +@pytest.mark.skip(render_skip_reason(SkipReason.AUTO_INC, extra=True)) +class LastrowidTest(LastrowidTest): + """SQLAlchemy docs describe that a column without an explicit Identity() may implicitly create one if autoincrement=True. + That is what this method tests. Databricks supports auto-incrementing IDENTITY columns but they must be explicitly + declared. This limitation is present in our dialect as well. Which means that SQLAlchemy's autoincrement setting of a column + is ignored. We emit a logging.WARN message if you try it. + + In the future we could handle this autoincrement by implicitly calling the visit_identity_column() method of our DDLCompiler + when autoincrement=True. There is an example of this in the Microsoft SQL Server dialect: MSSDDLCompiler.get_column_specification + + For now, if you need to create a SQLAlchemy column with an auto-incrementing identity, you must set this explicitly in your column + definition by passing an Identity() to the column constructor. + """ + + pass \ No newline at end of file diff --git a/src/databricks/sqlalchemy/test/test_suite.py b/src/databricks/sqlalchemy/test/test_suite.py index 76f6f4e5e..f4b91b9d5 100644 --- a/src/databricks/sqlalchemy/test/test_suite.py +++ b/src/databricks/sqlalchemy/test/test_suite.py @@ -1,8 +1,8 @@ # type: ignore from sqlalchemy.testing.suite import * from databricks.sqlalchemy.test._regression import * -from databricks.sqlalchemy.test._future import * from databricks.sqlalchemy.test._unsupported import * +from databricks.sqlalchemy.test._future import * # Test definitions are found here: # https://github.com/sqlalchemy/sqlalchemy/tree/main/lib/sqlalchemy/testing/suite From 56ee95ab63dc410fb32e6488b079e2bd1b63079d Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Fri, 20 Oct 2023 11:58:03 -0400 Subject: [PATCH 05/23] Split CTETest between regression, unsupported, and future Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 32 ++++---------- src/databricks/sqlalchemy/test/_regression.py | 7 +++- .../sqlalchemy/test/_unsupported.py | 42 ++++++++++++++----- 3 files changed, 45 insertions(+), 36 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index 4f06f4f99..20eb4e62f 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -16,7 +16,6 @@ IdentityReflectionTest, IdentityColumnTest, IdentityAutoincrementTest, - CTETest, NormalizedNameTest, BinaryTest, ArrayTest, @@ -28,6 +27,7 @@ ComponentReflectionTest, ComponentReflectionTestExtra, InsertBehaviorTest, + CTETest, ) from databricks.sqlalchemy.test._regression import ExpandingBoundInTest @@ -43,9 +43,11 @@ class FutureFeature(Enum): CHECK = "CHECK constraint handling" FK_OPTS = "foreign key option checking" EMPTY_INSERT = "empty INSERT support" - ARRAY = "ARRAY type handling" - BINARY = "BINARY type handling" + ARRAY = "ARRAY column type handling" + BINARY = "BINARY column type handling" + JSON = "JSON column type handling" TUPLE_LITERAL = "tuple-like IN markers completely" + CTE_FEAT = "required CTE features" def render_future_feature(rsn: FutureFeature, extra=False) -> str: @@ -94,30 +96,10 @@ def test_get_table_names(self): pass -@pytest.mark.reviewed class CTETest(CTETest): - """During the teardown for this test block, it tries to drop a constraint that it never named which raises - a compilation error. This could point to poor constraint reflection but our other constraint reflection - tests pass. Requires investigation. - """ - - @pytest.mark.skip( - reason="Databricks dialect doesn't implement multiple-table criteria within DELETE" - ) + @pytest.mark.skip(render_future_feature(FutureFeature.CTE_FEAT, True)) def test_delete_from_round_trip(self): - """This may be supported by Databricks but has not been implemented here.""" - pass - - @pytest.mark.skip(reason="Databricks doesn't support recursive CTE") - def test_select_recursive_round_trip(self): - pass - - @pytest.mark.skip(reason="Unsupported by Databricks. See test_suite.py") - def test_delete_scalar_subq_round_trip(self): - """Error received is [UNSUPPORTED_SUBQUERY_EXPRESSION_CATEGORY.MUST_AGGREGATE_CORRELATED_SCALAR_SUBQUERY] - - This suggests a limitation of the platform. But a workaround may be possible if customers require it. - """ + """Databricks dialect doesn't implement multiple-table criteria within DELETE""" pass diff --git a/src/databricks/sqlalchemy/test/_regression.py b/src/databricks/sqlalchemy/test/_regression.py index b56929de0..d27ad35e7 100644 --- a/src/databricks/sqlalchemy/test/_regression.py +++ b/src/databricks/sqlalchemy/test/_regression.py @@ -43,7 +43,8 @@ ComponentReflectionTestExtra, HasTableTest, NumericTest, - ExpandingBoundInTest + ExpandingBoundInTest, + CTETest ) @pytest.mark.reviewed @@ -268,4 +269,8 @@ class TableNoColumnsTest(TableNoColumnsTest): @pytest.mark.reviewed class ExpandingBoundInTest(ExpandingBoundInTest): + pass + +@pytest.mark.reviewed +class CTETest(CTETest): pass \ No newline at end of file diff --git a/src/databricks/sqlalchemy/test/_unsupported.py b/src/databricks/sqlalchemy/test/_unsupported.py index 1dff0a491..fa1b87657 100644 --- a/src/databricks/sqlalchemy/test/_unsupported.py +++ b/src/databricks/sqlalchemy/test/_unsupported.py @@ -19,7 +19,7 @@ LongNameBlowoutTest, ExceptionTest, QuotedNameArgumentTest, - LastrowidTest + LastrowidTest, ) from databricks.sqlalchemy.test._regression import ( @@ -31,7 +31,8 @@ NumericTest, HasTableTest, ComponentReflectionTestExtra, - InsertBehaviorTest + InsertBehaviorTest, + CTETest, ) from enum import Enum @@ -55,6 +56,8 @@ class SkipReason(Enum): IMPL_FLOAT_PREC = "required implicit float precision" STRING_FEAT = "required STRING type features" AUTO_INC = "implicit AUTO_INCREMENT" + CTE_FEAT = "required CTE features" + def render_skip_reason(rsn: SkipReason, setup_error=False, extra=False) -> str: prefix = "[BADSETUP]" if setup_error else "" @@ -72,8 +75,6 @@ class QuotedNameArgumentTest(QuotedNameArgumentTest): """ - - @pytest.mark.reviewed @pytest.mark.skip(reason=render_skip_reason(SkipReason.ENFORCE_KEYS)) class ExceptionTest(ExceptionTest): @@ -90,6 +91,7 @@ class ExceptionTest(ExceptionTest): @pytest.mark.skip(reason=render_skip_reason(SkipReason.IDENTIFIER_LENGTH)) class LongNameBlowoutTest(LongNameBlowoutTest): """These tests all include assertions that the tested name > 255 characters""" + pass @@ -245,7 +247,6 @@ def test_uuid_returning(self): @pytest.mark.reviewed class FutureTableDDLTest(FutureTableDDLTest): - @pytest.mark.skip(render_skip_reason(SkipReason.INDEXES)) def test_create_index_if_not_exists(self): """We could use requirements.index_reflection and requirements.index_ddl_if_exists @@ -347,6 +348,7 @@ def test_float_custom_scale(self): """This test automatically runs if requirements.precision_generic_float_type is open()""" pass + @pytest.mark.reviewed class HasTableTest(HasTableTest): """Databricks does not support temporary tables.""" @@ -355,7 +357,7 @@ class HasTableTest(HasTableTest): def test_has_table_temp_table(self): pass - @pytest.mark.skip(render_skip_reason(SkipReason.TEMP_TBL,True,True)) + @pytest.mark.skip(render_skip_reason(SkipReason.TEMP_TBL, True, True)) def test_has_table_temp_view(self): """Databricks supports temporary views but this test depends on requirements.has_temp_table, which we explicitly close so that we can run other tests in this group. See the comment under has_temp_table in @@ -371,9 +373,9 @@ def test_has_table_temp_view(self): """ pass + @pytest.mark.reviewed class ComponentReflectionTestExtra(ComponentReflectionTestExtra): - @pytest.mark.skip(render_skip_reason(SkipReason.INDEXES)) def test_reflect_covering_index(self): pass @@ -384,13 +386,12 @@ def test_reflect_expression_based_indexes(self): @pytest.mark.skip(render_skip_reason(SkipReason.STRING_FEAT, extra=True)) def test_varchar_reflection(self): - """Databricks doesn't enforce string length limitations like STRING(255). """ + """Databricks doesn't enforce string length limitations like STRING(255).""" pass @pytest.mark.reviewed class InsertBehaviorTest(InsertBehaviorTest): - @pytest.mark.skip(render_skip_reason(SkipReason.AUTO_INC, True, True)) def test_autoclose_on_insert(self): """The setup for this test creates a column with implicit autoincrement enabled. @@ -411,6 +412,7 @@ def test_insert_from_select_autoinc_no_rows(self): def test_autoclose_on_insert_implicit_returning(self): pass + @pytest.mark.reviewed @pytest.mark.skip(render_skip_reason(SkipReason.AUTO_INC, extra=True)) class LastrowidTest(LastrowidTest): @@ -426,4 +428,24 @@ class LastrowidTest(LastrowidTest): definition by passing an Identity() to the column constructor. """ - pass \ No newline at end of file + pass + + +@pytest.mark.reviewed +class CTETest(CTETest): + """During the teardown for this test block, it tries to drop a constraint that it never named which raises + a compilation error. This could point to poor constraint reflection but our other constraint reflection + tests pass. Requires investigation. + """ + + @pytest.mark.skip(render_skip_reason(SkipReason.CTE_FEAT, extra=True)) + def test_select_recursive_round_trip(self): + pass + + @pytest.mark.skip(render_skip_reason(SkipReason.CTE_FEAT, extra=True)) + def test_delete_scalar_subq_round_trip(self): + """Error received is [UNSUPPORTED_SUBQUERY_EXPRESSION_CATEGORY.MUST_AGGREGATE_CORRELATED_SCALAR_SUBQUERY] + + This suggests a limitation of the platform. But a workaround may be possible if customers require it. + """ + pass From b90361a0d31e82368f2926b9643e84de6fbbf192 Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Fri, 20 Oct 2023 12:08:04 -0400 Subject: [PATCH 06/23] Split NormalizedNameTest into regression and future Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 7 +++---- src/databricks/sqlalchemy/test/_regression.py | 7 ++++++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index 20eb4e62f..c7a055bc0 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -16,7 +16,6 @@ IdentityReflectionTest, IdentityColumnTest, IdentityAutoincrementTest, - NormalizedNameTest, BinaryTest, ArrayTest, ) @@ -30,7 +29,7 @@ CTETest, ) -from databricks.sqlalchemy.test._regression import ExpandingBoundInTest +from databricks.sqlalchemy.test._regression import ExpandingBoundInTest, NormalizedNameTest from enum import Enum @@ -48,6 +47,7 @@ class FutureFeature(Enum): JSON = "JSON column type handling" TUPLE_LITERAL = "tuple-like IN markers completely" CTE_FEAT = "required CTE features" + TEST_DESIGN = "required test-fixture overrides" def render_future_feature(rsn: FutureFeature, extra=False) -> str: @@ -80,9 +80,8 @@ def test_empty_homogeneous_tuples_direct(self): pass -@pytest.mark.reviewed class NormalizedNameTest(NormalizedNameTest): - @pytest.mark.skip(reason="Poor test design? See test_suite.py") + @pytest.mark.skip(render_future_feature(FutureFeature.TEST_DESIGN, True)) def test_get_table_names(self): """I'm not clear how this test can ever pass given that it's assertion looks like this: diff --git a/src/databricks/sqlalchemy/test/_regression.py b/src/databricks/sqlalchemy/test/_regression.py index d27ad35e7..d403e53a0 100644 --- a/src/databricks/sqlalchemy/test/_regression.py +++ b/src/databricks/sqlalchemy/test/_regression.py @@ -44,7 +44,8 @@ HasTableTest, NumericTest, ExpandingBoundInTest, - CTETest + CTETest, + NormalizedNameTest ) @pytest.mark.reviewed @@ -273,4 +274,8 @@ class ExpandingBoundInTest(ExpandingBoundInTest): @pytest.mark.reviewed class CTETest(CTETest): + pass + +@pytest.mark.reviewed +class NormalizedNameTest(NormalizedNameTest): pass \ No newline at end of file From fa5ccba612774eeb19f6f8af74bb0eb64ac19740 Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Fri, 20 Oct 2023 12:10:21 -0400 Subject: [PATCH 07/23] Move QuotedNameArgumentTest from unsupported to future Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 10 ++++++++++ src/databricks/sqlalchemy/test/_unsupported.py | 10 +--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index c7a055bc0..b4582d56d 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -18,6 +18,7 @@ IdentityAutoincrementTest, BinaryTest, ArrayTest, + QuotedNameArgumentTest ) from databricks.sqlalchemy.test._unsupported import ( @@ -366,3 +367,12 @@ class ArrayTest(ArrayTest): This makes them unusable to SQLAlchemy without some workaround. Potentially we could inline the values of these parameters (which risks sql injection). """ + +@pytest.mark.reviewed +@pytest.mark.skip(render_future_feature(FutureFeature.TEST_DESIGN, True)) +class QuotedNameArgumentTest(QuotedNameArgumentTest): + """These tests are challenging. The whole test setup depends on a table with a name like `quote ' one` + which will never work on Databricks because table names can't contains spaces. But QuotedNamedArgumentTest + also checks the behaviour of DDL identifier preparation process. We need to override some of IdentifierPreparer + methods because these are the ultimate control for whether or not CHECK and UNIQUE constraints are emitted. + """ \ No newline at end of file diff --git a/src/databricks/sqlalchemy/test/_unsupported.py b/src/databricks/sqlalchemy/test/_unsupported.py index fa1b87657..c91cb9594 100644 --- a/src/databricks/sqlalchemy/test/_unsupported.py +++ b/src/databricks/sqlalchemy/test/_unsupported.py @@ -18,7 +18,6 @@ HasSequenceTestEmpty, LongNameBlowoutTest, ExceptionTest, - QuotedNameArgumentTest, LastrowidTest, ) @@ -65,14 +64,7 @@ def render_skip_reason(rsn: SkipReason, setup_error=False, extra=False) -> str: return f"[UNSUPPORTED]{prefix}[{rsn.name}]: Databricks does not support {rsn.value}.{postfix}" -@pytest.mark.reviewed -@pytest.mark.skip(reason=render_skip_reason(SkipReason.SYMBOL_CHARSET, True, True)) -class QuotedNameArgumentTest(QuotedNameArgumentTest): - """These tests are challenging. The whole test setup depends on a table with a name like `quote ' one` - which will never work on Databricks because table names can't contains spaces. But QuotedNamedArgumentTest - also checks the behaviour of DDL identifier preparation process. We need to override some of IdentifierPreparer - methods because these are the ultimate control for whether or not CHECK and UNIQUE constraints are emitted. - """ + @pytest.mark.reviewed From 18c65dd32d7a3a1fd3a1456ce1a26745c6e3f45c Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Fri, 20 Oct 2023 12:12:33 -0400 Subject: [PATCH 08/23] Update skip reason for IdentityColumnTest Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index b4582d56d..a7a67797d 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -104,11 +104,11 @@ def test_delete_from_round_trip(self): @pytest.mark.reviewed -@pytest.mark.skip( - reason="Identity works. Test needs rewrite for Databricks. See comments in test_suite.py" -) +@pytest.mark.skip(render_future_feature(FutureFeature.TEST_DESIGN, True)) class IdentityColumnTest(IdentityColumnTest): - """The setup for these tests tries to create a table with a DELTA IDENTITY column but has two problems: + """Identity works. Test needs rewrite for Databricks. See comments in test_suite.py + + The setup for these tests tries to create a table with a DELTA IDENTITY column but has two problems: 1. It uses an Integer() type for the column. Whereas DELTA IDENTITY columns must be BIGINT. 2. It tries to set the start == 42, which Databricks doesn't support From 043675b1c41e545397de7357c8fa3c36e14f4a5b Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Fri, 20 Oct 2023 12:13:40 -0400 Subject: [PATCH 09/23] Update skip reason for IdentityAutoincrementTest Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index a7a67797d..b4a32b25c 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -123,9 +123,7 @@ class IdentityColumnTest(IdentityColumnTest): @pytest.mark.reviewed class IdentityAutoincrementTest(IdentityAutoincrementTest): - @pytest.mark.skip( - reason="Identity works. Test needs rewrite for Databricks. See comments in test_suite.py" - ) + @pytest.mark.skip(render_future_feature(FutureFeature.TEST_DESIGN, True)) def test_autoincrement_with_identity(self): """This test has the same issue as IdentityColumnTest.test_select_all in that it creates a table with identity using an Integer() rather than a BigInteger(). If I override this behaviour to use a BigInteger() instead, the From 3508000172db46b3e6492c0e204157ca282f5b01 Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Fri, 20 Oct 2023 16:24:35 -0400 Subject: [PATCH 10/23] Update future feature reasons for more test classes Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 31 +++++++++---------- src/databricks/sqlalchemy/test/_regression.py | 7 ++++- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index b4a32b25c..75616e2a3 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -15,10 +15,9 @@ DifficultParametersTest, IdentityReflectionTest, IdentityColumnTest, - IdentityAutoincrementTest, BinaryTest, ArrayTest, - QuotedNameArgumentTest + QuotedNameArgumentTest, ) from databricks.sqlalchemy.test._unsupported import ( @@ -30,7 +29,11 @@ CTETest, ) -from databricks.sqlalchemy.test._regression import ExpandingBoundInTest, NormalizedNameTest +from databricks.sqlalchemy.test._regression import ( + ExpandingBoundInTest, + NormalizedNameTest, + IdentityAutoincrementTest, +) from enum import Enum @@ -49,6 +52,7 @@ class FutureFeature(Enum): TUPLE_LITERAL = "tuple-like IN markers completely" CTE_FEAT = "required CTE features" TEST_DESIGN = "required test-fixture overrides" + IDENTITY = "identity reflection" def render_future_feature(rsn: FutureFeature, extra=False) -> str: @@ -107,7 +111,7 @@ def test_delete_from_round_trip(self): @pytest.mark.skip(render_future_feature(FutureFeature.TEST_DESIGN, True)) class IdentityColumnTest(IdentityColumnTest): """Identity works. Test needs rewrite for Databricks. See comments in test_suite.py - + The setup for these tests tries to create a table with a DELTA IDENTITY column but has two problems: 1. It uses an Integer() type for the column. Whereas DELTA IDENTITY columns must be BIGINT. 2. It tries to set the start == 42, which Databricks doesn't support @@ -132,7 +136,7 @@ def test_autoincrement_with_identity(self): @pytest.mark.reviewed -@pytest.mark.skip(reason="Implementation deferred. See test_suite.py") +@pytest.mark.skip(render_future_feature(FutureFeature.TEST_DESIGN)) class BizarroCharacterFKResolutionTest(BizarroCharacterFKResolutionTest): """Some of the combinations in this test pass. Others fail. Given the esoteric nature of these failures, we have opted to defer implementing fixes to a later time, guided by customer feedback. Passage of @@ -141,7 +145,7 @@ class BizarroCharacterFKResolutionTest(BizarroCharacterFKResolutionTest): @pytest.mark.reviewed -@pytest.mark.skip(reason="Implementation deferred. See test_suite.py") +@pytest.mark.skip(render_future_feature(FutureFeature.TEST_DESIGN)) class DifficultParametersTest(DifficultParametersTest): """Some of the combinations in this test pass. Others fail. Given the esoteric nature of these failures, we have opted to defer implementing fixes to a later time, guided by customer feedback. Passage of @@ -150,9 +154,7 @@ class DifficultParametersTest(DifficultParametersTest): @pytest.mark.reviewed -@pytest.mark.skip( - reason="Identity reflection is not implemented in this dialect. See test_suite.py" -) +@pytest.mark.skip(render_future_feature(FutureFeature.IDENTITY, True)) class IdentityReflectionTest(IdentityReflectionTest): """It's not clear _how_ to implement this for SQLAlchemy. Columns created with GENERATED ALWAYS AS IDENTITY are not specially demarked in the output of TGetColumnsResponse or DESCRIBE TABLE EXTENDED. @@ -162,9 +164,7 @@ class IdentityReflectionTest(IdentityReflectionTest): @pytest.mark.reviewed -@pytest.mark.skip( - reason="Databricks dialect doesn't implement JSON column types. See test_suite.py" -) +@pytest.mark.skip(render_future_feature(FutureFeature.JSON)) class JSONTest(JSONTest): """Databricks supports JSON path expressions in queries it's just not implemented in this dialect.""" @@ -172,9 +172,7 @@ class JSONTest(JSONTest): @pytest.mark.reviewed -@pytest.mark.skip( - reason="Databricks dialect doesn't implement JSON column types. See test_suite.py" -) +@pytest.mark.skip(render_future_feature(FutureFeature.JSON)) class JSONLegacyStringCastIndexTest(JSONLegacyStringCastIndexTest): """Same comment applies as JSONTest""" @@ -366,6 +364,7 @@ class ArrayTest(ArrayTest): the values of these parameters (which risks sql injection). """ + @pytest.mark.reviewed @pytest.mark.skip(render_future_feature(FutureFeature.TEST_DESIGN, True)) class QuotedNameArgumentTest(QuotedNameArgumentTest): @@ -373,4 +372,4 @@ class QuotedNameArgumentTest(QuotedNameArgumentTest): which will never work on Databricks because table names can't contains spaces. But QuotedNamedArgumentTest also checks the behaviour of DDL identifier preparation process. We need to override some of IdentifierPreparer methods because these are the ultimate control for whether or not CHECK and UNIQUE constraints are emitted. - """ \ No newline at end of file + """ diff --git a/src/databricks/sqlalchemy/test/_regression.py b/src/databricks/sqlalchemy/test/_regression.py index d403e53a0..d04129c1f 100644 --- a/src/databricks/sqlalchemy/test/_regression.py +++ b/src/databricks/sqlalchemy/test/_regression.py @@ -45,7 +45,8 @@ NumericTest, ExpandingBoundInTest, CTETest, - NormalizedNameTest + NormalizedNameTest, + IdentityAutoincrementTest ) @pytest.mark.reviewed @@ -278,4 +279,8 @@ class CTETest(CTETest): @pytest.mark.reviewed class NormalizedNameTest(NormalizedNameTest): + pass + +@pytest.mark.reviewed +class IdentityAutoincrementTest(IdentityAutoincrementTest): pass \ No newline at end of file From 0a5c6ff6307b750584631df3d92c3f9d127da8ef Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Fri, 20 Oct 2023 16:27:15 -0400 Subject: [PATCH 11/23] Split LikeFunctionsTest between regression and future with reason Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 12 ++++-------- src/databricks/sqlalchemy/test/_regression.py | 7 ++++++- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index 75616e2a3..3163d5c8d 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -8,7 +8,6 @@ CollateTest, TimeTZTest, DateTimeTZTest, - LikeFunctionsTest, JSONTest, JSONLegacyStringCastIndexTest, BizarroCharacterFKResolutionTest, @@ -33,6 +32,7 @@ ExpandingBoundInTest, NormalizedNameTest, IdentityAutoincrementTest, + LikeFunctionsTest ) from enum import Enum @@ -53,6 +53,7 @@ class FutureFeature(Enum): CTE_FEAT = "required CTE features" TEST_DESIGN = "required test-fixture overrides" IDENTITY = "identity reflection" + REGEXP = "_visit_regexp" def render_future_feature(rsn: FutureFeature, extra=False) -> str: @@ -179,18 +180,13 @@ class JSONLegacyStringCastIndexTest(JSONLegacyStringCastIndexTest): pass -@pytest.mark.reviewed class LikeFunctionsTest(LikeFunctionsTest): - @pytest.mark.skip( - reason="Databricks dialect doesn't implement regexp features. See test_suite.py" - ) + @pytest.mark.skip(render_future_feature(FutureFeature.REGEXP)) def test_not_regexp_match(self): """The defaul dialect doesn't implement _visit_regexp methods so we don't get them automatically.""" pass - @pytest.mark.skip( - reason="Databricks dialect doesn't implement regexp features. See test_suite.py" - ) + @pytest.mark.skip(render_future_feature(FutureFeature.REGEXP)) def test_regexp_match(self): """The defaul dialect doesn't implement _visit_regexp methods so we don't get them automatically.""" pass diff --git a/src/databricks/sqlalchemy/test/_regression.py b/src/databricks/sqlalchemy/test/_regression.py index d04129c1f..26399e015 100644 --- a/src/databricks/sqlalchemy/test/_regression.py +++ b/src/databricks/sqlalchemy/test/_regression.py @@ -46,7 +46,8 @@ ExpandingBoundInTest, CTETest, NormalizedNameTest, - IdentityAutoincrementTest + IdentityAutoincrementTest, + LikeFunctionsTest ) @pytest.mark.reviewed @@ -283,4 +284,8 @@ class NormalizedNameTest(NormalizedNameTest): @pytest.mark.reviewed class IdentityAutoincrementTest(IdentityAutoincrementTest): + pass + +@pytest.mark.reviewed +class LikeFunctionsTest(LikeFunctionsTest): pass \ No newline at end of file From 56448e0f4375d59a346e44b6670d292727c4ee0b Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Fri, 20 Oct 2023 16:28:57 -0400 Subject: [PATCH 12/23] Define FutureFeature for timezone tests Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index 3163d5c8d..3972d9dfc 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -54,6 +54,7 @@ class FutureFeature(Enum): TEST_DESIGN = "required test-fixture overrides" IDENTITY = "identity reflection" REGEXP = "_visit_regexp" + TIMEZONE = "timezone handling for DateTime() or Time() types" def render_future_feature(rsn: FutureFeature, extra=False) -> str: @@ -193,9 +194,7 @@ def test_regexp_match(self): @pytest.mark.reviewed -@pytest.mark.skip( - reason="Datetime handling doesn't handle timezones well. Priority to fix." -) +@pytest.mark.skip(render_future_feature(FutureFeature.TIMEZONE, True)) class DateTimeTZTest(DateTimeTZTest): """When I initially implemented DateTime type handling, I started using TIMESTAMP_NTZ because that's the default behaviour of the DateTime() type and the other tests passed. I simply missed @@ -207,9 +206,7 @@ class DateTimeTZTest(DateTimeTZTest): @pytest.mark.reviewed -@pytest.mark.skip( - reason="Databricks dialect does not implement timezone support for Timestamp() types. See test_suite.py" -) +@pytest.mark.skip(render_future_feature(FutureFeature.TIMEZONE, True)) class TimeTZTest(TimeTZTest): """Similar to DateTimeTZTest, this should be possible for the dialect since we can override type compilation and processing in _types.py. Implementation has been deferred. From 905cd574ba18a770f20cf65e94e154fac866f1fd Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Fri, 20 Oct 2023 16:30:54 -0400 Subject: [PATCH 13/23] Add COLLATE and UUID FutureFeatures Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index 3972d9dfc..42f38ea98 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -49,12 +49,14 @@ class FutureFeature(Enum): ARRAY = "ARRAY column type handling" BINARY = "BINARY column type handling" JSON = "JSON column type handling" + UUID = "native Uuid() type" TUPLE_LITERAL = "tuple-like IN markers completely" CTE_FEAT = "required CTE features" TEST_DESIGN = "required test-fixture overrides" IDENTITY = "identity reflection" REGEXP = "_visit_regexp" TIMEZONE = "timezone handling for DateTime() or Time() types" + COLLATE = "COLLATE DDL generation" def render_future_feature(rsn: FutureFeature, extra=False) -> str: @@ -214,15 +216,13 @@ class TimeTZTest(TimeTZTest): @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks dialect does not implement COLLATE support") +@pytest.mark.skip(render_future_feature(FutureFeature.COLLATE)) class CollateTest(CollateTest): """This is supported in Databricks. Not implemented here.""" @pytest.mark.reviewed -@pytest.mark.skip( - reason="Databricks dialect doesn't implement UUID type. See test_suite.py" -) +@pytest.mark.skip(render_future_feature(FutureFeature.UUID, True)) class NativeUUIDTest(NativeUUIDTest): """Type implementation will be straightforward. Since Databricks doesn't have a native UUID type we can use a STRING field, create a custom TypeDecorator for sqlalchemy.types.Uuid and add it to the dialect's colspecs. From 18efe205fbec3cc2917be93283473ff522be7d2e Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Fri, 20 Oct 2023 16:35:03 -0400 Subject: [PATCH 14/23] Add PROVISION future feature Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index 42f38ea98..48f9bc677 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -57,6 +57,8 @@ class FutureFeature(Enum): REGEXP = "_visit_regexp" TIMEZONE = "timezone handling for DateTime() or Time() types" COLLATE = "COLLATE DDL generation" + SANE_ROWCOUNT = "sane_rowcount support" + PROVISION = "event-driven engine configuration" def render_future_feature(rsn: FutureFeature, extra=False) -> str: @@ -232,19 +234,19 @@ class NativeUUIDTest(NativeUUIDTest): @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks dialect does not implement sane rowcount.") +@pytest.mark.skip(render_future_feature(FutureFeature.SANE_ROWCOUNT)) class RowCountTest(RowCountTest): pass @pytest.mark.reviewed -@pytest.mark.skip(reason="Databricks dialect does not implement sane rowcount.") +@pytest.mark.skip(render_future_feature(FutureFeature.SANE_ROWCOUNT)) class SimpleUpdateDeleteTest(SimpleUpdateDeleteTest): pass @pytest.mark.reviewed -@pytest.mark.skip(reason="Dialect doesn't implement provision.py See test_suite.py") +@pytest.mark.skip(render_future_feature(FutureFeature.PROVISION, True)) class WeCanSetDefaultSchemaWEventsTest(WeCanSetDefaultSchemaWEventsTest): """provision.py allows us to define event listeners that emit DDL for things like setting up a test schema or, in this case, changing the default schema for the connection after it's been built. This would override @@ -256,7 +258,7 @@ class WeCanSetDefaultSchemaWEventsTest(WeCanSetDefaultSchemaWEventsTest): @pytest.mark.reviewed -@pytest.mark.skip(reason="Dialect doesn't implement provision.py See test_suite.py") +@pytest.mark.skip(render_future_feature(FutureFeature.PROVISION, True)) class FutureWeCanSetDefaultSchemaWEventsTest(FutureWeCanSetDefaultSchemaWEventsTest): """provision.py allows us to define event listeners that emit DDL for things like setting up a test schema or, in this case, changing the default schema for the connection after it's been built. This would override From 8a4f8f9c297e4564fd9a1160550e687823710443 Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Fri, 20 Oct 2023 16:36:03 -0400 Subject: [PATCH 15/23] Sort SkipReason and FutureFeature keys alphabetically Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 28 +++++++++---------- .../sqlalchemy/test/_unsupported.py | 24 ++++++++-------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index 48f9bc677..0a291d86b 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -39,26 +39,26 @@ class FutureFeature(Enum): - TBL_COMMENTS = "table comment reflection" - VIEW_DEF = "get_view_definition method" - TBL_OPTS = "get_table_options method" - MULTI_PK = "get_multi_pk_constraint method" - CHECK = "CHECK constraint handling" - FK_OPTS = "foreign key option checking" - EMPTY_INSERT = "empty INSERT support" ARRAY = "ARRAY column type handling" BINARY = "BINARY column type handling" - JSON = "JSON column type handling" - UUID = "native Uuid() type" - TUPLE_LITERAL = "tuple-like IN markers completely" + CHECK = "CHECK constraint handling" + COLLATE = "COLLATE DDL generation" CTE_FEAT = "required CTE features" - TEST_DESIGN = "required test-fixture overrides" + EMPTY_INSERT = "empty INSERT support" + FK_OPTS = "foreign key option checking" IDENTITY = "identity reflection" + JSON = "JSON column type handling" + MULTI_PK = "get_multi_pk_constraint method" + PROVISION = "event-driven engine configuration" REGEXP = "_visit_regexp" - TIMEZONE = "timezone handling for DateTime() or Time() types" - COLLATE = "COLLATE DDL generation" SANE_ROWCOUNT = "sane_rowcount support" - PROVISION = "event-driven engine configuration" + TBL_COMMENTS = "table comment reflection" + TBL_OPTS = "get_table_options method" + TEST_DESIGN = "required test-fixture overrides" + TIMEZONE = "timezone handling for DateTime() or Time() types" + TUPLE_LITERAL = "tuple-like IN markers completely" + UUID = "native Uuid() type" + VIEW_DEF = "get_view_definition method" def render_future_feature(rsn: FutureFeature, extra=False) -> str: diff --git a/src/databricks/sqlalchemy/test/_unsupported.py b/src/databricks/sqlalchemy/test/_unsupported.py index c91cb9594..ced15d789 100644 --- a/src/databricks/sqlalchemy/test/_unsupported.py +++ b/src/databricks/sqlalchemy/test/_unsupported.py @@ -38,24 +38,24 @@ class SkipReason(Enum): + AUTO_INC = "implicit AUTO_INCREMENT" + CTE_FEAT = "required CTE features" + CURSORS = "server-side cursors" + DECIMAL_FEAT = "required decimal features" ENFORCE_KEYS = "enforcing primary or foreign key restraints" + FETCH = "fetch clauses" + GENERATED_COLUMNS = "computed / generated columns" IDENTIFIER_LENGTH = "identifiers > 255 characters" - SEQUENCES = "SQL SEQUENCES" + IMPL_FLOAT_PREC = "required implicit float precision" + IMPLICIT_ORDER = "deterministic return order if ORDER BY is not present" INDEXES = "SQL INDEXes" + RETURNING = "INSERT ... RETURNING syntax" + SEQUENCES = "SQL SEQUENCES" + STRING_FEAT = "required STRING type features" SYMBOL_CHARSET = "symbols expected by test" - CURSORS = "server-side cursors" + TEMP_TBL = "temporary tables" TRANSACTIONS = "transactions" - RETURNING = "INSERT ... RETURNING syntax" - GENERATED_COLUMNS = "computed / generated columns" - FETCH = "fetch clauses" - IMPLICIT_ORDER = "deterministic return order if ORDER BY is not present" UNIQUE = "UNIQUE constraints" - TEMP_TBL = "temporary tables" - DECIMAL_FEAT = "required decimal features" - IMPL_FLOAT_PREC = "required implicit float precision" - STRING_FEAT = "required STRING type features" - AUTO_INC = "implicit AUTO_INCREMENT" - CTE_FEAT = "required CTE features" def render_skip_reason(rsn: SkipReason, setup_error=False, extra=False) -> str: From 925d0d3dacb211243e68d1f87cfb06a94b9473dd Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Sat, 21 Oct 2023 15:17:46 -0400 Subject: [PATCH 16/23] Remove duplicate reviewed markers not needed because these are already included in _regression.py Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index 0a291d86b..c01977414 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -32,7 +32,7 @@ ExpandingBoundInTest, NormalizedNameTest, IdentityAutoincrementTest, - LikeFunctionsTest + LikeFunctionsTest, ) from enum import Enum @@ -72,7 +72,7 @@ class BinaryTest(BinaryTest): pass -@pytest.mark.reviewed + class ExpandingBoundInTest(ExpandingBoundInTest): @pytest.mark.skip(render_future_feature(FutureFeature.TUPLE_LITERAL)) def test_empty_heterogeneous_tuples_bindparam(self): @@ -131,7 +131,6 @@ class IdentityColumnTest(IdentityColumnTest): pass -@pytest.mark.reviewed class IdentityAutoincrementTest(IdentityAutoincrementTest): @pytest.mark.skip(render_future_feature(FutureFeature.TEST_DESIGN, True)) def test_autoincrement_with_identity(self): @@ -269,7 +268,6 @@ class FutureWeCanSetDefaultSchemaWEventsTest(FutureWeCanSetDefaultSchemaWEventsT pass -@pytest.mark.reviewed class FutureTableDDLTest(FutureTableDDLTest): @pytest.mark.skip(reason=render_future_feature(FutureFeature.TBL_COMMENTS)) def test_add_table_comment(self): @@ -282,7 +280,6 @@ def test_drop_table_comment(self): pass -@pytest.mark.reviewed class TableDDLTest(TableDDLTest): @pytest.mark.skip(reason=render_future_feature(FutureFeature.TBL_COMMENTS)) def test_add_table_comment(self, connection): @@ -295,7 +292,6 @@ def test_drop_table_comment(self, connection): pass -@pytest.mark.reviewed class ComponentReflectionTest(ComponentReflectionTest): @pytest.mark.skip(reason=render_future_feature(FutureFeature.TBL_COMMENTS)) def test_get_multi_table_comment(self): @@ -320,7 +316,6 @@ def test_get_multi_pk_constraint(self): pass -@pytest.mark.reviewed class ComponentReflectionTestExtra(ComponentReflectionTestExtra): @pytest.mark.skip(render_future_feature(FutureFeature.CHECK)) def test_get_check_constraints(self): @@ -332,7 +327,6 @@ def test_get_foreign_key_options(self): pass -@pytest.mark.reviewed class InsertBehaviorTest(InsertBehaviorTest): @pytest.mark.skip(render_future_feature(FutureFeature.EMPTY_INSERT, True)) def test_empty_insert(self): From 4d2f44d4ff1b0a081a2ededf02a9aa59bfcceec4 Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Sat, 21 Oct 2023 15:29:44 -0400 Subject: [PATCH 17/23] Remove duplicate reviewed markers Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_regression.py | 2 ++ .../sqlalchemy/test/_unsupported.py | 20 ++++++++----------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_regression.py b/src/databricks/sqlalchemy/test/_regression.py index 26399e015..bb1b18e50 100644 --- a/src/databricks/sqlalchemy/test/_regression.py +++ b/src/databricks/sqlalchemy/test/_regression.py @@ -78,6 +78,8 @@ class ComponentReflectionTest(ComponentReflectionTest): - A second schema named "test_schema" Note that test_get_multi_foreign keys is flaky because DBR does not guarantee the order of data returned in DESCRIBE TABLE EXTENDED + + _Most_ of these tests pass if we manually override the bad test setup. """ pass diff --git a/src/databricks/sqlalchemy/test/_unsupported.py b/src/databricks/sqlalchemy/test/_unsupported.py index ced15d789..359b73863 100644 --- a/src/databricks/sqlalchemy/test/_unsupported.py +++ b/src/databricks/sqlalchemy/test/_unsupported.py @@ -1,5 +1,4 @@ import pytest -from typing import Union # These are test suites that are fully skipped with a SkipReason from sqlalchemy.testing.suite import ( @@ -64,9 +63,6 @@ def render_skip_reason(rsn: SkipReason, setup_error=False, extra=False) -> str: return f"[UNSUPPORTED]{prefix}[{rsn.name}]: Databricks does not support {rsn.value}.{postfix}" - - - @pytest.mark.reviewed @pytest.mark.skip(reason=render_skip_reason(SkipReason.ENFORCE_KEYS)) class ExceptionTest(ExceptionTest): @@ -237,7 +233,7 @@ def test_uuid_returning(self): pass -@pytest.mark.reviewed + class FutureTableDDLTest(FutureTableDDLTest): @pytest.mark.skip(render_skip_reason(SkipReason.INDEXES)) def test_create_index_if_not_exists(self): @@ -254,7 +250,7 @@ def test_drop_index_if_exists(self): pass -@pytest.mark.reviewed + class TableDDLTest(TableDDLTest): @pytest.mark.skip(reason=render_skip_reason(SkipReason.INDEXES)) def test_create_index_if_not_exists(self, connection): @@ -271,7 +267,7 @@ def test_drop_index_if_exists(self, connection): pass -@pytest.mark.reviewed + class ComponentReflectionTest(ComponentReflectionTest): """This test requires two schemas be present in the target Databricks workspace: - The schema set in --dburi @@ -312,7 +308,7 @@ def test_reflect_table_temp_table(self): pass -@pytest.mark.reviewed + class NumericTest(NumericTest): @pytest.mark.skip(render_skip_reason(SkipReason.DECIMAL_FEAT)) def test_enotation_decimal(self): @@ -341,7 +337,7 @@ def test_float_custom_scale(self): pass -@pytest.mark.reviewed + class HasTableTest(HasTableTest): """Databricks does not support temporary tables.""" @@ -366,7 +362,7 @@ def test_has_table_temp_view(self): pass -@pytest.mark.reviewed + class ComponentReflectionTestExtra(ComponentReflectionTestExtra): @pytest.mark.skip(render_skip_reason(SkipReason.INDEXES)) def test_reflect_covering_index(self): @@ -382,7 +378,7 @@ def test_varchar_reflection(self): pass -@pytest.mark.reviewed + class InsertBehaviorTest(InsertBehaviorTest): @pytest.mark.skip(render_skip_reason(SkipReason.AUTO_INC, True, True)) def test_autoclose_on_insert(self): @@ -423,7 +419,7 @@ class LastrowidTest(LastrowidTest): pass -@pytest.mark.reviewed + class CTETest(CTETest): """During the teardown for this test block, it tries to drop a constraint that it never named which raises a compilation error. This could point to poor constraint reflection but our other constraint reflection From c57b6228045449a3ead8dbe264055b1ace53205a Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Sat, 21 Oct 2023 15:31:33 -0400 Subject: [PATCH 18/23] isort files Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 59 +++++++-------- src/databricks/sqlalchemy/test/_regression.py | 75 ++++++++++--------- .../sqlalchemy/test/_unsupported.py | 57 ++++++-------- 3 files changed, 93 insertions(+), 98 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index c01977414..c92f70b11 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -1,42 +1,40 @@ +from enum import Enum + import pytest -from sqlalchemy.testing.suite import ( - WeCanSetDefaultSchemaWEventsTest, - FutureWeCanSetDefaultSchemaWEventsTest, - SimpleUpdateDeleteTest, - RowCountTest, - NativeUUIDTest, - CollateTest, - TimeTZTest, - DateTimeTZTest, - JSONTest, - JSONLegacyStringCastIndexTest, - BizarroCharacterFKResolutionTest, - DifficultParametersTest, - IdentityReflectionTest, - IdentityColumnTest, - BinaryTest, - ArrayTest, - QuotedNameArgumentTest, +from databricks.sqlalchemy.test._regression import ( + ExpandingBoundInTest, + IdentityAutoincrementTest, + LikeFunctionsTest, + NormalizedNameTest, ) - from databricks.sqlalchemy.test._unsupported import ( - FutureTableDDLTest, - TableDDLTest, ComponentReflectionTest, ComponentReflectionTestExtra, - InsertBehaviorTest, CTETest, + FutureTableDDLTest, + InsertBehaviorTest, + TableDDLTest, ) - -from databricks.sqlalchemy.test._regression import ( - ExpandingBoundInTest, - NormalizedNameTest, - IdentityAutoincrementTest, - LikeFunctionsTest, +from sqlalchemy.testing.suite import ( + ArrayTest, + BinaryTest, + BizarroCharacterFKResolutionTest, + CollateTest, + DateTimeTZTest, + DifficultParametersTest, + FutureWeCanSetDefaultSchemaWEventsTest, + IdentityColumnTest, + IdentityReflectionTest, + JSONLegacyStringCastIndexTest, + JSONTest, + NativeUUIDTest, + QuotedNameArgumentTest, + RowCountTest, + SimpleUpdateDeleteTest, + TimeTZTest, + WeCanSetDefaultSchemaWEventsTest, ) -from enum import Enum - class FutureFeature(Enum): ARRAY = "ARRAY column type handling" @@ -72,7 +70,6 @@ class BinaryTest(BinaryTest): pass - class ExpandingBoundInTest(ExpandingBoundInTest): @pytest.mark.skip(render_future_feature(FutureFeature.TUPLE_LITERAL)) def test_empty_heterogeneous_tuples_bindparam(self): diff --git a/src/databricks/sqlalchemy/test/_regression.py b/src/databricks/sqlalchemy/test/_regression.py index bb1b18e50..aeeb5c3fc 100644 --- a/src/databricks/sqlalchemy/test/_regression.py +++ b/src/databricks/sqlalchemy/test/_regression.py @@ -1,61 +1,61 @@ import pytest from sqlalchemy.testing.suite import ( - TimeMicrosecondsTest, - TextTest, - StringTest, - DateTimeMicrosecondsTest, - TimestampMicrosecondsTest, - DateTimeCoercedToDateTimeTest, - TimeTest, - DateTimeTest, - DateTimeHistoricTest, - DateTest, - DateHistoricTest, - RowFetchTest, - CompositeKeyReflectionTest, - TrueDivTest, ArgSignatureTest, + BooleanTest, + CastTypeDecoratorTest, + ComponentReflectionTest, + ComponentReflectionTestExtra, + CompositeKeyReflectionTest, CompoundSelectTest, + CTETest, + DateHistoricTest, + DateTest, + DateTimeCoercedToDateTimeTest, + DateTimeHistoricTest, + DateTimeMicrosecondsTest, + DateTimeTest, DeprecatedCompoundSelectTest, - CastTypeDecoratorTest, DistinctOnTest, EscapingTest, ExistsTest, + ExpandingBoundInTest, + FetchLimitOffsetTest, + FutureTableDDLTest, + HasTableTest, + IdentityAutoincrementTest, + InsertBehaviorTest, IntegerTest, IsOrIsNotDistinctFromTest, JoinTest, + LikeFunctionsTest, + NormalizedNameTest, + NumericTest, OrderByLabelTest, PingTest, + PostCompileParamsTest, ReturningGuardsTest, + RowFetchTest, SameNamedSchemaTableTest, + StringTest, + TableDDLTest, + TableNoColumnsTest, + TextTest, + TimeMicrosecondsTest, + TimestampMicrosecondsTest, + TimeTest, + TrueDivTest, UnicodeTextTest, UnicodeVarcharTest, - TableNoColumnsTest, - PostCompileParamsTest, - BooleanTest, - ValuesExpressionTest, UuidTest, - FetchLimitOffsetTest, - FutureTableDDLTest, - TableDDLTest, - ComponentReflectionTest, - InsertBehaviorTest, - ComponentReflectionTestExtra, - HasTableTest, - NumericTest, - ExpandingBoundInTest, - CTETest, - NormalizedNameTest, - IdentityAutoincrementTest, - LikeFunctionsTest + ValuesExpressionTest, ) + @pytest.mark.reviewed class NumericTest(NumericTest): pass - @pytest.mark.reviewed class HasTableTest(HasTableTest): pass @@ -81,8 +81,10 @@ class ComponentReflectionTest(ComponentReflectionTest): _Most_ of these tests pass if we manually override the bad test setup. """ + pass + @pytest.mark.reviewed class TableDDLTest(TableDDLTest): pass @@ -267,6 +269,7 @@ class UnicodeTextTest(UnicodeTextTest): class UnicodeVarcharTest(UnicodeVarcharTest): pass + @pytest.mark.reviewed class TableNoColumnsTest(TableNoColumnsTest): pass @@ -276,18 +279,22 @@ class TableNoColumnsTest(TableNoColumnsTest): class ExpandingBoundInTest(ExpandingBoundInTest): pass + @pytest.mark.reviewed class CTETest(CTETest): pass + @pytest.mark.reviewed class NormalizedNameTest(NormalizedNameTest): pass + @pytest.mark.reviewed class IdentityAutoincrementTest(IdentityAutoincrementTest): pass + @pytest.mark.reviewed class LikeFunctionsTest(LikeFunctionsTest): - pass \ No newline at end of file + pass diff --git a/src/databricks/sqlalchemy/test/_unsupported.py b/src/databricks/sqlalchemy/test/_unsupported.py index 359b73863..a07ecb662 100644 --- a/src/databricks/sqlalchemy/test/_unsupported.py +++ b/src/databricks/sqlalchemy/test/_unsupported.py @@ -1,40 +1,39 @@ +from enum import Enum + import pytest +from databricks.sqlalchemy.test._regression import ( + ComponentReflectionTest, + ComponentReflectionTestExtra, + CTETest, + FetchLimitOffsetTest, + FutureTableDDLTest, + HasTableTest, + InsertBehaviorTest, + NumericTest, + TableDDLTest, + UuidTest, +) # These are test suites that are fully skipped with a SkipReason from sqlalchemy.testing.suite import ( - SequenceTest, - SequenceCompilerTest, + AutocommitIsolationTest, ComputedColumnTest, ComputedReflectionTest, - ReturningTest, - IsolationLevelTest, - AutocommitIsolationTest, - PercentSchemaNamesTest, - UnicodeSchemaTest, - ServerSideCursorsTest, + ExceptionTest, HasIndexTest, HasSequenceTest, HasSequenceTestEmpty, - LongNameBlowoutTest, - ExceptionTest, + IsolationLevelTest, LastrowidTest, + LongNameBlowoutTest, + PercentSchemaNamesTest, + ReturningTest, + SequenceCompilerTest, + SequenceTest, + ServerSideCursorsTest, + UnicodeSchemaTest, ) -from databricks.sqlalchemy.test._regression import ( - FetchLimitOffsetTest, - UuidTest, - FutureTableDDLTest, - TableDDLTest, - ComponentReflectionTest, - NumericTest, - HasTableTest, - ComponentReflectionTestExtra, - InsertBehaviorTest, - CTETest, -) - -from enum import Enum - class SkipReason(Enum): AUTO_INC = "implicit AUTO_INCREMENT" @@ -233,7 +232,6 @@ def test_uuid_returning(self): pass - class FutureTableDDLTest(FutureTableDDLTest): @pytest.mark.skip(render_skip_reason(SkipReason.INDEXES)) def test_create_index_if_not_exists(self): @@ -250,7 +248,6 @@ def test_drop_index_if_exists(self): pass - class TableDDLTest(TableDDLTest): @pytest.mark.skip(reason=render_skip_reason(SkipReason.INDEXES)) def test_create_index_if_not_exists(self, connection): @@ -267,7 +264,6 @@ def test_drop_index_if_exists(self, connection): pass - class ComponentReflectionTest(ComponentReflectionTest): """This test requires two schemas be present in the target Databricks workspace: - The schema set in --dburi @@ -308,7 +304,6 @@ def test_reflect_table_temp_table(self): pass - class NumericTest(NumericTest): @pytest.mark.skip(render_skip_reason(SkipReason.DECIMAL_FEAT)) def test_enotation_decimal(self): @@ -337,7 +332,6 @@ def test_float_custom_scale(self): pass - class HasTableTest(HasTableTest): """Databricks does not support temporary tables.""" @@ -362,7 +356,6 @@ def test_has_table_temp_view(self): pass - class ComponentReflectionTestExtra(ComponentReflectionTestExtra): @pytest.mark.skip(render_skip_reason(SkipReason.INDEXES)) def test_reflect_covering_index(self): @@ -378,7 +371,6 @@ def test_varchar_reflection(self): pass - class InsertBehaviorTest(InsertBehaviorTest): @pytest.mark.skip(render_skip_reason(SkipReason.AUTO_INC, True, True)) def test_autoclose_on_insert(self): @@ -419,7 +411,6 @@ class LastrowidTest(LastrowidTest): pass - class CTETest(CTETest): """During the teardown for this test block, it tries to drop a constraint that it never named which raises a compilation error. This could point to poor constraint reflection but our other constraint reflection From dd6b9d188ca6c209adb0c7edd5785802840e9c25 Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Sun, 22 Oct 2023 23:12:39 -0400 Subject: [PATCH 19/23] Add comment about binary handling Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index c92f70b11..07a9896b2 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -67,6 +67,9 @@ def render_future_feature(rsn: FutureFeature, extra=False) -> str: @pytest.mark.reviewed @pytest.mark.skip(render_future_feature(FutureFeature.BINARY)) class BinaryTest(BinaryTest): + """Databricks doesn't support binding of BINARY type values. When DBR supports this, we can implement + in this dialect. + """ pass From e9b50d93ed76ae9bd7fc00692a3007db68c0f94d Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Sun, 22 Oct 2023 23:12:48 -0400 Subject: [PATCH 20/23] Add marker description to pytest.ini Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/pytest.ini | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/databricks/sqlalchemy/pytest.ini b/src/databricks/sqlalchemy/pytest.ini index e69de29bb..affffd2f8 100644 --- a/src/databricks/sqlalchemy/pytest.ini +++ b/src/databricks/sqlalchemy/pytest.ini @@ -0,0 +1,3 @@ +[pytest] +markers = + reviewed: Test case has been reviewed by databricks \ No newline at end of file From ad5c342f21f6457bc39ae2b883b66d2b5414226c Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Sun, 22 Oct 2023 23:44:06 -0400 Subject: [PATCH 21/23] Document test layout in a new README Signed-off-by: Jesse Whitehouse --- CONTRIBUTING.md | 18 +-------- src/databricks/sqlalchemy/README.tests.md | 42 ++++++++++++++++++++ src/databricks/sqlalchemy/test/test_suite.py | 27 ++++--------- 3 files changed, 51 insertions(+), 36 deletions(-) create mode 100644 src/databricks/sqlalchemy/README.tests.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index eebf3353b..1d5c2e970 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -144,23 +144,9 @@ The `PySQLStagingIngestionTestSuite` namespace requires a cluster running DBR ve The suites marked `[not documented]` require additional configuration which will be documented at a later time. -#### SQLAlchemy dialog tests +#### SQLAlchemy dialect tests -SQLAlchemy provides reusable tests for testing dialect implementations. - -``` -poetry shell -cd src/databricks/sqlalchemy/test -python -m pytest test_suite.py --dburi \ - "databricks://token:$access_token@$host?http_path=$http_path&catalog=$catalog&schema=$schema" -``` - -Some of these of these tests fail currently. We're working on getting relevant tests passing and others skipped. The tests that we've already reviewed and verified -are decorated with a pytest marker called `reviewed`. To only run these tests and check for regressions, you can add `-m reviewed` to the invocation command above. - -These tests require two schemas exist in your catalog: -- An empty schema which can have an arbitrary name. It is configured in the SQLAlchemy --dburi -- An empty schema named `test_schema` +See README.tests.md for details. ### Code formatting diff --git a/src/databricks/sqlalchemy/README.tests.md b/src/databricks/sqlalchemy/README.tests.md new file mode 100644 index 000000000..52ad17b9b --- /dev/null +++ b/src/databricks/sqlalchemy/README.tests.md @@ -0,0 +1,42 @@ +## SQLAlchemy Dialect Compliance Test Suite with Databricks + +The contents of the `test/` directory follow the SQLAlchemy developers' [guidance] for running the reusable dialect compliance test suite. Since not every test in the suite is applicable to every dialect, two options are provided to skip tests: + +- Any test can be skipped by subclassing its parent class, re-declaring the test-case and adding a `pytest.mark.skip` directive. +- Any test that is decorated with a `@requires` decorator can be skipped by marking the indicated requirement as `.closed()` in `requirements.py` + +We prefer to skip test cases directly with the first method wherever possible. We only mark requirements as `closed()` if there is no easier option to avoid a test failure. This principally occurs in test cases where the same test in the suite is parametrized, and some parameter combinations are conditionally skipped depending on `requirements.py`. If we skip the entire test method, then we skip _all_ permutations, not just the combinations we don't support. + +## Regression, Unsupported, and Future test cases + +We maintain three files of test cases that we import from the SQLAlchemy source code: + +* **`_regression.py`** contains all the tests cases with tests that we expect to pass for our dialect. Each one is marked with `pytest.mark.reiewed` to indicate that we've evaluated it for relevance. This file only contains base class declarations. +* **`_unsupported.py`** contains test cases that fail because of missing features in Databricks. We mark them as skipped with a `SkipReason` enumeration. If Databricks comes to support these features, those test or entire classes can be moved to `_regression.py`. +* **`_future.py`** contains test cases that fail because of missing features in the dialect itself, but which _are_ supported by Databricks generally. We mark them as skipped with a `FutureFeature` enumeration. These are features that have not been prioritised or that do not violate our acceptance criteria. All of these test cases will eventually move to either `_regression.py`. + +In some cases, only certain tests in class should be skipped with a `SkipReason` or `FutureFeature` justification. In those cases, we import the class into `_regression.py`, then import it from there into one or both of `_future.py` and `_unsupported.py`. If a class needs to be "touched" by regression, unsupported, and future, the class will be imported in that order. If an entire class should be skipped, then we do not import it into `_regression.py` at all. + +## Running the reusable dialect tests + +``` +poetry shell +cd src/databricks/sqlalchemy/test +python -m pytest test_suite.py --dburi \ + "databricks://token:$access_token@$host?http_path=$http_path&catalog=$catalog&schema=$schema" +``` + +Whatever schema you pass in the `dburi` argument should be empty. Some tests also require the presence of an empty schema named `test_schema`. Note that we plan to implement our own `provision.py` which SQLAlchemy can automatically use to create an empty schema for testing. But for now this is a manual process. + +You can run only reviewed tests by appending `-m "reviewed"` to the test runner invocation. + +You can run only the unreviewed tests by appending `-m "not reviewed"` instead. + +Note that because these tests depend on SQLAlchemy's custom pytest plugin, they are not discoverable by IDE-based test runners like VSCode or PyCharm and must be invoked from a CLI. + +## Running local unit and e2e tests + +Apart from the SQLAlchemy reusable suite, we maintain our own unit and e2e tests under the `test_local/` directory. These can be invoked from a VSCode or Pycharm since they don't depend on a custom pytest plugin. Due to pytest's lookup order, the `pytest.ini` which is required for running the reusable dialect tests, also conflicts with VSCode and Pycharm's default pytest implementation and overrides the settings in `pyproject.toml`. So to run these tests, you can delete or rename `pytest.ini`. + + +[guidance]: "https://github.com/sqlalchemy/sqlalchemy/blob/rel_2_0_22/README.dialects.rst" \ No newline at end of file diff --git a/src/databricks/sqlalchemy/test/test_suite.py b/src/databricks/sqlalchemy/test/test_suite.py index f4b91b9d5..206390c08 100644 --- a/src/databricks/sqlalchemy/test/test_suite.py +++ b/src/databricks/sqlalchemy/test/test_suite.py @@ -1,29 +1,16 @@ +""" +The order of these imports is important. Test cases are imported first from SQLAlchemy, +then are overridden by our local skip markers in _regression, _unsupported, and _future. +""" + # type: ignore +# fmt: off from sqlalchemy.testing.suite import * from databricks.sqlalchemy.test._regression import * from databricks.sqlalchemy.test._unsupported import * from databricks.sqlalchemy.test._future import * -# Test definitions are found here: -# https://github.com/sqlalchemy/sqlalchemy/tree/main/lib/sqlalchemy/testing/suite - - -# Per the instructions for dialect authors, tests should be skippable based on -# dialect limitations defined in requirements.py. However, we found that these -# definitions are not universally honoured by the SQLAlchemy test runner so we -# opt to manually delete them from the test suite for the time-being. This makes -# it obvious what areas of dialect compliance we have not evaluated. The next -# step towards dialect compliance is to review each of these and document exactly -# which methods should or should not work. This can be done by removing the corr- -# esponding skip marker and then running the test. - -# If we find tests that are skippable for a documented reason, we can call these -# out directly in the way suggested by SQLAlchemy's document for dialect authors: -# -# > In the case that the decorators are not covering a particular test, a test -# > can also be directly modified or bypassed. -# -# See further: https://github.com/sqlalchemy/sqlalchemy/blob/rel_1_4_48/README.dialects.rst + From 6333a9a9f47922653f7655679ed655331c5494b8 Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Mon, 23 Oct 2023 12:01:10 -0400 Subject: [PATCH 22/23] Black the files Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 1 + src/databricks/sqlalchemy/test/test_suite.py | 30 -------------------- 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index 07a9896b2..948cea5e9 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -70,6 +70,7 @@ class BinaryTest(BinaryTest): """Databricks doesn't support binding of BINARY type values. When DBR supports this, we can implement in this dialect. """ + pass diff --git a/src/databricks/sqlalchemy/test/test_suite.py b/src/databricks/sqlalchemy/test/test_suite.py index 206390c08..e96c6a063 100644 --- a/src/databricks/sqlalchemy/test/test_suite.py +++ b/src/databricks/sqlalchemy/test/test_suite.py @@ -9,33 +9,3 @@ from databricks.sqlalchemy.test._regression import * from databricks.sqlalchemy.test._unsupported import * from databricks.sqlalchemy.test._future import * - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - From fc3aa374507ae7dc18b651dce666d42339c4bc0e Mon Sep 17 00:00:00 2001 From: Jesse Whitehouse Date: Mon, 23 Oct 2023 16:22:20 -0400 Subject: [PATCH 23/23] Move generated columns tests from _unsupported to _future During PR review, I learned that generated columns are in public preview https://docs.databricks.com/en/delta/generated-columns.html Therefore this is not unsupported but rather a future feature Signed-off-by: Jesse Whitehouse --- src/databricks/sqlalchemy/test/_future.py | 15 +++++++++++++++ src/databricks/sqlalchemy/test/_unsupported.py | 15 --------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/databricks/sqlalchemy/test/_future.py b/src/databricks/sqlalchemy/test/_future.py index 948cea5e9..0da38634d 100644 --- a/src/databricks/sqlalchemy/test/_future.py +++ b/src/databricks/sqlalchemy/test/_future.py @@ -20,6 +20,8 @@ BinaryTest, BizarroCharacterFKResolutionTest, CollateTest, + ComputedColumnTest, + ComputedReflectionTest, DateTimeTZTest, DifficultParametersTest, FutureWeCanSetDefaultSchemaWEventsTest, @@ -44,6 +46,7 @@ class FutureFeature(Enum): CTE_FEAT = "required CTE features" EMPTY_INSERT = "empty INSERT support" FK_OPTS = "foreign key option checking" + GENERATED_COLUMNS = "Delta computed / generated columns support" IDENTITY = "identity reflection" JSON = "JSON column type handling" MULTI_PK = "get_multi_pk_constraint method" @@ -363,3 +366,15 @@ class QuotedNameArgumentTest(QuotedNameArgumentTest): also checks the behaviour of DDL identifier preparation process. We need to override some of IdentifierPreparer methods because these are the ultimate control for whether or not CHECK and UNIQUE constraints are emitted. """ + + +@pytest.mark.reviewed +@pytest.mark.skip(reason=render_future_feature(FutureFeature.GENERATED_COLUMNS)) +class ComputedColumnTest(ComputedColumnTest): + pass + + +@pytest.mark.reviewed +@pytest.mark.skip(reason=render_future_feature(FutureFeature.GENERATED_COLUMNS)) +class ComputedReflectionTest(ComputedReflectionTest): + pass diff --git a/src/databricks/sqlalchemy/test/_unsupported.py b/src/databricks/sqlalchemy/test/_unsupported.py index a07ecb662..63932fe2e 100644 --- a/src/databricks/sqlalchemy/test/_unsupported.py +++ b/src/databricks/sqlalchemy/test/_unsupported.py @@ -17,8 +17,6 @@ # These are test suites that are fully skipped with a SkipReason from sqlalchemy.testing.suite import ( AutocommitIsolationTest, - ComputedColumnTest, - ComputedReflectionTest, ExceptionTest, HasIndexTest, HasSequenceTest, @@ -42,7 +40,6 @@ class SkipReason(Enum): DECIMAL_FEAT = "required decimal features" ENFORCE_KEYS = "enforcing primary or foreign key restraints" FETCH = "fetch clauses" - GENERATED_COLUMNS = "computed / generated columns" IDENTIFIER_LENGTH = "identifiers > 255 characters" IMPL_FLOAT_PREC = "required implicit float precision" IMPLICIT_ORDER = "deterministic return order if ORDER BY is not present" @@ -148,18 +145,6 @@ class SequenceCompilerTest(SequenceCompilerTest): pass -@pytest.mark.reviewed -@pytest.mark.skip(reason=render_skip_reason(SkipReason.GENERATED_COLUMNS)) -class ComputedColumnTest(ComputedColumnTest): - pass - - -@pytest.mark.reviewed -@pytest.mark.skip(reason=render_skip_reason(SkipReason.GENERATED_COLUMNS)) -class ComputedReflectionTest(ComputedReflectionTest): - pass - - class FetchLimitOffsetTest(FetchLimitOffsetTest): @pytest.mark.flaky @pytest.mark.skip(reason=render_skip_reason(SkipReason.IMPLICIT_ORDER, extra=True))