You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@quickstep.apache.org by zu...@apache.org on 2017/10/10 19:13:15 UTC
incubator-quickstep git commit: Relax the sort requirement in
columnstore.
Repository: incubator-quickstep
Updated Branches:
refs/heads/master 69fd94b89 -> ffb8e055a
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/master
Commit: ffb8e055a890a9002235a8516e5f2dece2d6228a
Parents: 69fd94b
Author: Zuyu Zhang <zu...@cs.wisc.edu>
Authored: Mon Oct 9 11:23:08 2017 -0500
Committer: Zuyu Zhang <zu...@cs.wisc.edu>
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