-
Notifications
You must be signed in to change notification settings - Fork 6.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ISSUES-117 support temporary table management #1824
Conversation
b9340f0
to
1070e46
Compare
1070e46
to
bfe48f1
Compare
0603d07
to
19e7c29
Compare
{ | ||
if (!name_p.parse(pos, database, expected)) | ||
return false; | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style:
}
else
{
...
}
@@ -30,6 +31,9 @@ static ColumnPtr getFilteredDatabases(const ASTPtr & query, const Context & cont | |||
for (const auto & db : context.getDatabases()) | |||
column->insert(db.first); | |||
|
|||
std::string temporary_database = ""; | |||
column->insert(temporary_database); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just column->insertDefault()
`column->insert(String{})
CREATE TEMPORARY TABLE temp_tab (number UInt64); | ||
INSERT INTO temp_tab SELECT number FROM system.numbers LIMIT 1; | ||
SELECT number FROM temp_tab; | ||
DROP TEMPORARY TABLE temp_tab; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the new syntax backwards-compatible with the previous one?
res_columns[1]->insert(table_name); | ||
res_columns[2]->insert(iterator->table()->getName()); | ||
res_columns[3]->insert(static_cast<UInt64>(database->getTableMetadataModificationTime(context, table_name))); | ||
Tables externalTables = context.getSessionContext().getExternalTables(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Camel case (externalTables
) is used only for function names.
@@ -30,6 +31,9 @@ static ColumnPtr getFilteredDatabases(const ASTPtr & query, const Context & cont | |||
for (const auto & db : context.getDatabases()) | |||
column->insert(db.first); | |||
|
|||
std::string temporary_database = ""; | |||
column->insert(temporary_database); | |||
|
|||
Block block { ColumnWithTypeAndName( std::move(column), std::make_shared<DataTypeString>(), "database" ) }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also put column temporary
into the block.
It allows excluding the temporary database in case of WHERE temporary != 0
@@ -38,6 +38,10 @@ String InterpreterShowTablesQuery::getRewrittenQuery() | |||
*/ | |||
context.assertDatabaseExists(database, false); | |||
|
|||
if (query.temporary) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style
3c4987f
to
02afa74
Compare
if (s_from.ignore(pos, expected)) | ||
{ | ||
if (query->temporary) | ||
throw Exception("Unable to parse FROM,Because the temporary table does not have a database."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add an error code?
I would suggest ErrorCodes::SYNTAX_ERROR
, but I am not sure it is a syntax error, it is some kind of user error, and the check should be done in the interpreter.
@@ -14,7 +14,7 @@ namespace DB | |||
class ParserShowTablesQuery : public IParserBase | |||
{ | |||
protected: | |||
const char * getName() const { return "SHOW TABLES|DATABASES query"; } | |||
const char * getName() const { return "SHOW [TEMPORARY] TABLES|DATABASES query"; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add the full syntax with NOT LIKE
...
{ | ||
Tables external_tables = context.getSessionContext().getExternalTables(); | ||
|
||
for (auto table = external_tables.begin(); table != external_tables.end(); table++) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO it is better to use for-range loop
a9d6e66
to
2c3451b
Compare
2c3451b
to
9f66aca
Compare
fix #117
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en