Relax the sort requirement in columnstore.
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 @@
quickstep_parser_ParseTreeNode
quickstep_utility_Macros
quickstep_utility_PtrList
+ quickstep_utility_SqlError
quickstep_utility_StringUtil)
target_link_libraries(quickstep_parser_ParseCaseExpressions
quickstep_parser_ParseExpression
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 @@
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();
}
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 @@
+-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)
--
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 @@
}
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: {
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 @@
// 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 @@
} 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 @@
} 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 @@
// 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(),
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 @@
^
==
-# 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 @@
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
