From ebf0a3119e6a9425f167dc5ecac1d3477e51800d Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Fri, 16 Apr 2021 13:15:35 +0300 Subject: [PATCH] fix data_type_default_nullable on attach --- src/Databases/DatabaseOnDisk.cpp | 4 ++-- src/Interpreters/InterpreterCreateQuery.cpp | 10 +++++----- src/Interpreters/InterpreterCreateQuery.h | 2 +- src/Interpreters/InterpreterSystemQuery.cpp | 2 +- .../parseColumnsListForTableFunction.cpp | 2 +- .../0_stateless/01269_create_with_null.reference | 3 +++ tests/queries/0_stateless/01269_create_with_null.sql | 12 ++++++++++++ 7 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/Databases/DatabaseOnDisk.cpp b/src/Databases/DatabaseOnDisk.cpp index 358f9030db5..14ad1c7e4c5 100644 --- a/src/Databases/DatabaseOnDisk.cpp +++ b/src/Databases/DatabaseOnDisk.cpp @@ -58,7 +58,7 @@ std::pair createTableFromAST( auto table_function = factory.get(ast_create_query.as_table_function, context); ColumnsDescription columns; if (ast_create_query.columns_list && ast_create_query.columns_list->columns) - columns = InterpreterCreateQuery::getColumnsDescription(*ast_create_query.columns_list->columns, context, false); + columns = InterpreterCreateQuery::getColumnsDescription(*ast_create_query.columns_list->columns, context, true); StoragePtr storage = table_function->execute(ast_create_query.as_table_function, context, ast_create_query.table, std::move(columns)); storage->renameInMemory(ast_create_query); return {ast_create_query.table, storage}; @@ -69,7 +69,7 @@ std::pair createTableFromAST( if (!ast_create_query.columns_list || !ast_create_query.columns_list->columns) throw Exception("Missing definition of columns.", ErrorCodes::EMPTY_LIST_OF_COLUMNS_PASSED); - ColumnsDescription columns = InterpreterCreateQuery::getColumnsDescription(*ast_create_query.columns_list->columns, context, false); + ColumnsDescription columns = InterpreterCreateQuery::getColumnsDescription(*ast_create_query.columns_list->columns, context, true); ConstraintsDescription constraints = InterpreterCreateQuery::getConstraintsDescription(ast_create_query.columns_list->constraints); return diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index 6d234f5f846..8db4415822f 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -363,7 +363,7 @@ ASTPtr InterpreterCreateQuery::formatConstraints(const ConstraintsDescription & } ColumnsDescription InterpreterCreateQuery::getColumnsDescription( - const ASTExpressionList & columns_ast, ContextPtr context_, bool sanity_check_compression_codecs) + const ASTExpressionList & columns_ast, ContextPtr context_, bool attach) { /// First, deduce implicit types. @@ -372,6 +372,7 @@ ColumnsDescription InterpreterCreateQuery::getColumnsDescription( ASTPtr default_expr_list = std::make_shared(); NamesAndTypesList column_names_and_types; + bool make_columns_nullable = !attach && context_->getSettingsRef().data_type_default_nullable; for (const auto & ast : columns_ast.children) { @@ -390,8 +391,7 @@ ColumnsDescription InterpreterCreateQuery::getColumnsDescription( if (*col_decl.null_modifier) column_type = makeNullable(column_type); } - /// XXX: context_ or context ? - else if (context_->getSettingsRef().data_type_default_nullable) + else if (make_columns_nullable) { column_type = makeNullable(column_type); } @@ -436,6 +436,7 @@ ColumnsDescription InterpreterCreateQuery::getColumnsDescription( if (!default_expr_list->children.empty()) defaults_sample_block = validateColumnsDefaultsAndGetSampleBlock(default_expr_list, column_names_and_types, context_); + bool sanity_check_compression_codecs = !attach && !context_->getSettingsRef().allow_suspicious_codecs; ColumnsDescription res; auto name_type_it = column_names_and_types.begin(); for (auto ast_it = columns_ast.children.begin(); ast_it != columns_ast.children.end(); ++ast_it, ++name_type_it) @@ -511,8 +512,7 @@ InterpreterCreateQuery::TableProperties InterpreterCreateQuery::setProperties(AS if (create.columns_list->columns) { - bool sanity_check_compression_codecs = !create.attach && !getContext()->getSettingsRef().allow_suspicious_codecs; - properties.columns = getColumnsDescription(*create.columns_list->columns, getContext(), sanity_check_compression_codecs); + properties.columns = getColumnsDescription(*create.columns_list->columns, getContext(), create.attach); } if (create.columns_list->indices) diff --git a/src/Interpreters/InterpreterCreateQuery.h b/src/Interpreters/InterpreterCreateQuery.h index 30db7dbdc8e..f674bf19123 100644 --- a/src/Interpreters/InterpreterCreateQuery.h +++ b/src/Interpreters/InterpreterCreateQuery.h @@ -53,7 +53,7 @@ public: /// Obtain information about columns, their types, default values and column comments, /// for case when columns in CREATE query is specified explicitly. - static ColumnsDescription getColumnsDescription(const ASTExpressionList & columns, ContextPtr context, bool sanity_check_compression_codecs); + static ColumnsDescription getColumnsDescription(const ASTExpressionList & columns, ContextPtr context, bool attach); static ConstraintsDescription getConstraintsDescription(const ASTExpressionList * constraints); static void prepareOnClusterQuery(ASTCreateQuery & create, ContextPtr context, const String & cluster_name); diff --git a/src/Interpreters/InterpreterSystemQuery.cpp b/src/Interpreters/InterpreterSystemQuery.cpp index c13b9d97037..02d5296a9d2 100644 --- a/src/Interpreters/InterpreterSystemQuery.cpp +++ b/src/Interpreters/InterpreterSystemQuery.cpp @@ -429,7 +429,7 @@ StoragePtr InterpreterSystemQuery::tryRestartReplica(const StorageID & replica, auto & create = create_ast->as(); create.attach = true; - auto columns = InterpreterCreateQuery::getColumnsDescription(*create.columns_list->columns, system_context, false); + auto columns = InterpreterCreateQuery::getColumnsDescription(*create.columns_list->columns, system_context, true); auto constraints = InterpreterCreateQuery::getConstraintsDescription(create.columns_list->constraints); auto data_path = database->getTableDataPath(create); diff --git a/src/TableFunctions/parseColumnsListForTableFunction.cpp b/src/TableFunctions/parseColumnsListForTableFunction.cpp index 659aa779ede..08e80ef425a 100644 --- a/src/TableFunctions/parseColumnsListForTableFunction.cpp +++ b/src/TableFunctions/parseColumnsListForTableFunction.cpp @@ -25,7 +25,7 @@ ColumnsDescription parseColumnsListFromString(const std::string & structure, Con if (!columns_list) throw Exception("Could not cast AST to ASTExpressionList", ErrorCodes::LOGICAL_ERROR); - return InterpreterCreateQuery::getColumnsDescription(*columns_list, context, !settings.allow_suspicious_codecs); + return InterpreterCreateQuery::getColumnsDescription(*columns_list, context, false); } } diff --git a/tests/queries/0_stateless/01269_create_with_null.reference b/tests/queries/0_stateless/01269_create_with_null.reference index 86be41bc06a..73f834da75a 100644 --- a/tests/queries/0_stateless/01269_create_with_null.reference +++ b/tests/queries/0_stateless/01269_create_with_null.reference @@ -2,3 +2,6 @@ Nullable(Int32) Int32 Nullable(Int32) Int32 CREATE TABLE default.data_null\n(\n `a` Nullable(Int32),\n `b` Int32,\n `c` Nullable(Int32),\n `d` Int32\n)\nENGINE = Memory Nullable(Int32) Int32 Nullable(Int32) Nullable(Int32) CREATE TABLE default.set_null\n(\n `a` Nullable(Int32),\n `b` Int32,\n `c` Nullable(Int32),\n `d` Nullable(Int32)\n)\nENGINE = Memory +CREATE TABLE default.set_null\n(\n `a` Nullable(Int32),\n `b` Int32,\n `c` Nullable(Int32),\n `d` Nullable(Int32)\n)\nENGINE = Memory +CREATE TABLE default.cannot_be_nullable\n(\n `n` Nullable(Int8),\n `a` Array(UInt8)\n)\nENGINE = Memory +CREATE TABLE default.cannot_be_nullable\n(\n `n` Nullable(Int8),\n `a` Array(UInt8)\n)\nENGINE = Memory diff --git a/tests/queries/0_stateless/01269_create_with_null.sql b/tests/queries/0_stateless/01269_create_with_null.sql index 856b6ea75f4..faa6b84e9e4 100644 --- a/tests/queries/0_stateless/01269_create_with_null.sql +++ b/tests/queries/0_stateless/01269_create_with_null.sql @@ -1,5 +1,6 @@ DROP TABLE IF EXISTS data_null; DROP TABLE IF EXISTS set_null; +DROP TABLE IF EXISTS cannot_be_nullable; SET data_type_default_nullable='false'; @@ -45,6 +46,17 @@ INSERT INTO set_null VALUES (NULL, 2, NULL, NULL); SELECT toTypeName(a), toTypeName(b), toTypeName(c), toTypeName(d) FROM set_null; SHOW CREATE TABLE set_null; +DETACH TABLE set_null; +ATTACH TABLE set_null; +SHOW CREATE TABLE set_null; + +CREATE TABLE cannot_be_nullable (n Int8, a Array(UInt8)) ENGINE=Memory; -- { serverError 43 } +CREATE TABLE cannot_be_nullable (n Int8, a Array(UInt8) NOT NULL) ENGINE=Memory; +SHOW CREATE TABLE cannot_be_nullable; +DETACH TABLE cannot_be_nullable; +ATTACH TABLE cannot_be_nullable; +SHOW CREATE TABLE cannot_be_nullable; DROP TABLE data_null; DROP TABLE set_null; +DROP TABLE cannot_be_nullable;