Better error message for NULLs in primary key columns (#11055)

2024-12-23 01:21:33 +03:00 · 2024-09-18 11:56:21 -04:00 · 2024-09-18 11:56:21 -04:00 · 859b572242
commit 859b572242
parent 8eb6aaa188
14 changed files with 174 additions and 23 deletions
--- a/distribution/lib/Standard/AWS/0.0.0-dev/src/Database/Redshift/Internal/Redshift_Dialect.enso
+++ b/distribution/lib/Standard/AWS/0.0.0-dev/src/Database/Redshift/Internal/Redshift_Dialect.enso
@ -145,7 +145,8 @@ type Redshift_Dialect
    dialect_flags : Dialect_Flags
    dialect_flags self -> Dialect_Flags =
        rounding = Rounding_Flags.Value supports_negative_decimal_places=True supports_float_decimal_places=False use_builtin_bankers=False
-        Dialect_Flags.Value rounding=rounding
+        primary_key = Primary_Key_Flags.Value allows_nulls=False
+        Dialect_Flags.Value rounding=rounding primary_key=primary_key

    ## PRIVATE
       Specifies how the database creates temp tables.
--- a/distribution/lib/Standard/AWS/0.0.0-dev/src/Database/Redshift/Internal/Redshift_Error_Mapper.enso
+++ b/distribution/lib/Standard/AWS/0.0.0-dev/src/Database/Redshift/Internal/Redshift_Error_Mapper.enso
@ -6,12 +6,17 @@ from Standard.Database.Errors import SQL_Error
 type Redshift_Error_Mapper

    ## PRIVATE
-    is_primary_key_violation : SQL_Error -> Boolean
-    is_primary_key_violation error =
+    is_duplicate_primary_key_violation : SQL_Error -> Boolean
+    is_duplicate_primary_key_violation error =
        # Currently not implemented, skipping the error recognition.
        _ = error
        False

+    ## PRIVATE
+    is_null_primary_key_violation : SQL_Error -> Boolean
+    is_null_primary_key_violation error =
+        error.java_exception.getMessage.contains "violates not-null constraint"
+
    ## PRIVATE
    transform_custom_errors : SQL_Error -> Any
    transform_custom_errors error = error
--- a/distribution/lib/Standard/Database/0.0.0-dev/src/Dialect_Flags.enso
+++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Dialect_Flags.enso
@ -11,10 +11,12 @@ from Standard.Base import all
   since feature flags are user-facing, and can be used to identify features
   that are not yet implemented.
 type Dialect_Flags
-    ## PRIVATE
-    Value (rounding : Rounding_Flags)
+    Value (rounding : Rounding_Flags) (primary_key : Primary_Key_Flags)

 ## PRIVATE
 type Rounding_Flags
    ## PRIVATE
    Value (supports_negative_decimal_places : Boolean) (supports_float_decimal_places : Boolean) (use_builtin_bankers : Boolean)
+
+type Primary_Key_Flags
+    Value (allows_nulls : Boolean)
--- a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Error_Mapper.enso
+++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Error_Mapper.enso
@ -11,8 +11,14 @@ type Error_Mapper
    ## PRIVATE
       Checks if the given error is related to a violation of PRIMARY KEY
       uniqueness constraint.
-    is_primary_key_violation : SQL_Error -> Boolean
-    is_primary_key_violation error =
+    is_duplicate_primary_key_violation : SQL_Error -> Boolean
+    is_duplicate_primary_key_violation error =
+        _ = error
+        Unimplemented.throw "This is an interface only."
+
+    ## PRIVATE
+    is_null_primary_key_violation : SQL_Error -> Boolean
+    is_null_primary_key_violation error =
        _ = error
        Unimplemented.throw "This is an interface only."

--- a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Postgres/Postgres_Dialect.enso
+++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Postgres/Postgres_Dialect.enso
@ -182,7 +182,8 @@ type Postgres_Dialect
    dialect_flags : Dialect_Flags
    dialect_flags self -> Dialect_Flags =
        rounding = Rounding_Flags.Value supports_negative_decimal_places=True supports_float_decimal_places=False use_builtin_bankers=False
-        Dialect_Flags.Value rounding=rounding
+        primary_key = Primary_Key_Flags.Value allows_nulls=False
+        Dialect_Flags.Value rounding=rounding primary_key=primary_key

    ## PRIVATE
       Specifies how the database creates temp tables.
--- a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Postgres/Postgres_Error_Mapper.enso
+++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Postgres/Postgres_Error_Mapper.enso
@ -6,10 +6,15 @@ from project.Errors import Invariant_Violation, SQL_Error
 type Postgres_Error_Mapper

    ## PRIVATE
-    is_primary_key_violation : SQL_Error -> Boolean
-    is_primary_key_violation error =
+    is_duplicate_primary_key_violation : SQL_Error -> Boolean
+    is_duplicate_primary_key_violation error =
        error.java_exception.getMessage.contains "duplicate key value violates unique constraint"

+    ## PRIVATE
+    is_null_primary_key_violation : SQL_Error -> Boolean
+    is_null_primary_key_violation error =
+        error.java_exception.getMessage.contains "violates not-null constraint"
+
    ## PRIVATE
    is_table_already_exists_error : SQL_Error -> Boolean
    is_table_already_exists_error error =
--- a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/SQLite/SQLite_Dialect.enso
+++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/SQLite/SQLite_Dialect.enso
@ -195,7 +195,8 @@ type SQLite_Dialect
    dialect_flags : Dialect_Flags
    dialect_flags self -> Dialect_Flags =
        rounding = Rounding_Flags.Value supports_negative_decimal_places=False supports_float_decimal_places=True use_builtin_bankers=False
-        Dialect_Flags.Value rounding=rounding
+        primary_key = Primary_Key_Flags.Value allows_nulls=True
+        Dialect_Flags.Value rounding=rounding primary_key=primary_key

    ## PRIVATE
       Specifies how the database creates temp tables.
--- a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/SQLite/SQLite_Error_Mapper.enso
+++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/SQLite/SQLite_Error_Mapper.enso
@ -9,12 +9,17 @@ polyglot java import org.sqlite.SQLiteException
 type SQLite_Error_Mapper

    ## PRIVATE
-    is_primary_key_violation : SQL_Error -> Boolean
-    is_primary_key_violation error =
+    is_duplicate_primary_key_violation : SQL_Error -> Boolean
+    is_duplicate_primary_key_violation error =
        case error.java_exception of
            sqlite_exception : SQLiteException ->
                sqlite_exception.getResultCode == SQLiteErrorCode.SQLITE_CONSTRAINT_PRIMARYKEY

+    ## PRIVATE
+       SQLite does not mind SQLite NULL primary keys.
+    is_null_primary_key_violation : SQL_Error -> Boolean
+    is_null_primary_key_violation _ = False
+
    ## PRIVATE
    is_table_already_exists_error : SQL_Error -> Boolean
    is_table_already_exists_error error =
--- a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Upload/Helpers/Error_Helpers.enso
+++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Upload/Helpers/Error_Helpers.enso
@ -3,7 +3,7 @@ private
 from Standard.Base import all

 from Standard.Table import Aggregate_Column
-from Standard.Table.Errors import Non_Unique_Key
+from Standard.Table.Errors import Non_Unique_Key, Null_Values_In_Key_Columns

 from Standard.Database.Errors import SQL_Error

@ -18,22 +18,39 @@ internal_translate_known_upload_errors source_table connection primary_key ~acti
    handler caught_panic =
        error_mapper = connection.dialect.get_error_mapper
        sql_error = caught_panic.payload
-        case error_mapper.is_primary_key_violation sql_error of
+        case error_mapper.is_duplicate_primary_key_violation sql_error of
            True -> Panic.throw (Non_Unique_Key_Recipe.Recipe source_table primary_key caught_panic)
-            False -> Panic.throw caught_panic
+            False -> case error_mapper.is_null_primary_key_violation sql_error of
+                True -> Panic.throw (Null_Key_Recipe.Recipe source_table primary_key caught_panic)
+                False -> Panic.throw caught_panic
    Panic.catch SQL_Error action handler

 ## PRIVATE
-handle_upload_errors ~action =
+handle_upload_duplicate_primary_key_errors ~action =
    Panic.catch Non_Unique_Key_Recipe action caught_panic->
        recipe = caught_panic.payload
        raise_duplicated_primary_key_error recipe.source_table recipe.primary_key recipe.original_panic

+## PRIVATE
+handle_upload_null_primary_key_errors ~action =
+    Panic.catch Null_Key_Recipe action caught_panic->
+        recipe = caught_panic.payload
+        raise_null_primary_key_error recipe.source_table recipe.primary_key recipe.original_panic
+
+## PRIVATE
+handle_upload_errors ~action =
+    handle_upload_duplicate_primary_key_errors (handle_upload_null_primary_key_errors action)
+
 ## PRIVATE
 type Non_Unique_Key_Recipe
    ## PRIVATE
    Recipe source_table primary_key original_panic

+## PRIVATE
+type Null_Key_Recipe
+    ## PRIVATE
+    Recipe source_table primary_key original_panic
+
 ## PRIVATE
   Creates a `Non_Unique_Key` error containing information about an
   example group violating the uniqueness constraint.
@ -52,3 +69,20 @@ raise_duplicated_primary_key_error source_table primary_key original_panic =
            example_count = row.last
            example_entry = row.drop (..Last 1)
            Error.throw (Non_Unique_Key.Error primary_key example_entry example_count)
+
+## PRIVATE
+   Creates a `Null_Key` error containing information about an
+   example group violating the non-null constraint.
+raise_null_primary_key_error source_table primary_key original_panic =
+    bad_rows_per_pk = primary_key.map primary_key->
+        filtered = source_table.filter column=primary_key Filter_Condition.Is_Nothing
+        filtered.read (..First 1)
+    tables_with_bad_rows = bad_rows_per_pk.filter (t-> t.row_count > 0)
+    case tables_with_bad_rows.length == 0 of
+        ## If we couldn't find a null key, we give up the translation and
+           rethrow the original panic containing the SQL error.
+        True -> Panic.throw original_panic
+        False ->
+            table_with_bad_rows = tables_with_bad_rows.first
+            row = table_with_bad_rows.first_row.to_vector
+            Error.throw (Null_Values_In_Key_Columns.Error row)
--- a/distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Dialect.enso
+++ b/distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Dialect.enso
@ -194,7 +194,8 @@ type SQLSever_Dialect
    dialect_flags : Dialect_Flags
    dialect_flags self -> Dialect_Flags =
        rounding = Rounding_Flags.Value supports_negative_decimal_places=True supports_float_decimal_places=True use_builtin_bankers=False
-        Dialect_Flags.Value rounding=rounding
+        primary_key = Primary_Key_Flags.Value allows_nulls=False
+        Dialect_Flags.Value rounding=rounding primary_key=primary_key

    ## PRIVATE
       Specifies how the database creates temp tables.
--- a/distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Error_Mapper.enso
+++ b/distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Error_Mapper.enso
@ -6,12 +6,17 @@ from Standard.Database.Errors import Invariant_Violation, SQL_Error
 type SQLServer_Error_Mapper

    ## PRIVATE
-    is_primary_key_violation : SQL_Error -> Boolean
-    is_primary_key_violation error =
+    is_duplicate_primary_key_violation : SQL_Error -> Boolean
+    is_duplicate_primary_key_violation error =
        ## TODO the SQL error actually contains the duplicated primary key value!
           We could use that to avoid a separate `Non_Unique_Key_Recipe` query.
        error.java_exception.getMessage.contains "Violation of PRIMARY KEY constraint"

+    ## PRIVATE
+    is_null_primary_key_violation : SQL_Error -> Boolean
+    is_null_primary_key_violation error =
+        error.java_exception.getMessage.contains "violates not-null constraint"
+
    ## PRIVATE
    is_table_already_exists_error : SQL_Error -> Boolean
    is_table_already_exists_error error =
--- a/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Dialect.enso
+++ b/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Dialect.enso
@ -207,7 +207,8 @@ type Snowflake_Dialect
    dialect_flags : Dialect_Flags
    dialect_flags self -> Dialect_Flags =
        rounding = Rounding_Flags.Value supports_negative_decimal_places=True supports_float_decimal_places=True use_builtin_bankers=True
-        Dialect_Flags.Value rounding=rounding
+        primary_key = Primary_Key_Flags.Value allows_nulls=False
+        Dialect_Flags.Value rounding=rounding primary_key=primary_key

    ## PRIVATE
       Specifies how the database creates temp tables.
--- a/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Error_Mapper.enso
+++ b/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Error_Mapper.enso
@ -6,11 +6,16 @@ from Standard.Database.Errors import Invariant_Violation, SQL_Error
 type Snowflake_Error_Mapper

    ## PRIVATE
-    is_primary_key_violation : SQL_Error -> Boolean
-    is_primary_key_violation error =
+    is_duplicate_primary_key_violation : SQL_Error -> Boolean
+    is_duplicate_primary_key_violation error =
        # TODO https://github.com/enso-org/enso/issues/7117
        error.java_exception.getMessage.contains "A primary key already exists."

+    ## PRIVATE
+    is_null_primary_key_violation : SQL_Error -> Boolean
+    is_null_primary_key_violation error =
+        error.java_exception.getMessage.contains "NULL result in a non-nullable column"
+
    ## PRIVATE
    is_table_already_exists_error : SQL_Error -> Boolean
    is_table_already_exists_error error =
--- a/test/Table_Tests/src/Database/Upload_Spec.enso
+++ b/test/Table_Tests/src/Database/Upload_Spec.enso
@ -370,6 +370,42 @@ add_specs suite_builder setup make_new_connection persistent_connector=True =
                r1 = data.in_memory_table.select_into_database_table data.connection (Name_Generator.random_name "primary-key-4") primary_key=["X", "nonexistent"]
                r1.should_fail_with Missing_Input_Columns

+        if data.connection.dialect.dialect_flags.primary_key.allows_nulls then
+            group_builder.specify "should not fail if the primary key contains nulls" <|
+                t1 = Table.new [['X', [1, Nothing, 3]], ['Y', [4, 5, Nothing]]]
+                run_with_and_without_output <|
+                    r2 = t1.select_into_database_table data.connection (Name_Generator.random_name "primary-key-null-1") temporary=True primary_key=['X']
+                    r2.row_count . should_equal 3
+
+        if data.connection.dialect.dialect_flags.primary_key.allows_nulls.not then
+            group_builder.specify "should fail if the primary key contains nulls" <|
+                t1 = Table.new [['X', [1, Nothing, 3]], ['Y', [4, 5, Nothing]]]
+                run_with_and_without_output <|
+                    r1 = t1.select_into_database_table data.connection (Name_Generator.random_name "primary-key-null-0") temporary=True primary_key=[]
+                    r1.row_count . should_equal 3
+
+                    r2 = t1.select_into_database_table data.connection (Name_Generator.random_name "primary-key-null-1") temporary=True primary_key=['X']
+                    r2.should_fail_with Null_Values_In_Key_Columns
+                    r2.catch . example_row . should_equal [Nothing, 5]
+
+                    r3 = t1.select_into_database_table data.connection (Name_Generator.random_name "primary-key-null-2") temporary=True primary_key=['Y']
+                    r3.should_fail_with Null_Values_In_Key_Columns
+                    r3.catch . example_row . should_equal [3, Nothing]
+
+                    r4 = t1.select_into_database_table data.connection (Name_Generator.random_name "primary-key-null-3") temporary=True primary_key=['X', 'Y']
+                    r4.should_fail_with Null_Values_In_Key_Columns
+                    r4.catch . example_row . should_equal [Nothing, 5]
+
+                large_with_null = (0.up_to 1010).to_vector + [Nothing]
+                big_table = Table.new [['X', large_with_null+large_with_null]]
+                Context.Output.with_disabled <|
+                    r5 = big_table.select_into_database_table data.connection (Name_Generator.random_name "primary-key-null-4") temporary=True primary_key=['X']
+                    r5.column_names . should_equal ['X']
+                    r5.row_count . should_equal 1000
+                Context.Output.with_enabled <|
+                    r5 = big_table.select_into_database_table data.connection (Name_Generator.random_name "primary-key-null-5") temporary=True primary_key=['X']
+                    r5.should_fail_with Null_Values_In_Key_Columns
+
        snowflake_constraints_pending = if prefix.contains "Snowflake" then "Disabled until https://github.com/enso-org/enso/issues/10737 is resolved."
        group_builder.specify "should fail if the primary key is not unique" pending=snowflake_constraints_pending <|
            t1 = Table.new [["X", [1, 2, 1]], ["Y", ['b', 'b', 'a']]]
@ -525,6 +561,49 @@ add_specs suite_builder setup make_new_connection persistent_connector=True =
                r1 = db_table.select_into_database_table data.connection (Name_Generator.random_name "copied-table") temporary=True primary_key=["nonexistent"]
                r1.should_fail_with Missing_Input_Columns

+        if data.connection.dialect.dialect_flags.primary_key.allows_nulls then
+            group_builder.specify "should fail when the primary key contains nulls" <|
+                t = Table.new [['X', [1, Nothing, 3]], ['Y', [4, 5, Nothing]]]
+                db_table = t.select_into_database_table data.connection (Name_Generator.random_name "source-table-nulls-1") temporary=True primary_key=Nothing
+                Problems.assume_no_problems db_table
+
+                run_with_and_without_output <|
+                    r2 = db_table.select_into_database_table data.connection (Name_Generator.random_name "primary-key-null-2") temporary=True primary_key=['X']
+                    r2.row_count . should_equal 3
+
+        if data.connection.dialect.dialect_flags.primary_key.allows_nulls.not then
+            group_builder.specify "should fail when the primary key contains nulls" <|
+                t = Table.new [['X', [1, Nothing, 3]], ['Y', [4, 5, Nothing]]]
+                db_table = t.select_into_database_table data.connection (Name_Generator.random_name "source-table-nulls-1") temporary=True primary_key=Nothing
+                Problems.assume_no_problems db_table
+
+                run_with_and_without_output <|
+                    r1 = db_table.select_into_database_table data.connection (Name_Generator.random_name "primary-key-null-1") temporary=True primary_key=[]
+                    r1.row_count . should_equal 3
+
+                    r2 = db_table.select_into_database_table data.connection (Name_Generator.random_name "primary-key-null-2") temporary=True primary_key=['X']
+                    r2.should_fail_with Null_Values_In_Key_Columns
+                    r2.catch . example_row . should_equal [Nothing, 5]
+
+                    r3 = db_table.select_into_database_table data.connection (Name_Generator.random_name "primary-key-null-3") temporary=True primary_key=['Y']
+                    r3.should_fail_with Null_Values_In_Key_Columns
+                    r3.catch . example_row . should_equal [3, Nothing]
+
+                    r4 = db_table.select_into_database_table data.connection (Name_Generator.random_name "primary-key-null-4") temporary=True primary_key=['X', 'Y']
+                    r4.should_fail_with Null_Values_In_Key_Columns
+                    r4.catch . example_row . should_equal [Nothing, 5]
+
+                large_with_null = (0.up_to 1010).to_vector + [Nothing]
+                big_table = Table.new [['X', large_with_null+large_with_null]]
+                db_table_2 = big_table.select_into_database_table data.connection (Name_Generator.random_name "source-table-nulls-2") temporary=True primary_key=Nothing
+                Context.Output.with_disabled <|
+                    r5 = db_table_2.select_into_database_table data.connection (Name_Generator.random_name "primary-key-null-5") temporary=True primary_key=['X']
+                    r5.column_names . should_equal ['X']
+                    r5.row_count . should_equal 1000
+                Context.Output.with_enabled <|
+                    r5 = db_table_2.select_into_database_table data.connection (Name_Generator.random_name "primary-key-null-6") temporary=True primary_key=['X']
+                    r5.should_fail_with Null_Values_In_Key_Columns
+
        snowflake_constraints_pending = if prefix.contains "Snowflake" then "Disabled until https://github.com/enso-org/enso/issues/10737 is resolved."
        group_builder.specify "should fail when the primary key is not unique" pending=snowflake_constraints_pending <|
            t = Table.new [["X", [1, 2, 1]], ["Y", ['b', 'b', 'a']]]