Skip to content

Commit

Permalink
*: Simplify the code of executing "CREATE TABLE" (#9629)
Browse files Browse the repository at this point in the history 
ref #6233

*: Simplify the code of executing "CREATE TABLE"
Remove the clickhouse code for "CREATE TEMPORARY TABLE"/"CREATE TABLE ... AS ..."/"CREATE Materialized View"

Co-authored-by: Lloyd-Pottiger <[email protected]>
  • Loading branch information
@JaySon-Huang @Lloyd-Pottiger
JaySon-Huang and Lloyd-Pottiger authored Nov 19, 2024
1 parent d064f96 commit d84d282
Show file tree
Hide file tree
Showing 11 changed files with 90 additions and 440 deletions.
12 changes: 0 additions & 12 deletions dbms/src/Databases/DatabasesCommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,7 @@ String getTableDefinitionFromCreateQuery(const ASTPtr & query)
/// We remove everything that is not needed for ATTACH from the query.
create.attach = true;
create.database.clear();
create.as_database.clear();
create.as_table.clear();
create.if_not_exists = false;
create.is_populate = false;

/// For views it is necessary to save the SELECT query itself, for the rest - on the contrary
if (!create.is_view && !create.is_materialized_view)
create.select = nullptr;

create.format = nullptr;
create.out_file = nullptr;
Expand All @@ -77,12 +70,7 @@ String getDatabaseDefinitionFromCreateQuery(const ASTPtr & query)
/// We remove everything that is not needed for ATTACH from the query
create.attach = true;
create.table.clear();
create.as_database.clear();
create.as_table.clear();
create.if_not_exists = false;
create.is_populate = false;

create.select = nullptr;

create.format = nullptr;
create.out_file = nullptr;
Expand Down
1 change: 0 additions & 1 deletion dbms/src/Debug/dbgFuncSchema.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include <Debug/dbgFuncSchema.h>
#include <Debug/dbgTools.h>
#include <Interpreters/Context.h>
#include <Interpreters/InterpreterCreateQuery.h>
#include <Parsers/ASTIdentifier.h>
#include <Parsers/ASTLiteral.h>
#include <Parsers/ParserCreateQuery.h>
Expand Down
158 changes: 20 additions & 138 deletions dbms/src/Interpreters/InterpreterCreateQuery.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,33 +400,12 @@ ColumnsDescription InterpreterCreateQuery::getColumnsDescription(
}


ColumnsDescription InterpreterCreateQuery::setColumns(
ASTCreateQuery & create,
const Block & as_select_sample,
const StoragePtr & as_storage) const
ColumnsDescription InterpreterCreateQuery::setColumns(ASTCreateQuery & create) const
{
ColumnsDescription res;

if (create.columns)
{
res = getColumnsDescription(*create.columns, context);
}
else if (!create.as_table.empty())
{
res = as_storage->getColumns();
}
else if (create.select)
{
for (size_t i = 0; i < as_select_sample.columns(); ++i)
res.ordinary.emplace_back(
as_select_sample.safeGetByPosition(i).name,
as_select_sample.safeGetByPosition(i).type);
}
else
throw Exception(
"Incorrect CREATE query: required list of column descriptions or AS section or SELECT.",
ErrorCodes::INCORRECT_QUERY);
if (!create.columns)
throw Exception("Incorrect CREATE query: required list of column descriptions", ErrorCodes::INCORRECT_QUERY);

ColumnsDescription res = getColumnsDescription(*create.columns, context);
/// Even if query has list of columns, canonicalize it (unfold Nested columns).
ASTPtr new_columns = formatColumns(res);
if (create.columns)
Expand All @@ -453,45 +432,6 @@ ColumnsDescription InterpreterCreateQuery::setColumns(
return res;
}

void InterpreterCreateQuery::setEngine(ASTCreateQuery & create) const
{
if (create.storage)
{
if (create.is_temporary && create.storage->engine->name != "Memory")
throw Exception(
"Temporary tables can only be created with ENGINE = Memory, not " + create.storage->engine->name,
ErrorCodes::INCORRECT_QUERY);

return;
}

if (create.is_temporary)
{
auto engine_ast = std::make_shared<ASTFunction>();
engine_ast->name = "Memory";
auto storage_ast = std::make_shared<ASTStorage>();
storage_ast->set(storage_ast->engine, engine_ast);
create.set(create.storage, storage_ast);
}
else if (!create.as_table.empty())
{
/// NOTE Getting the structure from the table specified in the AS is done not atomically with the creation of the table.

String as_database_name = create.as_database.empty() ? context.getCurrentDatabase() : create.as_database;
String as_table_name = create.as_table;

ASTPtr as_create_ptr = context.getCreateTableQuery(as_database_name, as_table_name);
const auto & as_create = typeid_cast<const ASTCreateQuery &>(*as_create_ptr);

if (as_create.is_view)
throw Exception(
"Cannot CREATE a table AS " + as_database_name + "." + as_table_name + ", it is a View",
ErrorCodes::INCORRECT_QUERY);

create.set(create.storage, as_create.storage->ptr());
}
}


/**
* Try to acquire a DDLGuard to execute the "CREATE TABLE" actions.
Expand Down Expand Up @@ -595,60 +535,26 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
create.attach = true;
}

if (create.to_database.empty())
create.to_database = current_database;

if (create.select && (create.is_view || create.is_materialized_view))
create.select->setDatabaseIfNeeded(current_database);

Block as_select_sample;
if (create.select && (!create.attach || !create.columns))
as_select_sample = InterpreterSelectWithUnionQuery::getSampleBlock(create.select->clone(), context);

String as_database_name = create.as_database.empty() ? current_database : create.as_database;
String as_table_name = create.as_table;

StoragePtr as_storage;
TableStructureLockHolder as_storage_lock;
if (!as_table_name.empty())
{
as_storage = context.getTable(as_database_name, as_table_name);
as_storage_lock = as_storage->lockStructureForShare(context.getCurrentQueryId());
}

/// Set and retrieve list of columns.
ColumnsDescription columns = setColumns(create, as_select_sample, as_storage);

/// Set the table engine if it was not specified explicitly.
setEngine(create);
ColumnsDescription columns = setColumns(create);

{
std::unique_ptr<DDLGuard> guard;

String data_path;
DatabasePtr database;
DatabasePtr database = context.getDatabase(database_name);
String data_path = database->getDataPath();

if (!create.is_temporary)
const auto guard = tryGetDDLGuard(
context,
database_name,
table_name,
create.if_not_exists,
/*timeout_seconds=*/5,
log_suffix);
if (!guard)
{
database = context.getDatabase(database_name);
data_path = database->getDataPath();

guard = tryGetDDLGuard(
context,
database_name,
table_name,
create.if_not_exists,
/*timeout_seconds=*/5,
log_suffix);
if (!guard)
{
// Not the owner to create IStorage instance, and the table is created
// completely, let's return
return {};
}
}
else if (context.tryGetExternalTable(table_name) && create.if_not_exists)
// Not the owner to create IStorage instance, and the table is created
// completely, let's return
return {};
}

