Revert "Not better"

This reverts commit 66175cb9c8.
This commit is contained in:
kssenii 2022-11-18 13:00:46 +01:00
parent 66175cb9c8
commit babc0c84b0
13 changed files with 262 additions and 212 deletions

View File

@ -82,8 +82,8 @@ BackupSettings BackupSettings::fromBackupQuery(const ASTBackupQuery & query)
const auto & settings = query.settings->as<const ASTSetQuery &>().changes;
for (const auto & setting : settings)
{
if (setting.name == "compression_level")
res.compression_level = static_cast<int>(SettingFieldInt64{setting.value}.value);
if (setting.getName() == "compression_level")
res.compression_level = static_cast<int>(SettingFieldInt64{setting.getFieldValue()}.value);
else
#define GET_SETTINGS_FROM_BACKUP_QUERY_HELPER(TYPE, NAME) \
if (setting.name == #NAME) \

View File

@ -14,12 +14,16 @@ struct SettingChange
String name;
Field value;
/// A setting value which cannot be put in Field.
ASTPtr value_ast;
SettingChange() = default;
SettingChange(std::string_view name_, const Field & value_) : name(name_), value(value_) {}
SettingChange(std::string_view name_, Field && value_) : name(name_), value(std::move(value_)) {}
SettingChange(std::string_view name_, const ASTPtr & value_) : name(name_), value_ast(value_->clone()) {}
friend bool operator ==(const SettingChange & lhs, const SettingChange & rhs) { return (lhs.name == rhs.name) && (lhs.value == rhs.value); }
friend bool operator ==(const SettingChange & lhs, const SettingChange & rhs) { return (lhs.name == rhs.name) && (lhs.value == rhs.value) && (lhs.value_ast == rhs.value_ast); }
friend bool operator !=(const SettingChange & lhs, const SettingChange & rhs) { return !(lhs == rhs); }
};

View File

@ -511,7 +511,7 @@ void buildSourceConfiguration(
{
AutoPtr<Element> settings_element(doc->createElement("settings"));
outer_element->appendChild(settings_element);
for (const auto & [name, value] : settings->changes)
for (const auto & [name, value, _] : settings->changes)
{
AutoPtr<Element> setting_change_element(doc->createElement(name));
settings_element->appendChild(setting_change_element);

View File

@ -0,0 +1,93 @@
#include <Disks/getDiskConfigurationFromAST.h>
#include <Common/assert_cast.h>
#include <Common/FieldVisitorToString.h>
#include <Common/logger_useful.h>
#include <Disks/DiskFactory.h>
#include <Interpreters/evaluateConstantExpression.h>
#include <Parsers/ASTFunction.h>
#include <Parsers/ASTIdentifier.h>
#include <Parsers/ASTLiteral.h>
#include <Parsers/ASTFunctionWithKeyValueArguments.h>
#include <Storages/checkAndGetLiteralArgument.h>
#include <Poco/DOM/Document.h>
#include <Poco/DOM/Element.h>
#include <Poco/DOM/Text.h>
namespace DB
{
namespace ErrorCodes
{
extern const int BAD_ARGUMENTS;
}
[[noreturn]] static void throwBadConfiguration(const std::string & message = "")
{
throw Exception(
ErrorCodes::BAD_ARGUMENTS,
"Incorrect configuration{}. Example of expected configuration: `(type=s3 ...`)`",
message.empty() ? "" : ": " + message);
}
Poco::AutoPtr<Poco::XML::Document> getDiskConfigurationFromASTImpl(const std::string & root_name, const ASTs & disk_args, ContextPtr context)
{
if (disk_args.empty())
throwBadConfiguration("expected non-empty list of arguments");
Poco::AutoPtr<Poco::XML::Document> xml_document(new Poco::XML::Document());
Poco::AutoPtr<Poco::XML::Element> root(xml_document->createElement("disk"));
xml_document->appendChild(root);
Poco::AutoPtr<Poco::XML::Element> disk_configuration(xml_document->createElement(root_name));
root->appendChild(disk_configuration);
for (const auto & arg : disk_args)
{
const auto * setting_function = arg->as<const ASTFunction>();
if (!setting_function || setting_function->name != "equals")
throwBadConfiguration("expected configuration arguments as key=value pairs");
const auto * function_args_expr = assert_cast<const ASTExpressionList *>(setting_function->arguments.get());
if (!function_args_expr)
throwBadConfiguration("expected a list of key=value arguments");
auto function_args = function_args_expr->children;
if (function_args.empty())
throwBadConfiguration("expected a non-empty list of key=value arguments");
auto * key_identifier = function_args[0]->as<ASTIdentifier>();
if (!key_identifier)
throwBadConfiguration("expected the key (key=value) to be identifier");
const std::string & key = key_identifier->name();
Poco::AutoPtr<Poco::XML::Element> key_element(xml_document->createElement(key));
disk_configuration->appendChild(key_element);
if (!function_args[1]->as<ASTLiteral>() && !function_args[1]->as<ASTIdentifier>())
throwBadConfiguration("expected values to be literals or identifiers");
auto value = evaluateConstantExpressionOrIdentifierAsLiteral(function_args[1], context);
Poco::AutoPtr<Poco::XML::Text> value_element(xml_document->createTextNode(convertFieldToString(value->as<ASTLiteral>()->value)));
key_element->appendChild(value_element);
}
return xml_document;
}
DiskConfigurationPtr getDiskConfigurationFromAST(const std::string & root_name, const ASTs & disk_args, ContextPtr context)
{
auto xml_document = getDiskConfigurationFromASTImpl(root_name, disk_args, context);
Poco::AutoPtr<Poco::Util::XMLConfiguration> conf(new Poco::Util::XMLConfiguration());
conf->load(xml_document);
std::ostringstream ss; // STYLE_CHECK_ALLOW_STD_STRING_STREAM
ss.exceptions(std::ios::failbit);
conf->save(ss);
LOG_TEST(&Poco::Logger::get("getDiskConfigurationFromAST"), "Received disk configuration: {}", ss.str());
return conf;
}
}

View File

@ -0,0 +1,28 @@
#pragma once
#include <Poco/Util/AbstractConfiguration.h>
#include <Poco/DOM/AutoPtr.h>
#include <Poco/Util/XMLConfiguration.h>
#include <Parsers/IAST_fwd.h>
#include <Interpreters/Context_fwd.h>
namespace DB
{
using DiskConfigurationPtr = Poco::AutoPtr<Poco::Util::AbstractConfiguration>;
/**
* Transform a list of pairs ( key1=value1, key2=value2, ... ), where keys and values are ASTLiteral or ASTIdentifier
* into
* <root_name>
* <key1>value1</key1>
* <key2>value2</key2>
* ...
* </root_name>
*
* Used in case disk configuration is passed via AST when creating
* a disk object on-the-fly without any configuration file.
*/
DiskConfigurationPtr getDiskConfigurationFromAST(const std::string & root_name, const ASTs & disk_args, ContextPtr context);
}

View File

@ -169,10 +169,6 @@ namespace
/// S3('url', ['aws_access_key_id', 'aws_secret_access_key',] ...)
wipePasswordFromS3TableEngineArguments(*storage.engine, data);
}
else if (storage.engine->arguments && storage.settings)
{
wipeDiskSettingsArguments(*storage.settings, data);
}
}
static void wipePasswordFromS3TableEngineArguments(ASTFunction & engine, Data & data)
@ -391,33 +387,6 @@ namespace
data.password_was_hidden = true;
}
static void wipeDiskSettingsArguments(ASTSetQuery & settings, Data & data)
{
if (settings.changes.empty())
return;
for (auto & setting : settings.changes)
{
if (setting.name != "disk")
continue;
if constexpr (check_only)
{
data.can_contain_password = true;
return;
}
if (setting.value.getType() != Field::Types::Which::Array)
continue;
for (auto & disk_setting : setting.value.safeGet<Array>())
disk_setting = Tuple({disk_setting.safeGet<Tuple>()[0], "[HIDDEN]"});
data.password_was_hidden = true;
return;
}
}
static void removeArgumentsAfter(ASTFunction & function, Data & data, size_t new_num_arguments)
{
if (!function.arguments)

View File

@ -19,7 +19,7 @@ void ASTSetQuery::updateTreeHashImpl(SipHash & hash_state) const
}
}
void ASTSetQuery::formatImpl(const FormatSettings & format, FormatState &, FormatStateStacked) const
void ASTSetQuery::formatImpl(const FormatSettings & format, FormatState & state, FormatStateStacked stacked) const
{
if (is_standalone)
format.ostr << (format.hilite ? hilite_keyword : "") << "SET " << (format.hilite ? hilite_none : "");
@ -34,6 +34,12 @@ void ASTSetQuery::formatImpl(const FormatSettings & format, FormatState &, Forma
first = false;
formatSettingName(change.name, format.ostr);
if (change.value_ast)
{
format.ostr << " = ";
change.value_ast->formatImpl(format, state, stacked);
}
else
format.ostr << " = " << applyVisitor(FieldVisitorToString(), change.value);
}

View File

@ -13,7 +13,6 @@
#include <Common/FieldVisitorToString.h>
#include <Common/SettingsChanges.h>
#include <Common/typeid_cast.h>
#include <ranges>
namespace DB
{
@ -99,11 +98,11 @@ bool ParserSetQuery::parseNameValuePair(SettingChange & change, IParser::Pos & p
ParserLiteralOrMap literal_or_map_p;
ParserToken s_eq(TokenType::Equals);
ParserSetQuery set_p(true);
ParserToken l_br(TokenType::OpeningRoundBracket);
ParserToken r_br(TokenType::ClosingRoundBracket);
ParserFunction function_p;
ASTPtr name;
ASTPtr value;
ASTPtr function_ast;
if (!name_p.parse(pos, name, expected))
return false;
@ -115,15 +114,12 @@ bool ParserSetQuery::parseNameValuePair(SettingChange & change, IParser::Pos & p
value = std::make_shared<ASTLiteral>(Field(static_cast<UInt64>(1)));
else if (ParserKeyword("FALSE").ignore(pos, expected))
value = std::make_shared<ASTLiteral>(Field(static_cast<UInt64>(0)));
else if (ParserKeyword("SETTINGS").ignore(pos, expected)
&& l_br.ignore(pos, expected)
&& set_p.parse(pos, value, expected)
&& r_br.ignore(pos, expected))
/// for SETTINGS disk=disk(type='s3', path='', ...)
else if (function_p.parse(pos, function_ast, expected) && function_ast->as<ASTFunction>()->name == "disk")
{
tryGetIdentifierNameInto(name, change.name);
auto changes = value->as<ASTSetQuery>()->changes;
auto values = changes | std::views::transform([](const auto & setting) { return Tuple{setting.name, setting.value}; });
change.value = Array(values.begin(), values.end());
change.value_ast = function_ast;
return true;
}
else if (!literal_or_map_p.parse(pos, value, expected))
@ -140,13 +136,12 @@ bool ParserSetQuery::parseNameValuePairWithDefault(SettingChange & change, Strin
ParserCompoundIdentifier name_p;
ParserLiteralOrMap value_p;
ParserToken s_eq(TokenType::Equals);
ParserSetQuery set_p(true);
ParserToken l_br(TokenType::OpeningRoundBracket);
ParserToken r_br(TokenType::ClosingRoundBracket);
ParserFunction function_p;
ASTPtr name;
ASTPtr value;
bool is_default = false;
ASTPtr function_ast;
if (!name_p.parse(pos, name, expected))
return false;
@ -160,25 +155,22 @@ bool ParserSetQuery::parseNameValuePairWithDefault(SettingChange & change, Strin
value = std::make_shared<ASTLiteral>(Field(static_cast<UInt64>(0)));
else if (ParserKeyword("DEFAULT").ignore(pos, expected))
is_default = true;
else if (ParserKeyword("SETTINGS").ignore(pos, expected)
&& l_br.ignore(pos, expected)
&& set_p.parse(pos, value, expected)
&& r_br.ignore(pos, expected))
/// for SETTINGS disk=disk(type='s3', path='', ...)
else if (function_p.parse(pos, function_ast, expected) && function_ast->as<ASTFunction>()->name == "disk")
{
tryGetIdentifierNameInto(name, change.name);
auto changes = value->as<ASTSetQuery>()->changes;
auto values = changes | std::views::transform([](const auto & setting) { return Tuple{setting.name, setting.value}; });
change.value = Array(values.begin(), values.end());
tryGetIdentifierNameInto(name, change.getName());
change.setASTValue(function_ast);
return true;
}
else if (!value_p.parse(pos, value, expected))
return false;
tryGetIdentifierNameInto(name, change.name);
tryGetIdentifierNameInto(name, change.getName());
if (is_default)
default_settings = change.name;
default_settings = change.getName();
else
change.value = value->as<ASTLiteral &>().value;
change.setFieldValue(value->as<ASTLiteral &>().value);
return true;
}

View File

@ -5,7 +5,7 @@
#include <Storages/StorageFactory.h>
#include <Storages/StorageMergeTree.h>
#include <Storages/StorageReplicatedMergeTree.h>
#include <Storages/getOrCreateDiskFromSettings.h>
#include <Storages/createDiskFromDiskAST.h>
#include <Common/Macros.h>
#include <Common/OptimizedRegularExpression.h>
@ -24,7 +24,6 @@
#include <Interpreters/Context.h>
#include <Interpreters/FunctionNameNormalizer.h>
#include <Interpreters/evaluateConstantExpression.h>
#include <ranges>
namespace DB
@ -608,26 +607,21 @@ static StoragePtr create(const StorageFactory::Arguments & args)
metadata.column_ttls_by_name[name] = new_ttl_entry;
}
storage_settings->loadFromQuery(*args.storage_def);
/**
* If settings contain disk setting as disk=setting(<disk_configuration>),
* create a disk from this configuration and substitute disk setting with result disk name.
*/
Field disk_value;
if (storage_settings->tryGet("disk", disk_value))
if (args.storage_def->settings)
{
auto disk_settings = disk_value.safeGet<Array>() | std::views::transform([](const Field & field)
for (auto & [_, value, value_ast] : args.storage_def->settings->changes)
{
auto setting = field.safeGet<Tuple>();
return SettingChange(setting[0].safeGet<String>(), setting[1]);
});
auto changes = SettingsChanges(disk_settings.begin(), disk_settings.end());
auto disk_name = getOrCreateDiskFromSettings(changes, context);
storage_settings->set("disk", disk_name);
if (isDiskFunction(value_ast))
{
value = createDiskFromDiskAST(*value_ast->as<ASTFunction>(), context);
break;
}
}
}
/// Updates the default storage_settings with settings specified via SETTINGS arg in a query
storage_settings->loadFromQuery(*args.storage_def);
// updates the default storage_settings with settings specified via SETTINGS arg in a query
if (args.storage_def->settings)
metadata.settings_changes = args.storage_def->settings->ptr();
}

View File

@ -0,0 +1,74 @@
#include <Storages/createDiskFromDiskAST.h>
#include <Common/logger_useful.h>
#include <Common/assert_cast.h>
#include <Common/filesystemHelpers.h>
#include <Disks/getDiskConfigurationFromAST.h>
#include <Disks/DiskSelector.h>
#include <Parsers/formatAST.h>
#include <Parsers/ASTExpressionList.h>
#include <Parsers/ASTFunction.h>
#include <Interpreters/Context.h>
namespace DB
{
namespace ErrorCodes
{
extern const int BAD_ARGUMENTS;
}
bool isDiskFunction(ASTPtr ast)
{
if (!ast)
return false;
const auto * function = ast->as<ASTFunction>();
return function && function->name == "disk" && function->arguments->as<ASTExpressionList>();
}
std::string createDiskFromDiskAST(const ASTFunction & function, ContextPtr context)
{
/// We need a unique name for a created custom disk, but it needs to be the same
/// after table is reattached or server is restarted, so take a hash of the disk
/// configuration serialized ast as a disk name suffix.
auto disk_setting_string = serializeAST(function, true);
auto disk_name = DiskSelector::TMP_DISK_PREFIX
+ toString(sipHash128(disk_setting_string.data(), disk_setting_string.size()));
LOG_TRACE(
&Poco::Logger::get("createDiskFromDiskAST"),
"Using disk name `{}` for custom disk {}",
disk_name, disk_setting_string);
auto result_disk = context->getOrCreateDisk(disk_name, [&](const DisksMap & disks_map) -> DiskPtr {
const auto * function_args_expr = assert_cast<const ASTExpressionList *>(function.arguments.get());
const auto & function_args = function_args_expr->children;
auto config = getDiskConfigurationFromAST(disk_name, function_args, context);
auto disk = DiskFactory::instance().create(disk_name, *config, disk_name, context, disks_map);
/// Mark that disk can be used without storage policy.
disk->markDiskAsCustom();
return disk;
});
if (!result_disk->isRemote())
{
static constexpr auto custom_disks_base_dir_in_config = "custom_local_disks_base_directory";
auto disk_path_expected_prefix = context->getConfigRef().getString(custom_disks_base_dir_in_config, "");
if (disk_path_expected_prefix.empty())
throw Exception(
ErrorCodes::BAD_ARGUMENTS,
"Base path for custom local disks must be defined in config file by `{}`",
custom_disks_base_dir_in_config);
if (!pathStartsWith(result_disk->getPath(), disk_path_expected_prefix))
throw Exception(
ErrorCodes::BAD_ARGUMENTS,
"Path of the custom local disk must be inside `{}` directory",
disk_path_expected_prefix);
}
return disk_name;
}
}

View File

@ -0,0 +1,23 @@
#pragma once
#include <string>
#include <Interpreters/Context_fwd.h>
#include <Parsers/IAST_fwd.h>
namespace DB
{
class ASTFunction;
/**
* Create a DiskPtr from disk AST function like disk(<disk_configuration>),
* add it to DiskSelector by a unique (but always the same for given configuration) disk name
* and return this name.
*/
std::string createDiskFromDiskAST(const ASTFunction & function, ContextPtr context);
/*
* Is given ast has form of a disk(<disk_configuration>) function.
*/
bool isDiskFunction(ASTPtr ast);
}

View File

@ -1,112 +0,0 @@
#include <Storages/getOrCreateDiskFromSettings.h>
#include <Common/logger_useful.h>
#include <Common/FieldVisitorToString.h>
#include <Common/filesystemHelpers.h>
#include <Disks/DiskSelector.h>
#include <Interpreters/Context.h>
#include <Poco/Util/XMLConfiguration.h>
#include <Poco/DOM/Document.h>
#include <Poco/DOM/Element.h>
#include <Poco/DOM/Text.h>
#include <ranges>
namespace DB
{
namespace ErrorCodes
{
extern const int BAD_ARGUMENTS;
}
namespace
{
using DiskSettings = std::vector<std::pair<std::string, std::string>>;
Poco::AutoPtr<Poco::XML::Document> getDiskConfigurationFromSettingsImpl(const std::string & root_name, const DiskSettings & settings)
{
Poco::AutoPtr<Poco::XML::Document> xml_document(new Poco::XML::Document());
Poco::AutoPtr<Poco::XML::Element> root(xml_document->createElement("disk"));
xml_document->appendChild(root);
Poco::AutoPtr<Poco::XML::Element> disk_configuration(xml_document->createElement(root_name));
root->appendChild(disk_configuration);
for (const auto & [name, value] : settings)
{
Poco::AutoPtr<Poco::XML::Element> key_element(xml_document->createElement(name));
disk_configuration->appendChild(key_element);
Poco::AutoPtr<Poco::XML::Text> value_element(xml_document->createTextNode(value));
key_element->appendChild(value_element);
}
return xml_document;
}
DiskConfigurationPtr getDiskConfigurationFromSettings(const std::string & root_name, const DiskSettings & settings)
{
auto xml_document = getDiskConfigurationFromSettingsImpl(root_name, settings);
Poco::AutoPtr<Poco::Util::XMLConfiguration> conf(new Poco::Util::XMLConfiguration());
conf->load(xml_document);
return conf;
}
}
std::string getOrCreateDiskFromSettings(const SettingsChanges & configuration, ContextPtr context)
{
/// We need a unique name for a created custom disk, but it needs to be the same
/// after table is reattached or server is restarted, so take a hash of the disk
/// configuration serialized ast as a disk name suffix.
auto settings_range = configuration
| std::views::transform(
[](const auto & setting) { return std::pair(setting.name, convertFieldToString(setting.value)); });
DiskSettings sorted_settings(settings_range.begin(), settings_range.end());
std::sort(sorted_settings.begin(), sorted_settings.end());
std::string disk_setting_string;
for (auto it = sorted_settings.begin(); it != sorted_settings.end(); ++it)
{
if (it != sorted_settings.begin())
disk_setting_string += ",";
disk_setting_string += std::get<0>(*it) + '=' + std::get<1>(*it);
}
auto disk_name = DiskSelector::TMP_DISK_PREFIX
+ toString(sipHash128(disk_setting_string.data(), disk_setting_string.size()));
LOG_TRACE(
&Poco::Logger::get("getOrCreateDiskFromSettings"),
"Using disk name `{}` for custom disk {}",
disk_name, disk_setting_string);
auto result_disk = context->getOrCreateDisk(disk_name, [&](const DisksMap & disks_map) -> DiskPtr {
auto config = getDiskConfigurationFromSettings(disk_name, sorted_settings);
auto disk = DiskFactory::instance().create(disk_name, *config, disk_name, context, disks_map);
/// Mark that disk can be used without storage policy.
disk->markDiskAsCustom();
return disk;
});
if (!result_disk->isRemote())
{
static constexpr auto custom_disks_base_dir_in_config = "custom_local_disks_base_directory";
auto disk_path_expected_prefix = context->getConfigRef().getString(custom_disks_base_dir_in_config, "");
if (disk_path_expected_prefix.empty())
throw Exception(
ErrorCodes::BAD_ARGUMENTS,
"Base path for custom local disks must be defined in config file by `{}`",
custom_disks_base_dir_in_config);
if (!pathStartsWith(result_disk->getPath(), disk_path_expected_prefix))
throw Exception(
ErrorCodes::BAD_ARGUMENTS,
"Path of the custom local disk must be inside `{}` directory",
disk_path_expected_prefix);
}
return disk_name;
}
}

View File

@ -1,21 +0,0 @@
#pragma once
#include <Interpreters/Context_fwd.h>
#include <Poco/Util/AbstractConfiguration.h>
#include <Poco/DOM/AutoPtr.h>
namespace DB
{
class SettingsChanges;
using DiskConfigurationPtr = Poco::AutoPtr<Poco::Util::AbstractConfiguration>;
/**
* Create a DiskPtr from key value list of disk settings.
* add it to DiskSelector by a unique (but always the same for given configuration) disk name
* and return this name.
*/
std::string getOrCreateDiskFromSettings(const SettingsChanges & configuration, ContextPtr context);
}