Relax the sort requirement in columnstore.
Project: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/commit/ffb8e055 Tree: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/tree/ffb8e055 Diff: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/diff/ffb8e055 Branch: refs/heads/trace Commit: ffb8e055a890a9002235a8516e5f2dece2d6228a Parents: 69fd94b Author: Zuyu Zhang <[email protected]> Authored: Mon Oct 9 11:23:08 2017 -0500 Committer: Zuyu Zhang <[email protected]> Committed: Tue Oct 10 14:10:03 2017 -0500 ---------------------------------------------------------------------- parser/CMakeLists.txt | 1 + parser/ParseBlockProperties.hpp | 10 ++++-- parser/tests/Create.test | 45 +++++++++++++++++++++++++ query_optimizer/OptimizerTree.hpp | 9 +++-- query_optimizer/resolver/Resolver.cpp | 15 +++++---- query_optimizer/tests/resolver/Create.test | 38 +++++++++++++++++---- 6 files changed, 99 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/ffb8e055/parser/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/parser/CMakeLists.txt b/parser/CMakeLists.txt index b3ddf30..d4aaab4 100644 --- a/parser/CMakeLists.txt +++ b/parser/CMakeLists.txt @@ -150,6 +150,7 @@ target_link_libraries(quickstep_parser_ParseBlockProperties quickstep_parser_ParseTreeNode quickstep_utility_Macros quickstep_utility_PtrList + quickstep_utility_SqlError quickstep_utility_StringUtil) target_link_libraries(quickstep_parser_ParseCaseExpressions quickstep_parser_ParseExpression http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/ffb8e055/parser/ParseBlockProperties.hpp ---------------------------------------------------------------------- diff --git a/parser/ParseBlockProperties.hpp b/parser/ParseBlockProperties.hpp index ce0cd92..fa176b1 100644 --- a/parser/ParseBlockProperties.hpp +++ b/parser/ParseBlockProperties.hpp @@ -31,6 +31,7 @@ #include "parser/ParseTreeNode.hpp" #include "utility/Macros.hpp" #include "utility/PtrList.hpp" +#include "utility/SqlError.hpp" #include "utility/StringUtil.hpp" #include "glog/logging.h" @@ -143,10 +144,13 @@ class ParseBlockProperties : public ParseTreeNode { if (sort_key_value == nullptr) { return nullptr; } - if (sort_key_value->getKeyValueType() != - ParseKeyValue::KeyValueType::kStringString) { - return nullptr; + if (sort_key_value->getKeyValueType() == + ParseKeyValue::KeyValueType::kStringStringList) { + THROW_SQL_ERROR_AT(sort_key_value) + << "The SORT property must be a string, not a string list."; } + + DCHECK(sort_key_value->getKeyValueType() == ParseKeyValue::KeyValueType::kStringString); return static_cast<const ParseKeyStringValue*>(sort_key_value)->value(); } http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/ffb8e055/parser/tests/Create.test ---------------------------------------------------------------------- diff --git a/parser/tests/Create.test b/parser/tests/Create.test index 8c11054..3923c13 100644 --- a/parser/tests/Create.test +++ b/parser/tests/Create.test @@ -259,6 +259,51 @@ CreateTableStatement[relation_name=test] +-value=String[value=rowstore] == +CREATE TABLE test (attr INT, attr2 INT) WITH BLOCKPROPERTIES +(TYPE columnstore) +-- +CreateTableStatement[relation_name=test] ++-attribute_list= +| +-AttributeDefinition[name=attr,type=Int] +| +-AttributeDefinition[name=attr2,type=Int] ++-block_properties= + +-BlockProperties + +-block_property=KeyStringValue[key=TYPE] + +-value=String[value=columnstore] +== + +CREATE TABLE test (attr INT, attr2 INT) WITH BLOCKPROPERTIES +(TYPE columnstore, SORT attr) +-- +CreateTableStatement[relation_name=test] ++-attribute_list= +| +-AttributeDefinition[name=attr,type=Int] +| +-AttributeDefinition[name=attr2,type=Int] ++-block_properties= + +-BlockProperties + +-block_property=KeyStringValue[key=TYPE] + | +-value=String[value=columnstore] + +-block_property=KeyStringValue[key=SORT] + +-value=String[value=attr] +== + +CREATE TABLE test (attr INT, attr2 INT) WITH BLOCKPROPERTIES +(TYPE columnstore, SORT (attr, attr2)) +-- +CreateTableStatement[relation_name=test] ++-attribute_list= +| +-AttributeDefinition[name=attr,type=Int] +| +-AttributeDefinition[name=attr2,type=Int] ++-block_properties= + +-BlockProperties + +-block_property=KeyStringValue[key=TYPE] + | +-value=String[value=columnstore] + +-block_property=KeyStringList[key=SORT] + +-value_list= + +-String[value=attr] + +-String[value=attr2] +== + CREATE TABLE test (attr INT) WITH BLOCKPROPERTIES (TYPE compressed_columnstore, SORT attr, COMPRESS ALL) -- http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/ffb8e055/query_optimizer/OptimizerTree.hpp ---------------------------------------------------------------------- diff --git a/query_optimizer/OptimizerTree.hpp b/query_optimizer/OptimizerTree.hpp index c54ce20..0f4713e 100644 --- a/query_optimizer/OptimizerTree.hpp +++ b/query_optimizer/OptimizerTree.hpp @@ -240,9 +240,12 @@ OptimizerProtoRepresentation<TreeNodeType>* getOptimizerRepresentationForProto( } case TupleStorageSubBlockDescription::BASIC_COLUMN_STORE: { node->addProperty("blocktype", "columnstore"); - node->addProperty("sort", - storage_block_description.GetExtension( - quickstep::BasicColumnStoreTupleStorageSubBlockDescription::sort_attribute_id)); + if (storage_block_description.HasExtension( + quickstep::BasicColumnStoreTupleStorageSubBlockDescription::sort_attribute_id)) { + node->addProperty("sort", + storage_block_description.GetExtension( + quickstep::BasicColumnStoreTupleStorageSubBlockDescription::sort_attribute_id)); + } break; } case TupleStorageSubBlockDescription::COMPRESSED_COLUMN_STORE: { http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/ffb8e055/query_optimizer/resolver/Resolver.cpp ---------------------------------------------------------------------- diff --git a/query_optimizer/resolver/Resolver.cpp b/query_optimizer/resolver/Resolver.cpp index 935e235..2991568 100644 --- a/query_optimizer/resolver/Resolver.cpp +++ b/query_optimizer/resolver/Resolver.cpp @@ -687,7 +687,7 @@ StorageBlockLayoutDescription* Resolver::resolveBlockProperties( // Resolve TYPE property. // The type of the block will determine these: - bool block_requires_sort = false; + bool block_allows_sort = false; bool block_requires_compress = false; const ParseString *type_parse_string = block_properties->getType(); @@ -702,7 +702,8 @@ StorageBlockLayoutDescription* Resolver::resolveBlockProperties( } else if (type_string.compare("columnstore") == 0) { description->set_sub_block_type( quickstep::TupleStorageSubBlockDescription::BASIC_COLUMN_STORE); - block_requires_sort = true; + // NOTE(zuyu): sort is optional. + block_allows_sort = true; } else if (type_string.compare("compressed_rowstore") == 0) { description->set_sub_block_type( quickstep::TupleStorageSubBlockDescription::COMPRESSED_PACKED_ROW_STORE); @@ -710,7 +711,7 @@ StorageBlockLayoutDescription* Resolver::resolveBlockProperties( } else if (type_string.compare("compressed_columnstore") == 0) { description->set_sub_block_type( quickstep::TupleStorageSubBlockDescription::COMPRESSED_COLUMN_STORE); - block_requires_sort = true; + block_allows_sort = true; block_requires_compress = true; } else { THROW_SQL_ERROR_AT(type_parse_string) << "Unrecognized storage type."; @@ -718,10 +719,12 @@ StorageBlockLayoutDescription* Resolver::resolveBlockProperties( // Resolve the SORT property. const ParseString *sort_parse_string = block_properties->getSort(); - if (block_requires_sort) { + if (block_allows_sort) { if (sort_parse_string == nullptr) { - THROW_SQL_ERROR_AT(type_parse_string) - << "The SORT property must be specified as an attribute name."; + if (description->sub_block_type() != TupleStorageSubBlockDescription::BASIC_COLUMN_STORE) { + THROW_SQL_ERROR_AT(type_parse_string) + << "The SORT property must be specified as an attribute name."; + } } else { // Lookup the name and map to a column id. const attribute_id sort_id = GetAttributeIdFromName(create_table_statement.attribute_definition_list(), http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/ffb8e055/query_optimizer/tests/resolver/Create.test ---------------------------------------------------------------------- diff --git a/query_optimizer/tests/resolver/Create.test b/query_optimizer/tests/resolver/Create.test index c216c85..ed9158a 100644 --- a/query_optimizer/tests/resolver/Create.test +++ b/query_optimizer/tests/resolver/Create.test @@ -133,13 +133,37 @@ ERROR: The SORT property does not apply to this block type. (2 : 28) ^ == -# Columnstores require a sort attribute. +# Columnstores do not require a sort attribute. CREATE TABLE foo (attr INT) WITH BLOCKPROPERTIES (TYPE columnstore); -- -ERROR: The SORT property must be specified as an attribute name. (2 : 7) -(TYPE columnstore); - ^ +TopLevelPlan ++-plan=CreateTable[relation=foo] +| +-block_properties=ProtoDescription +| | +-Property=ProtoProperty[Property=blocktype,Value=columnstore] +| | +-Property=ProtoProperty[Property=slots,Value=1] +| +-attributes= +| +-AttributeReference[id=0,name=attr,relation=foo,type=Int] ++-output_attributes= + +-AttributeReference[id=0,name=attr,relation=foo,type=Int] +== + +# Columnstores have a optional sort attribute. +CREATE TABLE foo (attr INT, attr2 INT) WITH BLOCKPROPERTIES +(TYPE columnstore, SORT attr2); +-- +TopLevelPlan ++-plan=CreateTable[relation=foo] +| +-block_properties=ProtoDescription +| | +-Property=ProtoProperty[Property=blocktype,Value=columnstore] +| | +-Property=ProtoProperty[Property=sort,Value=1] +| | +-Property=ProtoProperty[Property=slots,Value=1] +| +-attributes= +| +-AttributeReference[id=0,name=attr,relation=foo,type=Int] +| +-AttributeReference[id=1,name=attr2,relation=foo,type=Int] ++-output_attributes= + +-AttributeReference[id=0,name=attr,relation=foo,type=Int] + +-AttributeReference[id=1,name=attr2,relation=foo,type=Int] == # Non-existant columns should be caught. @@ -155,9 +179,9 @@ ERROR: The SORT property did not match any attribute name. (2 : 25) CREATE TABLE foo (attr INT, attr2 INT) WITH BLOCKPROPERTIES (TYPE columnstore, SORT (attr, attr2)); -- -ERROR: The SORT property must be specified as an attribute name. (2 : 7) -(TYPE columnstore, SORT (attr, attr2... - ^ +ERROR: The SORT property must be a string, not a string list. (2 : 20) +(TYPE columnstore, SORT (attr, attr2)); + ^ == # Compress should only be specified on compressed blocks. CREATE TABLE foo (attr INT) WITH BLOCKPROPERTIES