// Guard is acquired, let's create the IStorage instance
StoragePtr res = StorageFactory::instance().get(
Expand All @@ -671,35 +577,16 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
// If we do step 2 before step 1, we may run into "can't find .sql file" error when applying DDL jobs.
// Besides, we make step 3 the final one, to ensure once we pass the check of context.isTableExist(database_name, table_name)`, the table must be created completely.

if (create.is_temporary)
context.getSessionContext().addExternalTable(table_name, res, query_ptr);
else
database->createTable(context, table_name, query_ptr);
database->createTable(context, table_name, query_ptr);

// register the storage instance into `ManagedStorages`
res->startup();

if (!create.is_temporary)
database->attachTable(table_name, res);
database->attachTable(table_name, res);

// the table has been created completely
}

/// If the query is a CREATE SELECT, insert the data into the table.
if (create.select && !create.attach && !create.is_view && (!create.is_materialized_view || create.is_populate))
{
auto insert = std::make_shared<ASTInsertQuery>();

if (!create.is_temporary)
insert->database = database_name;

insert->table = table_name;
insert->select = create.select->clone();

return InterpreterInsertQuery(insert, create.is_temporary ? context.getSessionContext() : context, false)
.execute();
}

return {};
}

Expand Down Expand Up @@ -740,11 +627,6 @@ void InterpreterCreateQuery::checkAccess(const ASTCreateQuery & create)
throw Exception("Cannot create database in readonly mode", ErrorCodes::READONLY);
}

if (create.is_temporary && readonly >= 2)
{
return;
}

throw Exception("Cannot create table in readonly mode", ErrorCodes::READONLY);
}
} // namespace DB
6 changes: 1 addition & 5 deletions dbms/src/Interpreters/InterpreterCreateQuery.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,7 @@ class InterpreterCreateQuery : public IInterpreter
BlockIO createTable(ASTCreateQuery & create);

/// Calculate list of columns of table and return it.
ColumnsDescription setColumns(
ASTCreateQuery & create,
const Block & as_select_sample,
const StoragePtr & as_storage) const;
void setEngine(ASTCreateQuery & create) const;
ColumnsDescription setColumns(ASTCreateQuery & create) const;
void checkAccess(const ASTCreateQuery & create);

ASTPtr query_ptr;
Expand Down
12 changes: 5 additions & 7 deletions dbms/src/Interpreters/tests/gtest_interpreter_create_query.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@
#include <TiDB/Schema/SchemaNameMapper.h>
#include <TiDB/Schema/TiDB.h>

namespace DB
{
namespace tests
namespace DB::tests
{
class InterperCreateQueryTiFlashTest : public ::testing::Test
{
Expand All @@ -34,7 +32,7 @@ class InterperCreateQueryTiFlashTest : public ::testing::Test
static void TearDownTestCase() {}

InterperCreateQueryTiFlashTest()
: log(&Poco::Logger::get("InterperCreateQuery"))
: log(Logger::get("InterperCreateQuery"))
, context(TiFlashTestEnv::getGlobalContext())
{}

Expand Down Expand Up @@ -136,7 +134,7 @@ class InterperCreateQueryTiFlashTest : public ::testing::Test


protected:
Poco::Logger * log;
LoggerPtr log;
Context & context;
};

Expand Down Expand Up @@ -165,5 +163,5 @@ try
}
}
CATCH
} // namespace tests
} // namespace DB

} // namespace DB::tests
48 changes: 2 additions & 46 deletions dbms/src/Parsers/ASTCreateQuery.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,22 +92,13 @@ class ASTCreateQuery : public ASTQueryWithOutput
public:
bool attach{false}; /// Query ATTACH TABLE, not CREATE TABLE.
bool if_not_exists{false};
bool is_view{false};
bool is_materialized_view{false};
bool is_populate{false};
bool is_temporary{false};
String database;
String table;
ASTExpressionList * columns = nullptr;
String to_database; /// For CREATE MATERIALIZED VIEW mv TO table.
String to_table;
ASTStorage * storage = nullptr;
String as_database;
String as_table;
ASTSelectWithUnionQuery * select = nullptr;

/** Get the text that identifies this element. */
String getID() const override { return (attach ? "AttachQuery_" : "CreateQuery_") + database + "_" + table; };
String getID() const override { return (attach ? "AttachQuery_" : "CreateQuery_") + database + "_" + table; }

ASTPtr clone() const override
{
Expand All @@ -118,8 +109,6 @@ class ASTCreateQuery : public ASTQueryWithOutput
res->set(res->columns, columns->clone());
if (storage)
res->set(res->storage, storage->clone());
if (select)
res->set(res->select, select->clone());

cloneOutputOptions(*res);

Expand All @@ -146,31 +135,11 @@ class ASTCreateQuery : public ASTQueryWithOutput

{
std::string what = "TABLE";
if (is_view)
what = "VIEW";
if (is_materialized_view)
what = "MATERIALIZED VIEW";

settings.ostr << (settings.hilite ? hilite_keyword : "") << (attach ? "ATTACH " : "CREATE ")
<< (is_temporary ? "TEMPORARY " : "") << what << " "
settings.ostr << (settings.hilite ? hilite_keyword : "") << (attach ? "ATTACH " : "CREATE ") << what << " "
<< (if_not_exists ? "IF NOT EXISTS " : "") << (settings.hilite ? hilite_none : "")
<< (!database.empty() ? backQuoteIfNeed(database) + "." : "") << backQuoteIfNeed(table);
}

if (!to_table.empty())
{
settings.ostr << (settings.hilite ? hilite_keyword : "") << " TO " << (settings.hilite ? hilite_none : "")
<< (!to_database.empty() ? backQuoteIfNeed(to_database) + "." : "")
<< backQuoteIfNeed(to_table);
}

if (!as_table.empty())
{
settings.ostr << (settings.hilite ? hilite_keyword : "") << " AS " << (settings.hilite ? hilite_none : "")
<< (!as_database.empty() ? backQuoteIfNeed(as_database) + "." : "")
<< backQuoteIfNeed(as_table);
}

if (columns)
{
settings.ostr << (settings.one_line ? " (" : "\n(");
Expand All @@ -182,19 +151,6 @@ class ASTCreateQuery : public ASTQueryWithOutput

if (storage)
storage->formatImpl(settings, state, frame);

if (is_populate)
{
settings.ostr << (settings.hilite ? hilite_keyword : "") << " POPULATE"
<< (settings.hilite ? hilite_none : "");
}

if (select)
{
settings.ostr << (settings.hilite ? hilite_keyword : "") << " AS" << settings.nl_or_ws
<< (settings.hilite ? hilite_none : "");
select->formatImpl(settings, state, frame);
}
}
};

Expand Down
2 changes: 1 addition & 1 deletion dbms/src/Parsers/ASTQueryWithOutput.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void ASTQueryWithOutput::formatImpl(const FormatSettings & s, FormatState & stat

bool ASTQueryWithOutput::resetOutputASTIfExist(IAST & ast)
{
if (auto ast_with_output = dynamic_cast<ASTQueryWithOutput *>(&ast))
if (auto * ast_with_output = dynamic_cast<ASTQueryWithOutput *>(&ast))
{
ast_with_output->format.reset();
ast_with_output->out_file.reset();
Expand Down
Loading

0 comments on commit d84d282

Please sign in to comment.