This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push: new 345fd44ca KUDU-1945 Auto-Incrementing Column, C++ client 345fd44ca is described below commit 345fd44ca3b0ecd6e80b4e554cac0d8cb230ccd7 Author: Marton Greber <greber...@gmail.com> AuthorDate: Thu Jan 12 17:27:39 2023 +0100 KUDU-1945 Auto-Incrementing Column, C++ client This patch adds the initial part of the cpp client side changes to the auto incrementing column feature. A new KuduColumnSpec called NonUniquePrimaryKey is added. Semantically it behaves like PrimaryKey: - only one column can have the NonUniquePrimaryKey ColumnSpec in a given KuduSchemaBuilder context, - if it exists, it must be defined in the first place, - compound keys are defined through a set function. Functionally non-unique primary keys don't need to fulfill the uniqueness constraint. An auto incrementing column is added in the background automatically once a non-unique primary key is specified. The non-unique keys and the auto incrementing columns together form the effective primary key. Some technical notes: - The name of the auto incrementing column is hardcoded into the Schema class. This is a reserved column name, users can't create columns with it. On the client facing side, this reserved string is reachable through: KuduSchema::GetAutoIncrementingColumnName(). - In this initial version there is no support for UPSERT and UPSERT_IGNORE operations. As suggested in the server side patch [1], a specific builder is added to construct the column spec for the auto incrementing column. Since this pins down the properties of the column, I took the liberty to remove unit tests and checks which verify these properties. [1] https://gerrit.cloudera.org/#/c/19097/ Change-Id: Ic133e3d44cc56c8351e33d95b523ed7b6b13617b Reviewed-on: http://gerrit.cloudera.org:8080/19272 Reviewed-by: Alexey Serbin <ale...@apache.org> Reviewed-by: Abhishek Chennaka <achenn...@cloudera.com> Tested-by: Alexey Serbin <ale...@apache.org> --- src/kudu/client/client-test.cc | 172 +++++++--- src/kudu/client/client-unittest.cc | 376 +++++++++++++++++---- src/kudu/client/client.h | 1 + src/kudu/client/scan_token-internal.cc | 3 + src/kudu/client/scan_token-test.cc | 92 +++++ src/kudu/client/schema-internal.h | 2 + src/kudu/client/schema.cc | 111 ++++-- src/kudu/client/schema.h | 68 +++- src/kudu/client/session-internal.cc | 32 +- src/kudu/client/table_alterer-internal.cc | 23 ++ src/kudu/common/partial_row.cc | 5 + src/kudu/common/partial_row.h | 4 + src/kudu/common/schema-test.cc | 2 +- src/kudu/common/schema.cc | 50 +-- src/kudu/common/schema.h | 7 + .../integration-tests/auto_incrementing-itest.cc | 9 +- 16 files changed, 760 insertions(+), 197 deletions(-) diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc index 9bb45617e..104d2a213 100644 --- a/src/kudu/client/client-test.cc +++ b/src/kudu/client/client-test.cc @@ -9856,11 +9856,7 @@ class ClientTestAutoIncrementingColumn : public ClientTest { TEST_F(ClientTestAutoIncrementingColumn, ReadAndWrite) { const string kTableName = "table_with_auto_incrementing_column"; KuduSchemaBuilder b; - // TODO(Marton): Once the NonUnique column Spec is in place - // update the column specs below to match the implementation - b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); - b.AddColumn("auto_incrementing_id")->Type(KuduColumnSchema::INT64) - ->NotNull()->AutoIncrementing(); + b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->NonUniquePrimaryKey(); ASSERT_OK(b.Build(&schema_)); // Create a table with a couple of range partitions @@ -9920,11 +9916,7 @@ TEST_F(ClientTestAutoIncrementingColumn, ReadAndWrite) { TEST_F(ClientTestAutoIncrementingColumn, ConcurrentWrites) { const string kTableName = "concurrent_writes_auto_incrementing_column"; KuduSchemaBuilder b; - // TODO(Marton): Once the NonUnique column Spec is in place - // update the column specs below to match the implementation - b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); - b.AddColumn("auto_incrementing_id")->Type(KuduColumnSchema::INT64) - ->NotNull()->AutoIncrementing(); + b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->NonUniquePrimaryKey(); ASSERT_OK(b.Build(&schema_)); static constexpr int num_clients = 8; @@ -9995,48 +9987,144 @@ TEST_F(ClientTestAutoIncrementingColumn, ConcurrentWrites) { } } -TEST_F(ClientTestAutoIncrementingColumn, Negatives) { - // TODO(Marton): Once the NonUnique column Spec is in place - // update the column specs below to match the implementation +TEST_F(ClientTestAutoIncrementingColumn, AlterOperationPositives) { + const string kTableName = "alter_operation_positives_auto_incrementing_column"; + KuduSchemaBuilder b; + // Create a schema with non-unique PK, such that auto incrementing col is present. + b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->NonUniquePrimaryKey(); + ASSERT_OK(b.Build(&schema_)); + + unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator()); + ASSERT_OK(table_creator->table_name(kTableName) + .schema(&schema_) + .add_hash_partitions({"key"}, 2) + .num_replicas(3) + .Create()); + { - KuduSchemaBuilder b; - b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); - b.AddColumn("auto_incrementing_id")->Type(KuduColumnSchema::INT32) - ->NotNull()->AutoIncrementing(); - Status s = b.Build(&schema_); - ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString(); - ASSERT_EQ("Invalid argument: auto-incrementing column should be of type INT64", s.ToString()); + unique_ptr<KuduTableAlterer> alterer(client_->NewTableAlterer(kTableName)); + alterer->AlterColumn(Schema::GetAutoIncrementingColumnName())->BlockSize(1); + ASSERT_OK(alterer->Alter()); } { - KuduSchemaBuilder b; - b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); - b.AddColumn("auto_incrementing_id")->Type(KuduColumnSchema::INT64) - ->Nullable()->AutoIncrementing(); - Status s = b.Build(&schema_); - ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString(); - ASSERT_EQ("Invalid argument: auto-incrementing column should not be nullable", s.ToString()); + unique_ptr<KuduTableAlterer> alterer(client_->NewTableAlterer(kTableName)); + alterer->AlterColumn(Schema::GetAutoIncrementingColumnName()) + ->Encoding(KuduColumnStorageAttributes::PLAIN_ENCODING); + ASSERT_OK(alterer->Alter()); } { - KuduSchemaBuilder b; - b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); - b.AddColumn("auto_incrementing_id")->Type(KuduColumnSchema::INT64) - ->NotNull()->Default(KuduValue::FromInt(20))->AutoIncrementing(); - Status s = b.Build(&schema_); - ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString(); - ASSERT_EQ("Invalid argument: auto-incrementing column cannot have a " - "default value", s.ToString()); + unique_ptr<KuduTableAlterer> alterer(client_->NewTableAlterer(kTableName)); + alterer->AlterColumn(Schema::GetAutoIncrementingColumnName()) + ->Compression(KuduColumnStorageAttributes::NO_COMPRESSION); + ASSERT_OK(alterer->Alter()); } { - KuduSchemaBuilder b; - b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); - b.AddColumn("auto_incrementing_id")->Type(KuduColumnSchema::INT64) - ->NotNull()->Immutable()->AutoIncrementing(); - Status s = b.Build(&schema_); - ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString(); - ASSERT_EQ("Invalid argument: auto-incrementing column should not be immutable", s.ToString()); + unique_ptr<KuduTableAlterer> alterer(client_->NewTableAlterer(kTableName)); + alterer->AlterColumn(Schema::GetAutoIncrementingColumnName())->Comment("new_comment"); + ASSERT_OK(alterer->Alter()); + } +} + +TEST_F(ClientTestAutoIncrementingColumn, AlterOperationNegatives) { + const string kTableName = "alter_operation_negatives_auto_incrementing_column"; + KuduSchemaBuilder b; + // Create a schema with non-unique PK, such that auto incrementing col is present. + b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->NonUniquePrimaryKey(); + ASSERT_OK(b.Build(&schema_)); + + unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator()); + ASSERT_OK(table_creator->table_name(kTableName) + .schema(&schema_) + .add_hash_partitions({"key"}, 2) + .num_replicas(3) + .Create()); + + { + unique_ptr<KuduTableAlterer> alterer(client_->NewTableAlterer(kTableName)); + alterer->DropColumn(Schema::GetAutoIncrementingColumnName()); + ASSERT_EQ(Substitute("Invalid argument: can't drop column: $0", + Schema::GetAutoIncrementingColumnName()), + alterer->Alter().ToString()); + } + + { + unique_ptr<KuduTableAlterer> alterer(client_->NewTableAlterer(kTableName)); + alterer->AddColumn(Schema::GetAutoIncrementingColumnName())->Type(KuduColumnSchema::INT64); + ASSERT_EQ(Substitute("Invalid argument: can't add a column with reserved name: $0", + Schema::GetAutoIncrementingColumnName()), + alterer->Alter().ToString()); + } + + { + unique_ptr<KuduTableAlterer> alterer(client_->NewTableAlterer(kTableName)); + alterer->AlterColumn(Schema::GetAutoIncrementingColumnName())->RenameTo("new_name"); + ASSERT_EQ(Substitute("Invalid argument: can't change name for column: $0", + Schema::GetAutoIncrementingColumnName()), + alterer->Alter().ToString()); + } + + { + unique_ptr<KuduTableAlterer> alterer(client_->NewTableAlterer(kTableName)); + alterer->AlterColumn(Schema::GetAutoIncrementingColumnName())->RemoveDefault(); + ASSERT_EQ(Substitute("Invalid argument: can't change remove default for column: $0", + Schema::GetAutoIncrementingColumnName()), + alterer->Alter().ToString()); + } + + { + unique_ptr<KuduTableAlterer> alterer(client_->NewTableAlterer(kTableName)); + alterer->AlterColumn(Schema::GetAutoIncrementingColumnName())->Default(KuduValue::FromInt(1)); + ASSERT_EQ(Substitute("Invalid argument: can't change default value for column: $0", + Schema::GetAutoIncrementingColumnName()), + alterer->Alter().ToString()); + } + + { + unique_ptr<KuduTableAlterer> alterer(client_->NewTableAlterer(kTableName)); + alterer->AlterColumn(Schema::GetAutoIncrementingColumnName())->Immutable(); + ASSERT_EQ(Substitute("Invalid argument: can't change immutability for column: $0", + Schema::GetAutoIncrementingColumnName()), + alterer->Alter().ToString()); + } +} + +TEST_F(ClientTestAutoIncrementingColumn, InsertOperationNegatives) { + const string kTableName = "insert_operation_negatives_auto_incrementing_column"; + KuduSchemaBuilder b; + // Create a schema with non-unique PK, such that auto incrementing col is present. + b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->NonUniquePrimaryKey(); + ASSERT_OK(b.Build(&schema_)); + + unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator()); + ASSERT_OK(table_creator->table_name(kTableName) + .schema(&schema_) + .add_hash_partitions({"key"}, 2) + .num_replicas(3) + .Create()); + + shared_ptr<KuduSession> session(client_->NewSession()); + shared_ptr<KuduTable> table; + client_->OpenTable(kTableName, &table); + + { + unique_ptr<KuduUpsert> op(table->NewUpsert()); + auto* row = op->mutable_row(); + ASSERT_OK(row->SetInt32("key", 1)); + ASSERT_STR_CONTAINS(session->Apply(op.release()).ToString(), + "Illegal state: this type of write operation is not supported on table " + "with auto-incrementing column"); + } + + { + unique_ptr<KuduUpsertIgnore> op(table->NewUpsertIgnore()); + auto* row = op->mutable_row(); + ASSERT_OK(row->SetInt32("key", 1)); + ASSERT_STR_CONTAINS(session->Apply(op.release()).ToString(), + "Illegal state: this type of write operation is not supported on table " + "with auto-incrementing column"); } } } // namespace client diff --git a/src/kudu/client/client-unittest.cc b/src/kudu/client/client-unittest.cc index a5c116e89..d9fb6aa2f 100644 --- a/src/kudu/client/client-unittest.cc +++ b/src/kudu/client/client-unittest.cc @@ -40,10 +40,10 @@ #include "kudu/util/status.h" #include "kudu/util/test_macros.h" +using kudu::client::internal::ErrorCollector; using std::string; using std::vector; using strings::Substitute; -using kudu::client::internal::ErrorCollector; namespace kudu { namespace client { @@ -51,7 +51,7 @@ namespace client { TEST(ClientUnitTest, TestSchemaBuilder_EmptySchema) { KuduSchema s; KuduSchemaBuilder b; - ASSERT_EQ("Invalid argument: no primary key specified", + ASSERT_EQ("Invalid argument: no primary key or non-unique primary key specified", b.Build(&s).ToString()); } @@ -60,7 +60,7 @@ TEST(ClientUnitTest, TestSchemaBuilder_KeyNotSpecified) { KuduSchemaBuilder b; b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull(); b.AddColumn("b")->Type(KuduColumnSchema::INT32)->NotNull(); - ASSERT_EQ("Invalid argument: no primary key specified", + ASSERT_EQ("Invalid argument: no primary key or non-unique primary key specified", b.Build(&s).ToString()); } @@ -70,16 +70,43 @@ TEST(ClientUnitTest, TestSchemaBuilder_DuplicateColumn) { b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); b.AddColumn("x")->Type(KuduColumnSchema::INT32); b.AddColumn("x")->Type(KuduColumnSchema::INT32); - ASSERT_EQ("Invalid argument: Duplicate column name: x", - b.Build(&s).ToString()); + ASSERT_EQ("Invalid argument: Duplicate column name: x", b.Build(&s).ToString()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_KeyAndNonUniqueKeyOnSameColumn) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey()->NonUniquePrimaryKey(); + ASSERT_OK(b.Build(&s)); + ASSERT_EQ(2, s.num_columns()); + ASSERT_EQ(1, s.GetAutoIncrementingColumnIndex()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_NonUniqueKeyAndKeyOnSameColumn) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->NonUniquePrimaryKey()->PrimaryKey(); + ASSERT_OK(b.Build(&s)); + ASSERT_EQ(1, s.num_columns()); + ASSERT_EQ(-1, s.GetAutoIncrementingColumnIndex()); } TEST(ClientUnitTest, TestSchemaBuilder_KeyNotFirstColumn) { KuduSchema s; KuduSchemaBuilder b; b.AddColumn("key")->Type(KuduColumnSchema::INT32); - b.AddColumn("x")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); - b.AddColumn("x")->Type(KuduColumnSchema::INT32); + b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); + b.AddColumn("b")->Type(KuduColumnSchema::INT32); + ASSERT_EQ("Invalid argument: primary key column must be the first column", + b.Build(&s).ToString()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_NonUniqueKeyNotFirstColumn) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("key")->Type(KuduColumnSchema::INT32); + b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull()->NonUniquePrimaryKey(); + b.AddColumn("b")->Type(KuduColumnSchema::INT32); ASSERT_EQ("Invalid argument: primary key column must be the first column", b.Build(&s).ToString()); } @@ -93,24 +120,90 @@ TEST(ClientUnitTest, TestSchemaBuilder_TwoPrimaryKeys) { b.Build(&s).ToString()); } -TEST(ClientUnitTest, TestSchemaBuilder_PrimaryKeyOnColumnAndSet) { +TEST(ClientUnitTest, TestSchemaBuilder_PrimaryKeyAndNonUniquePrimaryKey) { KuduSchema s; KuduSchemaBuilder b; b.AddColumn("a")->Type(KuduColumnSchema::INT32)->PrimaryKey(); - b.AddColumn("b")->Type(KuduColumnSchema::INT32); - b.SetPrimaryKey({ "a", "b" }); - ASSERT_EQ("Invalid argument: primary key specified by both " - "SetPrimaryKey() and on a specific column: a", + b.AddColumn("b")->Type(KuduColumnSchema::INT32)->NonUniquePrimaryKey(); + ASSERT_EQ("Invalid argument: multiple columns specified for primary key: a, b", + b.Build(&s).ToString()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_TwoNonUniquePrimaryKeys) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NonUniquePrimaryKey(); + b.AddColumn("b")->Type(KuduColumnSchema::INT32)->NonUniquePrimaryKey(); + ASSERT_EQ("Invalid argument: multiple columns specified for primary key: a, b", b.Build(&s).ToString()); } +TEST(ClientUnitTest, TestSchemaBuilder_PrimaryKeyOnColumnAndSetPrimaryKey) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("a")->Type(KuduColumnSchema::INT32)->PrimaryKey(); + b.AddColumn("b")->Type(KuduColumnSchema::INT32); + b.SetPrimaryKey({"a", "b"}); + ASSERT_EQ( + "Invalid argument: primary key specified by both " + "SetPrimaryKey() and on a specific column: a", + b.Build(&s).ToString()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_PrimaryKeyOnColumnAndSetNonUniquePrimaryKey) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("a")->Type(KuduColumnSchema::INT32)->PrimaryKey(); + b.AddColumn("b")->Type(KuduColumnSchema::INT32); + b.SetNonUniquePrimaryKey({"a", "b"}); + ASSERT_EQ( + "Invalid argument: primary key specified by both " + "SetNonUniquePrimaryKey() and on a specific column: a", + b.Build(&s).ToString()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_NonUniquePrimaryKeyOnColumnAndSetPrimaryKey) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NonUniquePrimaryKey(); + b.AddColumn("b")->Type(KuduColumnSchema::INT32); + b.SetPrimaryKey({"a", "b"}); + ASSERT_EQ( + "Invalid argument: primary key specified by both " + "SetPrimaryKey() and on a specific column: a", + b.Build(&s).ToString()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_NonUniquePrimaryKeyOnColumnAndSetNonUniquePrimaryKey) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NonUniquePrimaryKey(); + b.AddColumn("b")->Type(KuduColumnSchema::INT32); + b.SetNonUniquePrimaryKey({"a", "b"}); + ASSERT_EQ( + "Invalid argument: primary key specified by both " + "SetNonUniquePrimaryKey() and on a specific column: a", + b.Build(&s).ToString()); +} + TEST(ClientUnitTest, TestSchemaBuilder_SingleKey_GoodSchema) { KuduSchema s; KuduSchemaBuilder b; b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); b.AddColumn("b")->Type(KuduColumnSchema::INT32); b.AddColumn("c")->Type(KuduColumnSchema::INT32)->NotNull(); - ASSERT_EQ("OK", b.Build(&s).ToString()); + ASSERT_OK(b.Build(&s)); +} + +TEST(ClientUnitTest, TestSchemaBuilder_SingleNonUniqueKey_GoodSchema) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull()->NonUniquePrimaryKey(); + b.AddColumn("b")->Type(KuduColumnSchema::INT32); + b.AddColumn("c")->Type(KuduColumnSchema::INT32)->NotNull(); + ASSERT_OK(b.Build(&s)); + ASSERT_EQ(4, s.num_columns()); + ASSERT_EQ(1, s.GetAutoIncrementingColumnIndex()); } TEST(ClientUnitTest, TestSchemaBuilder_CompoundKey_GoodSchema) { @@ -118,32 +211,117 @@ TEST(ClientUnitTest, TestSchemaBuilder_CompoundKey_GoodSchema) { KuduSchemaBuilder b; b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull(); b.AddColumn("b")->Type(KuduColumnSchema::INT32)->NotNull(); - b.SetPrimaryKey({ "a", "b" }); - ASSERT_EQ("OK", b.Build(&s).ToString()); + b.SetPrimaryKey({"a", "b"}); + ASSERT_OK(b.Build(&s)); + + vector<int> key_columns; + s.GetPrimaryKeyColumnIndexes(&key_columns); + ASSERT_EQ(vector<int>({0, 1}), key_columns); + ASSERT_EQ(-1, s.GetAutoIncrementingColumnIndex()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_CompoundNonUniqueKey_GoodSchema) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull(); + b.AddColumn("b")->Type(KuduColumnSchema::INT32)->NotNull(); + b.SetNonUniquePrimaryKey({"a", "b"}); + ASSERT_OK(b.Build(&s)); vector<int> key_columns; s.GetPrimaryKeyColumnIndexes(&key_columns); - ASSERT_EQ(vector<int>({ 0, 1 }), key_columns); + ASSERT_EQ(vector<int>({0, 1, 2}), key_columns); + ASSERT_EQ(2, s.GetAutoIncrementingColumnIndex()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_UniquePrimaryAndNonUniquePrimaryCompound) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull(); + b.AddColumn("b")->Type(KuduColumnSchema::INT32)->NotNull(); + b.SetPrimaryKey({"a", "b"}); + b.SetNonUniquePrimaryKey({"a", "b"}); + ASSERT_OK(b.Build(&s)); + + vector<int> key_columns; + s.GetPrimaryKeyColumnIndexes(&key_columns); + ASSERT_EQ(vector<int>({0, 1, 2}), key_columns); + ASSERT_EQ(2, s.GetAutoIncrementingColumnIndex()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_NonUniqueAndUniquePrimaryCompound) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull(); + b.AddColumn("b")->Type(KuduColumnSchema::INT32)->NotNull(); + b.SetNonUniquePrimaryKey({"a", "b"}); + b.SetPrimaryKey({"a", "b"}); + ASSERT_OK(b.Build(&s)); + ASSERT_EQ(-1, s.GetAutoIncrementingColumnIndex()); + + vector<int> key_columns; + s.GetPrimaryKeyColumnIndexes(&key_columns); + ASSERT_EQ(vector<int>({0, 1}), key_columns); + ASSERT_EQ(-1, s.GetAutoIncrementingColumnIndex()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_KeyColumnWithAutoIncrementingReservedName) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn(Schema::GetAutoIncrementingColumnName()) + ->Type(KuduColumnSchema::INT64) + ->NotNull() + ->PrimaryKey(); + ASSERT_EQ(Substitute("Invalid argument: $0 is a reserved column name", + Schema::GetAutoIncrementingColumnName()), + b.Build(&s).ToString()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_NonUniqueKeyColumnWithAutoIncrementingReservedName) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn(Schema::GetAutoIncrementingColumnName()) + ->Type(KuduColumnSchema::INT64) + ->NotNull() + ->NonUniquePrimaryKey(); + ASSERT_EQ(Substitute("Invalid argument: $0 is a reserved column name", + Schema::GetAutoIncrementingColumnName()), + b.Build(&s).ToString()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_ColumnWithAutoIncrementingReservedName) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); + b.AddColumn(Schema::GetAutoIncrementingColumnName()) + ->Type(KuduColumnSchema::INT64) + ->NotNull(); + ASSERT_EQ(Substitute("Invalid argument: $0 is a reserved column name", + Schema::GetAutoIncrementingColumnName()), + b.Build(&s).ToString()); } TEST(ClientUnitTest, TestSchemaBuilder_DefaultValues) { KuduSchema s; KuduSchemaBuilder b; b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); - b.AddColumn("b")->Type(KuduColumnSchema::INT32)->NotNull() - ->Default(KuduValue::FromInt(12345)); - ASSERT_EQ("OK", b.Build(&s).ToString()); + b.AddColumn("b")->Type(KuduColumnSchema::INT32)->NotNull()->Default(KuduValue::FromInt(12345)); + ASSERT_OK(b.Build(&s)); } TEST(ClientUnitTest, TestSchemaBuilder_DefaultValueString) { KuduSchema s; KuduSchemaBuilder b; b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); - b.AddColumn("b")->Type(KuduColumnSchema::STRING)->NotNull() - ->Default(KuduValue::CopyString("abc")); - b.AddColumn("c")->Type(KuduColumnSchema::BINARY)->NotNull() - ->Default(KuduValue::CopyString("def")); - ASSERT_EQ("OK", b.Build(&s).ToString()); + b.AddColumn("b") + ->Type(KuduColumnSchema::STRING) + ->NotNull() + ->Default(KuduValue::CopyString("abc")); + b.AddColumn("c") + ->Type(KuduColumnSchema::BINARY) + ->NotNull() + ->Default(KuduValue::CopyString("def")); + ASSERT_OK(b.Build(&s)); } TEST(ClientUnitTest, TestSchemaBuilder_CompoundKey_KeyNotFirst) { @@ -152,9 +330,21 @@ TEST(ClientUnitTest, TestSchemaBuilder_CompoundKey_KeyNotFirst) { b.AddColumn("x")->Type(KuduColumnSchema::INT32)->NotNull(); b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull(); b.AddColumn("b")->Type(KuduColumnSchema::INT32)->NotNull(); - b.SetPrimaryKey({ "a", "b" }); - ASSERT_EQ("Invalid argument: primary key columns must be listed " - "first in the schema: a", + b.SetPrimaryKey({"a", "b"}); + ASSERT_EQ( + "Invalid argument: primary key columns must be listed " + "first in the schema: a", + b.Build(&s).ToString()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_CompoundNonUniqueKey_NonUniqueKeyNotFirst) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("x")->Type(KuduColumnSchema::INT32)->NotNull(); + b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull(); + b.AddColumn("b")->Type(KuduColumnSchema::INT32)->NotNull(); + b.SetNonUniquePrimaryKey({"a", "b"}); + ASSERT_EQ("Invalid argument: primary key columns must be listed first in the schema: a", b.Build(&s).ToString()); } @@ -163,9 +353,17 @@ TEST(ClientUnitTest, TestSchemaBuilder_CompoundKey_BadColumnName) { KuduSchemaBuilder b; b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull(); b.AddColumn("b")->Type(KuduColumnSchema::INT32)->NotNull(); - b.SetPrimaryKey({ "foo" }); - ASSERT_EQ("Invalid argument: primary key column not defined: foo", - b.Build(&s).ToString()); + b.SetPrimaryKey({"foo"}); + ASSERT_EQ("Invalid argument: primary key column not defined: foo", b.Build(&s).ToString()); +} + +TEST(ClientUnitTest, TestSchemaBuilder_CompoundNonUniqueKey_BadColumnName) { + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("a")->Type(KuduColumnSchema::INT32)->NotNull(); + b.AddColumn("b")->Type(KuduColumnSchema::INT32)->NotNull(); + b.SetNonUniquePrimaryKey({"foo"}); + ASSERT_EQ("Invalid argument: primary key column not defined: foo", b.Build(&s).ToString()); } TEST(ClientUnitTest, TestDisableSslFailsIfNotInitialized) { @@ -239,57 +437,87 @@ TEST(ClientUnitTest, TestErrorCollector) { } } -TEST(KuduSchemaTest, TestToString) { +TEST(KuduSchemaTest, TestToString_OneUniquePrimaryKey) { // Test on unique PK. - KuduSchema s1; - KuduSchemaBuilder b1; - b1.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); - b1.AddColumn("int_val")->Type(KuduColumnSchema::INT32)->NotNull(); - b1.AddColumn("string_val")->Type(KuduColumnSchema::STRING)->Nullable(); - b1.AddColumn("non_null_with_default")->Type(KuduColumnSchema::INT32)->NotNull() - ->Default(KuduValue::FromInt(12345)); - ASSERT_OK(b1.Build(&s1)); + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); + b.AddColumn("int_val")->Type(KuduColumnSchema::INT32)->NotNull(); + b.AddColumn("string_val")->Type(KuduColumnSchema::STRING)->Nullable(); + b.AddColumn("non_null_with_default") + ->Type(KuduColumnSchema::INT32) + ->NotNull() + ->Default(KuduValue::FromInt(12345)); + ASSERT_OK(b.Build(&s)); + + string schema_str = + "(\n" + " key INT32 NOT NULL,\n" + " int_val INT32 NOT NULL,\n" + " string_val STRING NULLABLE,\n" + " non_null_with_default INT32 NOT NULL,\n" + " PRIMARY KEY (key)\n" + ")"; + EXPECT_EQ(schema_str, s.ToString()); +} - string schema_str_1 = "(\n" - " key INT32 NOT NULL,\n" - " int_val INT32 NOT NULL,\n" - " string_val STRING NULLABLE,\n" - " non_null_with_default INT32 NOT NULL,\n" - " PRIMARY KEY (key)\n" - ")"; - EXPECT_EQ(schema_str_1, s1.ToString()); +TEST(KuduSchemaTest, TestToString_OneNonUniquePrimaryKey) { + // Test on non-unique PK. + KuduSchema s; + KuduSchemaBuilder b; + b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->NonUniquePrimaryKey(); + b.AddColumn("int_val")->Type(KuduColumnSchema::INT32)->NotNull(); + ASSERT_OK(b.Build(&s)); + + string schema_str = Substitute( + "(\n" + " key INT32 NOT NULL,\n" + " $0 INT64 NOT NULL,\n" + " int_val INT32 NOT NULL,\n" + " PRIMARY KEY (key, $0)\n" + ")", + KuduSchema::GetAutoIncrementingColumnName()); + EXPECT_EQ(schema_str, s.ToString()); +} +TEST(KuduSchemaTest, TestToString_EmptySchem) { // Test empty schema. - KuduSchema s2; - EXPECT_EQ("()", s2.ToString()); + KuduSchema s; + EXPECT_EQ("()", s.ToString()); +} +TEST(KuduSchemaTest, TestToString_CompositePrimaryKey) { // Test on composite PK. // Create a different schema with a multi-column PK. - KuduSchemaBuilder b2; - b2.AddColumn("k1")->Type(KuduColumnSchema::INT32)->NotNull(); - b2.AddColumn("k2")->Type(KuduColumnSchema::UNIXTIME_MICROS)->NotNull(); - b2.AddColumn("k3")->Type(KuduColumnSchema::INT8)->NotNull(); - b2.AddColumn("date_val")->Type(KuduColumnSchema::DATE)->NotNull(); - b2.AddColumn("dec_val")->Type(KuduColumnSchema::DECIMAL)->Nullable()->Precision(9)->Scale(2); - b2.AddColumn("int_val")->Type(KuduColumnSchema::INT32)->NotNull(); - b2.AddColumn("string_val")->Type(KuduColumnSchema::STRING)->Nullable(); - b2.AddColumn("non_null_with_default")->Type(KuduColumnSchema::INT32)->NotNull() - ->Default(KuduValue::FromInt(12345)); - b2.SetPrimaryKey({"k1", "k2", "k3"}); - ASSERT_OK(b2.Build(&s2)); - - string schema_str_2 = "(\n" - " k1 INT32 NOT NULL,\n" - " k2 UNIXTIME_MICROS NOT NULL,\n" - " k3 INT8 NOT NULL,\n" - " date_val DATE NOT NULL,\n" - " dec_val DECIMAL(9, 2) NULLABLE,\n" - " int_val INT32 NOT NULL,\n" - " string_val STRING NULLABLE,\n" - " non_null_with_default INT32 NOT NULL,\n" - " PRIMARY KEY (k1, k2, k3)\n" - ")"; - EXPECT_EQ(schema_str_2, s2.ToString()); + KuduSchemaBuilder b; + KuduSchema s; + b.AddColumn("k1")->Type(KuduColumnSchema::INT32)->NotNull(); + b.AddColumn("k2")->Type(KuduColumnSchema::UNIXTIME_MICROS)->NotNull(); + b.AddColumn("k3")->Type(KuduColumnSchema::INT8)->NotNull(); + b.AddColumn("date_val")->Type(KuduColumnSchema::DATE)->NotNull(); + b.AddColumn("dec_val")->Type(KuduColumnSchema::DECIMAL)->Nullable()->Precision(9)->Scale(2); + b.AddColumn("int_val")->Type(KuduColumnSchema::INT32)->NotNull(); + b.AddColumn("string_val")->Type(KuduColumnSchema::STRING)->Nullable(); + b.AddColumn("non_null_with_default") + ->Type(KuduColumnSchema::INT32) + ->NotNull() + ->Default(KuduValue::FromInt(12345)); + b.SetPrimaryKey({"k1", "k2", "k3"}); + ASSERT_OK(b.Build(&s)); + + string schema_str = + "(\n" + " k1 INT32 NOT NULL,\n" + " k2 UNIXTIME_MICROS NOT NULL,\n" + " k3 INT8 NOT NULL,\n" + " date_val DATE NOT NULL,\n" + " dec_val DECIMAL(9, 2) NULLABLE,\n" + " int_val INT32 NOT NULL,\n" + " string_val STRING NULLABLE,\n" + " non_null_with_default INT32 NOT NULL,\n" + " PRIMARY KEY (k1, k2, k3)\n" + ")"; + EXPECT_EQ(schema_str, s.ToString()); } TEST(KuduSchemaTest, TestEquals) { diff --git a/src/kudu/client/client.h b/src/kudu/client/client.h index 8db69404a..026f75467 100644 --- a/src/kudu/client/client.h +++ b/src/kudu/client/client.h @@ -3156,6 +3156,7 @@ class KUDU_EXPORT KuduScanner { FRIEND_TEST(ClientTest, TestReadAtSnapshotNoTimestampSet); FRIEND_TEST(ConsistencyITest, TestSnapshotScanTimestampReuse); FRIEND_TEST(ScanTokenTest, TestScanTokens); + FRIEND_TEST(ScanTokenTest, TestScanTokens_NonUniquePrimaryKey); // Owned. Data* data_; diff --git a/src/kudu/client/scan_token-internal.cc b/src/kudu/client/scan_token-internal.cc index ff16460d0..17fce1b3b 100644 --- a/src/kudu/client/scan_token-internal.cc +++ b/src/kudu/client/scan_token-internal.cc @@ -202,6 +202,9 @@ Status KuduScanToken::Data::PBIntoScanner(KuduClient* client, unique_ptr<KuduScanner> scan_builder(new KuduScanner(table.get())); + // TODO: for statements like "create table as select *", auto_incrementing_id has to be + // omitted from projected columns. We could provide API to toggle the presence of + // the auto incrementing column. vector<int> column_indexes; if (!message.projected_column_idx().empty()) { for (const int column_idx : message.projected_column_idx()) { diff --git a/src/kudu/client/scan_token-test.cc b/src/kudu/client/scan_token-test.cc index 792faa654..1cf0ccde0 100644 --- a/src/kudu/client/scan_token-test.cc +++ b/src/kudu/client/scan_token-test.cc @@ -47,6 +47,7 @@ #include "kudu/client/write_op.h" #include "kudu/common/common.pb.h" #include "kudu/common/partial_row.h" +#include "kudu/common/schema.h" #include "kudu/common/wire_protocol.pb.h" #include "kudu/gutil/map-util.h" #include "kudu/gutil/ref_counted.h" @@ -335,6 +336,15 @@ class ScanTokenTest : public KuduTest { } } + KuduSchema GetTokenProjectionSchema(const KuduScanToken& token) { + string serialized_token; + CHECK_OK(token.Serialize(&serialized_token)); + KuduScanner* scanner_ptr; + CHECK_OK(KuduScanToken::DeserializeIntoScanner(client_.get(), serialized_token, &scanner_ptr)); + unique_ptr<KuduScanner> scanner(scanner_ptr); + return scanner->GetProjectionSchema(); + } + shared_ptr<KuduClient> client_; unique_ptr<InternalMiniCluster> cluster_; }; @@ -534,6 +544,88 @@ TEST_F(ScanTokenTest, TestScanTokens) { } } +TEST_F(ScanTokenTest, TestScanTokens_NonUniquePrimaryKey) { + // Create schema + KuduSchema schema; + { + KuduSchemaBuilder builder; + builder.AddColumn("col")->NotNull()->Type(KuduColumnSchema::INT64)->NonUniquePrimaryKey(); + ASSERT_OK(builder.Build(&schema)); + } + + // Create table + shared_ptr<KuduTable> table; + { + unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator()); + ASSERT_OK(table_creator->table_name("table") + .schema(&schema) + .add_hash_partitions({"col"}, 4) + .num_replicas(1) + .Create()); + ASSERT_OK(client_->OpenTable("table", &table)); + } + + // Create session + shared_ptr<KuduSession> session = client_->NewSession(); + + // Insert rows + for (int i = 0; i < 10; i++) { + unique_ptr<KuduInsert> insert(table->NewInsert()); + ASSERT_OK(insert->mutable_row()->SetInt64("col", i)); + ASSERT_OK(session->Apply(insert.release())); + } + ASSERT_OK(session->Flush()); + + // No projection, testing default behaviour + { + vector<KuduScanToken*> tokens; + ElementDeleter deleter(&tokens); + KuduScanTokenBuilder builder(table.get()); + ASSERT_OK(builder.Build(&tokens)); + ASSERT_EQ(4, tokens.size()); + + for (KuduScanToken* token : tokens) { + KuduSchema s = GetTokenProjectionSchema(*token); + ASSERT_TRUE(s.HasColumn("col", nullptr)); + ASSERT_TRUE(s.HasColumn(Schema::GetAutoIncrementingColumnName(), nullptr)); + } + } + + // Projection including auto incrementing column + { + vector<KuduScanToken*> tokens; + ElementDeleter deleter(&tokens); + KuduScanTokenBuilder builder(table.get()); + vector<string> column_names = { "col", Schema::GetAutoIncrementingColumnName() }; + ASSERT_OK(builder.SetProjectedColumnNames(column_names)); + ASSERT_OK(builder.Build(&tokens)); + ASSERT_EQ(4, tokens.size()); + + for (KuduScanToken* token : tokens) { + KuduSchema s = GetTokenProjectionSchema(*token); + ASSERT_TRUE(s.HasColumn("col", nullptr)); + ASSERT_TRUE(s.HasColumn(Schema::GetAutoIncrementingColumnName(), nullptr)); + } + } + + // Projection excluding auto incrementing column + { + vector<KuduScanToken*> tokens; + ElementDeleter deleter(&tokens); + KuduScanTokenBuilder builder(table.get()); + vector<string> column_names = { "col" }; + ASSERT_OK(builder.SetProjectedColumnNames(column_names)); + ASSERT_OK(builder.Build(&tokens)); + ASSERT_EQ(4, tokens.size()); + + for (KuduScanToken* token : tokens) { + KuduSchema s = GetTokenProjectionSchema(*token); + ASSERT_TRUE(s.HasColumn("col", nullptr)); + ASSERT_FALSE(s.HasColumn(Schema::GetAutoIncrementingColumnName(), nullptr)); + } + } +} + TEST_F(ScanTokenTest, TestScanTokensWithNonCoveringRange) { // Create schema KuduSchema schema; diff --git a/src/kudu/client/schema-internal.h b/src/kudu/client/schema-internal.h index c892569a4..09aa6dca6 100644 --- a/src/kudu/client/schema-internal.h +++ b/src/kudu/client/schema-internal.h @@ -61,6 +61,7 @@ class KuduColumnSpec::Data { explicit Data(std::string name) : name(std::move(name)), primary_key(false), + primary_key_unique(false), auto_incrementing(false), remove_default(false) { } @@ -83,6 +84,7 @@ class KuduColumnSpec::Data { std::optional<bool> nullable; std::optional<bool> immutable; bool primary_key; + bool primary_key_unique; std::optional<KuduValue*> default_val; // Owned. bool auto_incrementing; bool remove_default; // For ALTER diff --git a/src/kudu/client/schema.cc b/src/kudu/client/schema.cc index 81f6888e0..d73b09148 100644 --- a/src/kudu/client/schema.cc +++ b/src/kudu/client/schema.cc @@ -335,11 +335,13 @@ KuduColumnSpec* KuduColumnSpec::Length(uint16_t length) { KuduColumnSpec* KuduColumnSpec::PrimaryKey() { data_->primary_key = true; + data_->primary_key_unique = true; return this; } -KuduColumnSpec* KuduColumnSpec::AutoIncrementing() { - data_->auto_incrementing = true; +KuduColumnSpec* KuduColumnSpec::NonUniquePrimaryKey() { + data_->primary_key = true; + data_->primary_key_unique = false; return this; } @@ -473,27 +475,6 @@ Status KuduColumnSpec::ToColumnSchema(KuduColumnSchema* col) const { bool nullable = data_->nullable ? data_->nullable.value() : true; bool immutable = data_->immutable ? data_->immutable.value() : false; - if (data_->auto_incrementing) { - if (internal_type != kudu::INT64) { - return Status::InvalidArgument("auto-incrementing column should be of type INT64"); - } - if (nullable) { - return Status::InvalidArgument("auto-incrementing column should not be nullable"); - } - if (immutable) { - return Status::InvalidArgument("auto-incrementing column should not be immutable"); - } - if (data_->default_val) { - return Status::InvalidArgument("auto-incrementing column cannot have a " - "default value"); - } - // TODO(Marton): Uncomment once the client patch promotes the - // auto increment column to primary key - //if (!data_->primary_key) { - // return Status::InvalidArgument("auto-incrementing column is not set as primary key " - // "column"); - //} - } void* default_val = nullptr; // TODO(unknown): distinguish between DEFAULT NULL and no default? if (data_->default_val) { @@ -577,7 +558,7 @@ Slice KuduColumnSpec::DefaultValueAsSlice() const { class KuduSchemaBuilder::Data { public: - Data() { + Data() : key_cols_unique(true) { } ~Data() { @@ -586,7 +567,41 @@ class KuduSchemaBuilder::Data { // headers declaring friend classes with nested classes. } + Status AddAutoIncrementingColumn(int* num_key_cols, vector<KuduColumnSchema>* cols) { + KuduAutoIncrementingColumnSpecBuilder b; + DCHECK(specs.begin() + *num_key_cols <= specs.end()); + specs.insert(specs.begin() + *num_key_cols, b.Build()); + DCHECK(cols->begin() + *num_key_cols <= cols->end()); + cols->insert(cols->begin() + *num_key_cols, KuduColumnSchema()); + RETURN_NOT_OK(specs[*num_key_cols]->ToColumnSchema(&cols->at(*num_key_cols))); + + // Make the above inserted auto-incrementing column a key column. + (*num_key_cols)++; + return Status::OK(); + } + + class KuduAutoIncrementingColumnSpecBuilder { + public: + KuduAutoIncrementingColumnSpecBuilder() { + } + + ~KuduAutoIncrementingColumnSpecBuilder() { + } + + KuduColumnSpec* Build() { + auto c = new KuduColumnSpec(name); + c->Type(type)->NotNull(); + c->data_->auto_incrementing = true; + return c; + } + + private: + static constexpr const char* const name = Schema::GetAutoIncrementingColumnName(); + static constexpr KuduColumnSchema::DataType type = KuduColumnSchema::INT64; + }; + std::optional<vector<string>> key_col_names; + bool key_cols_unique; vector<KuduColumnSpec*> specs; }; @@ -610,9 +625,15 @@ KuduColumnSpec* KuduSchemaBuilder::AddColumn(const string& name) { return c; } -KuduSchemaBuilder* KuduSchemaBuilder::SetPrimaryKey( - const vector<string>& key_col_names) { +KuduSchemaBuilder* KuduSchemaBuilder::SetPrimaryKey(const vector<string>& key_col_names) { data_->key_col_names = key_col_names; + data_->key_cols_unique = true; + return this; +} + +KuduSchemaBuilder* KuduSchemaBuilder::SetNonUniquePrimaryKey(const vector<string>& key_col_names) { + data_->key_col_names = key_col_names; + data_->key_cols_unique = false; return this; } @@ -632,17 +653,16 @@ Status KuduSchemaBuilder::Build(KuduSchema* schema) { for (int i = 0; i < cols.size(); i++) { if (data_->specs[i]->data_->primary_key) { if (single_key_col_idx != -1) { - return Status::InvalidArgument("multiple columns specified for primary key", - Substitute("$0, $1", - cols[single_key_col_idx].name(), - cols[i].name())); + return Status::InvalidArgument( + "multiple columns specified for primary key", + Substitute("$0, $1", cols[single_key_col_idx].name(), cols[i].name())); } single_key_col_idx = i; } } if (single_key_col_idx == -1) { - return Status::InvalidArgument("no primary key specified"); + return Status::InvalidArgument("no primary key or non-unique primary key specified"); } // TODO: eventually allow primary keys which aren't the first column @@ -651,6 +671,9 @@ Status KuduSchemaBuilder::Build(KuduSchema* schema) { } num_key_cols = 1; + if (!data_->specs[single_key_col_idx]->data_->primary_key_unique) { + data_->AddAutoIncrementingColumn(&num_key_cols, &cols); + } } else { // Build a map from name to index of all of the columns. unordered_map<string, int> name_to_idx_map; @@ -659,8 +682,17 @@ Status KuduSchemaBuilder::Build(KuduSchema* schema) { // If they did pass the key column names, then we should not have explicitly // set it on any columns. if (spec->data_->primary_key) { - return Status::InvalidArgument("primary key specified by both SetPrimaryKey() and on a " - "specific column", spec->data_->name); + if (data_->key_cols_unique) { + return Status::InvalidArgument( + "primary key specified by both SetPrimaryKey() and on a " + "specific column", + spec->data_->name); + } else { + return Status::InvalidArgument( + "primary key specified by both SetNonUniquePrimaryKey() and on a " + "specific column", + spec->data_->name); + } } // If we have a duplicate column name, the Schema::Reset() will catch it later, // anyway. @@ -676,7 +708,6 @@ Status KuduSchemaBuilder::Build(KuduSchema* schema) { } key_col_indexes.push_back(idx); } - // Currently we require that the key columns be contiguous at the front // of the schema. We'll lift this restriction later -- hence the more // flexible user-facing API. @@ -688,6 +719,10 @@ Status KuduSchemaBuilder::Build(KuduSchema* schema) { } num_key_cols = key_col_indexes.size(); + + if (!data_->key_cols_unique) { + data_->AddAutoIncrementingColumn(&num_key_cols, &cols); + } } #pragma GCC diagnostic push @@ -993,6 +1028,14 @@ void KuduSchema::GetPrimaryKeyColumnIndexes(vector<int>* indexes) const { } } +int KuduSchema::GetAutoIncrementingColumnIndex() const { + return schema_->auto_incrementing_col_idx(); +} + +const char* const KuduSchema::GetAutoIncrementingColumnName() { + return Schema::GetAutoIncrementingColumnName(); +} + string KuduSchema::ToString() const { if (!schema_) { return "()"; diff --git a/src/kudu/client/schema.h b/src/kudu/client/schema.h index 8c15edc00..3ccf3beec 100644 --- a/src/kudu/client/schema.h +++ b/src/kudu/client/schema.h @@ -486,21 +486,33 @@ class KUDU_EXPORT KuduColumnSpec { /// /// @note Primary keys may not be changed after a table is created. /// + /// @note A call to PrimaryKey() or NonUniquePrimaryKey() overrides any previous call + /// to these two methods. + /// /// @return Pointer to the modified object. KuduColumnSpec* PrimaryKey(); ///@{ - /// Set the column to be auto-incrementing. + /// Set the column to be a non-unique primary key of the table. + /// + /// This may only be used to set non-composite non-unique primary keys. If a composite + /// key is desired, use KuduSchemaBuilder::SetNonUniquePrimaryKey(). This may not be + /// used in conjunction with KuduSchemaBuilder::SetNonUniquePrimaryKey(). + /// + /// @note Non-unique primary keys may not be changed after a table is created. /// - /// Upon inserting rows to a table with an auto-incrementing column, values are - /// automatically assigned to field that are unique to the partition. This - /// makes such columns good candidates to include in a table's primary key. + /// @note By specifying non-unique primary key, an auto incrementing column is created + /// automatically. They form together the effective primary key. The auto incrementing field + /// is populated on the server side, it must not be specified during insertion. All subsequent + /// operations like scans will contain the auto incrementing column by default. If one wants to + /// omit the auto incrementing column, it can be accomplished through existing projection + /// methods. /// - /// @note Column auto-incrementing may not be changed once a table is - /// created. + /// @note A call to PrimaryKey() or NonUniquePrimaryKey() overrides any previous call + /// to these two methods. /// /// @return Pointer to the modified object. - KuduColumnSpec* AutoIncrementing(); + KuduColumnSpec* NonUniquePrimaryKey(); /// Set the column to be not nullable. /// @@ -624,11 +636,34 @@ class KUDU_EXPORT KuduSchemaBuilder { /// /// This may be used to specify a compound primary key. /// + /// @note A call to SetPrimaryKey() or SetNonUniquePrimaryKey() overrides any previous + /// call to these two methods. + /// /// @param [in] key_col_names /// Names of the columns to include into the compound primary key. /// @return Pointer to the modified object. KuduSchemaBuilder* SetPrimaryKey(const std::vector<std::string>& key_col_names); + /// Set the non-unique primary key of the new Schema based on the given column names. + /// + /// This may be used to specify a compound non-unique primary key. + /// + /// @note By specifying non-unique primary keys, an auto incrementing column is created + /// automatically. They form together the effective primary key. The auto incrementing field + /// is populated on the server side, it must not be specified during insertion. All subsequent + /// operations like scans will contain the auto incrementing column by default. If one wants to + /// omit the auto incrementing column, it can be accomplished through existing projection + /// methods. + /// + /// @note A call to SetPrimaryKey() or SetNonUniquePrimaryKey() overrides any previous + /// call to these two methods. + /// + /// @param [in] key_col_names + /// Names of the columns to include into the compound non-unique primary key. + /// @return Pointer to the modified object. + KuduSchemaBuilder* SetNonUniquePrimaryKey( + const std::vector<std::string>& key_col_names); + /// Build the schema based on current configuration of the builder object. /// /// @param [out] schema @@ -678,6 +713,9 @@ class KUDU_EXPORT KuduSchema { /// /// @todo Remove KuduSchema::Reset(). /// + /// @note KuduSchema::Reset() is deprecated API. It does not support + /// non-unique primary key and does not add auto incrementing column. + /// /// @param [in] columns /// Per-column schema information. /// @param [in] key_columns @@ -739,6 +777,22 @@ class KUDU_EXPORT KuduSchema { /// The placeholder for the result. void GetPrimaryKeyColumnIndexes(std::vector<int>* indexes) const; + /// Get the index of the auto incrementing column within this Schema. + /// + /// @attention In current versions of Kudu, key column indexes will always be + /// contiguous indexes starting with 0. The auto incrementing column is + /// always the last key column. However, in future versions this assumption + /// may not hold, so callers should not assume it is the case. + /// + /// @return The index of the auto incrementing column. If the column does not + /// exist the return value is -1. + int GetAutoIncrementingColumnIndex() const; + + /// Utility function to return the actual name of the auto incrementing column. + /// + /// @return The name of the auto incrementing column. + static const char* const GetAutoIncrementingColumnName(); + /// Create a new row corresponding to this schema. /// /// @note The new row refers to this KuduSchema object, so it must be diff --git a/src/kudu/client/session-internal.cc b/src/kudu/client/session-internal.cc index aec71592b..88d507b0b 100644 --- a/src/kudu/client/session-internal.cc +++ b/src/kudu/client/session-internal.cc @@ -344,6 +344,14 @@ Status CheckForPrimaryKey(const KuduWriteOperation& op) { return Status::OK(); } +// Check if the non-unique primary key is set for the write operation. +Status CheckForNonUniquePrimaryKey(const KuduWriteOperation& op) { + if (PREDICT_FALSE(!op.row().IsNonUniqueKeySet())) { + return Status::IllegalState("Non-unique key not specified", KUDU_REDACT(op.ToString())); + } + return Status::OK(); +} + // Check if the values for the non-nullable columns are present. Status CheckForNonNullableColumns(const KuduWriteOperation& op) { const auto& row = op.row(); @@ -360,6 +368,16 @@ Status CheckForNonNullableColumns(const KuduWriteOperation& op) { } return Status::OK(); } + +Status CheckForAutoIncrementingColumn(const KuduWriteOperation& op) { + if (op.row().schema()->has_auto_incrementing()) { + return Status::IllegalState( + Substitute( + "this type of write operation is not supported on table with auto-incrementing column"), + KUDU_REDACT(op.ToString())); + } + return Status::OK(); +} } // anonymous namespace #define RETURN_NOT_OK_ADD_ERROR(_func, _op, _error_collector) \ @@ -373,11 +391,23 @@ Status CheckForNonNullableColumns(const KuduWriteOperation& op) { } while (false) Status KuduSession::Data::ValidateWriteOperation(KuduWriteOperation* op) const { - RETURN_NOT_OK_ADD_ERROR(CheckForPrimaryKey, op, error_collector_); + if (op->row().schema()->has_auto_incrementing()) { + RETURN_NOT_OK_ADD_ERROR(CheckForNonUniquePrimaryKey, op, error_collector_); + } else { + RETURN_NOT_OK_ADD_ERROR(CheckForPrimaryKey, op, error_collector_); + } + // TODO(martongreber): UPSERT and UPSERT IGNORE are not supported initially for tables + // with a non-unique primary key. We plan to add this later. switch (op->type()) { case KuduWriteOperation::INSERT: + RETURN_NOT_OK_ADD_ERROR(CheckForNonNullableColumns, op, error_collector_); + break; case KuduWriteOperation::UPSERT: RETURN_NOT_OK_ADD_ERROR(CheckForNonNullableColumns, op, error_collector_); + RETURN_NOT_OK_ADD_ERROR(CheckForAutoIncrementingColumn, op, error_collector_); + break; + case KuduWriteOperation::UPSERT_IGNORE: + RETURN_NOT_OK_ADD_ERROR(CheckForAutoIncrementingColumn, op, error_collector_); break; default: // Nothing else to validate for other types of write operations. diff --git a/src/kudu/client/table_alterer-internal.cc b/src/kudu/client/table_alterer-internal.cc index ff869d5f0..cd02bb61c 100644 --- a/src/kudu/client/table_alterer-internal.cc +++ b/src/kudu/client/table_alterer-internal.cc @@ -112,6 +112,10 @@ Status KuduTableAlterer::Data::ToRequest(AlterTableRequestPB* req) { switch (s.step_type) { case AlterTableRequestPB::ADD_COLUMN: { + if (s.spec->data_->name == Schema::GetAutoIncrementingColumnName()) { + return Status::InvalidArgument("can't add a column with reserved name", + s.spec->data_->name); + } KuduColumnSchema col; RETURN_NOT_OK(s.spec->ToColumnSchema(&col)); ColumnSchemaToPB(*col.col_, @@ -121,6 +125,9 @@ Status KuduTableAlterer::Data::ToRequest(AlterTableRequestPB* req) { } case AlterTableRequestPB::DROP_COLUMN: { + if (s.spec->data_->name == Schema::GetAutoIncrementingColumnName()) { + return Status::InvalidArgument("can't drop column", s.spec->data_->name); + } pb_step->mutable_drop_column()->set_name(s.spec->data_->name); break; } @@ -143,6 +150,22 @@ Status KuduTableAlterer::Data::ToRequest(AlterTableRequestPB* req) { return Status::InvalidArgument("no alter operation specified", s.spec->data_->name); } + if (s.spec->data_->name == Schema::GetAutoIncrementingColumnName()) { + if (s.spec->data_->rename_to) { + return Status::InvalidArgument("can't change name for column", + Schema::GetAutoIncrementingColumnName()); + } else if (s.spec->data_->remove_default) { + return Status::InvalidArgument("can't change remove default for column", + Schema::GetAutoIncrementingColumnName()); + } else if (s.spec->data_->default_val) { + return Status::InvalidArgument("can't change default value for column", + Schema::GetAutoIncrementingColumnName()); + } else if (s.spec->data_->immutable) { + return Status::InvalidArgument("can't change immutability for column", + Schema::GetAutoIncrementingColumnName()); + } + } + // If the alter is solely a column rename, fall back to using // RENAME_COLUMN, for backwards compatibility. // TODO(wdb) Change this when compat can be broken. diff --git a/src/kudu/common/partial_row.cc b/src/kudu/common/partial_row.cc index 19e43793b..432f77e25 100644 --- a/src/kudu/common/partial_row.cc +++ b/src/kudu/common/partial_row.cc @@ -896,6 +896,11 @@ bool KuduPartialRow::IsKeySet() const { return BitmapIsAllSet(isset_bitmap_, 0, schema_->num_key_columns()); } +bool KuduPartialRow::IsNonUniqueKeySet() const { + DCHECK_GE(schema_->num_key_columns(), 1); + DCHECK(schema_->has_auto_incrementing()); + return BitmapIsAllSet(isset_bitmap_, 0, schema_->num_key_columns() - 1); +} std::string KuduPartialRow::ToString() const { ScopedDisableRedaction no_redaction; diff --git a/src/kudu/common/partial_row.h b/src/kudu/common/partial_row.h index 0016b1965..25f2efec8 100644 --- a/src/kudu/common/partial_row.h +++ b/src/kudu/common/partial_row.h @@ -630,6 +630,10 @@ class KUDU_EXPORT KuduPartialRow { /// for this mutation. bool IsKeySet() const; + /// @return @c true if all non-unique key column values have been set + /// for this mutation. + bool IsNonUniqueKeySet() const; + /// @return @c true if all column values have been set. bool AllColumnsSet() const; diff --git a/src/kudu/common/schema-test.cc b/src/kudu/common/schema-test.cc index b1f15ca88..d27838df5 100644 --- a/src/kudu/common/schema-test.cc +++ b/src/kudu/common/schema-test.cc @@ -295,7 +295,7 @@ TEST_F(TestSchema, TestColumnSchemaEquals) { ColumnSchema col1("key", STRING); ColumnSchema col2("key1", STRING); ColumnSchema col3("key", STRING, true); - ColumnSchema col4("key", STRING, true, false, &default_str, &default_str); + ColumnSchema col4("key", STRING, true, false, false, &default_str, &default_str); ASSERT_TRUE(col1.Equals(col1)); ASSERT_FALSE(col1.Equals(col2, ColumnSchema::COMPARE_NAME)); diff --git a/src/kudu/common/schema.cc b/src/kudu/common/schema.cc index 46100f48e..679b3548a 100644 --- a/src/kudu/common/schema.cc +++ b/src/kudu/common/schema.cc @@ -290,34 +290,15 @@ Status Schema::Reset(vector<ColumnSchema> cols, cols_[i].name())); } if (cols_[i].is_auto_incrementing()) { - if (auto_incrementing_col_idx != kColumnNotFound) { - return Status::InvalidArgument( - "Bad schema", "Schemas can have at most one auto-incrementing column"); - } - if (cols_[i].type_info()->type() != INT64) { - return Status::InvalidArgument( - "Bad schema", "auto-incrementing column should be of type int64"); - } - if (cols_[i].is_immutable()) { - return Status::InvalidArgument( - "Bad schema", "auto-incrementing column should be immutable"); - } + // Schemas can have at most one auto-incrementing column + DCHECK_EQ(auto_incrementing_col_idx, kColumnNotFound); + DCHECK_EQ(cols_[i].type_info()->type(), INT64); + DCHECK(!cols_[i].is_nullable()); + DCHECK(!cols_[i].is_immutable()); auto_incrementing_col_idx = i; } } - // TODO:(achennaka) Remove the below section once auto_increment cols are key cols - for (int i = 0; i < cols_.size(); i++) { - if (cols_[i].is_auto_incrementing()) { - auto_incrementing_col_idx = i; - if (cols_[i].type_info()->type() != INT64) { - return Status::InvalidArgument( - "Bad schema", "auto-incrementing column should be of type int64"); - } - break; - } - } - auto_incrementing_col_idx_ = auto_incrementing_col_idx; // Calculate the offset of each column in the row format. col_offsets_.clear(); @@ -329,6 +310,11 @@ Status Schema::Reset(vector<ColumnSchema> cols, if (col.name().empty()) { return Status::InvalidArgument("column names must be non-empty"); } + if (col.name() == Schema::GetAutoIncrementingColumnName() && + !col.is_auto_incrementing()) { + return Status::InvalidArgument(Substitute( + "$0 is a reserved column name", Schema::GetAutoIncrementingColumnName())); + } // The map uses the 'name' string from within the ColumnSchema object. if (!InsertIfNotPresent(&name_to_index_, col.name(), i++)) { return Status::InvalidArgument("Duplicate column name", col.name()); @@ -659,17 +645,13 @@ Status SchemaBuilder::AddColumn(const std::string& name, } if (is_auto_incrementing) { - if (is_nullable || is_immutable) { - return Status::InvalidArgument("auto-incrementing column cannot be nullable or immutable"); - } - if (read_default != nullptr || write_default != nullptr) { - return Status::InvalidArgument("auto-incrementing column cannot have read/write " - "defaults set"); - } - if (type != kudu::INT64) { - return Status::InvalidArgument("auto-incrementing column should be of type INT64"); - } + DCHECK_EQ(type, INT64); + DCHECK(!is_nullable); + DCHECK(!is_immutable); + DCHECK_EQ(read_default, (void *)nullptr); + DCHECK_EQ(write_default, (void *)nullptr); } + return AddColumn(ColumnSchema(name, type, is_nullable, is_immutable, is_auto_incrementing, read_default, write_default), false); } diff --git a/src/kudu/common/schema.h b/src/kudu/common/schema.h index c7154d0fa..fae14cfcf 100644 --- a/src/kudu/common/schema.h +++ b/src/kudu/common/schema.h @@ -985,6 +985,11 @@ class Schema { return first_is_deleted_virtual_column_idx_; } + // Utility function to return the actual name of the auto incrementing column. + static constexpr const char* const GetAutoIncrementingColumnName() { + return auto_incrementing_col_name_; + } + private: // Return a stringified version of the first 'num_columns' columns of the // row. @@ -1040,6 +1045,8 @@ class Schema { // such column exists in the schema. int auto_incrementing_col_idx_; + static constexpr const char* const auto_incrementing_col_name_ = "auto_incrementing_id"; + // NOTE: if you add more members, make sure to add the appropriate code to // CopyFrom() and the move constructor and assignment operator as well, to // prevent subtle bugs. diff --git a/src/kudu/integration-tests/auto_incrementing-itest.cc b/src/kudu/integration-tests/auto_incrementing-itest.cc index 7086c731a..ab2f9a9a3 100644 --- a/src/kudu/integration-tests/auto_incrementing-itest.cc +++ b/src/kudu/integration-tests/auto_incrementing-itest.cc @@ -87,8 +87,7 @@ class AutoIncrementingItest : public tserver::TabletServerTestBase { Status CreateTableWithData() { KuduSchemaBuilder b; - b.AddColumn("c0")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey(); - b.AddColumn("c1")->Type(KuduColumnSchema::INT64)->NotNull()->AutoIncrementing(); + b.AddColumn("c0")->Type(KuduColumnSchema::INT32)->NotNull()->NonUniquePrimaryKey(); RETURN_NOT_OK(b.Build(&kudu_schema_)); // Create a table with a range partition @@ -132,7 +131,8 @@ class AutoIncrementingItest : public tserver::TabletServerTestBase { scan->set_read_mode(READ_LATEST); Schema schema = Schema({ ColumnSchema("c0", INT32), - ColumnSchema("c1",INT64, false,false, true), + ColumnSchema(Schema::GetAutoIncrementingColumnName(), + INT64, false,false, true), },2); RETURN_NOT_OK(SchemaToColumnPBs(schema, scan->mutable_projected_columns())); RETURN_NOT_OK(cluster_->tserver_proxy(ts)->Scan(req, &resp, &rpc)); @@ -160,7 +160,8 @@ TEST_F(AutoIncrementingItest, TestAutoIncrementingItest) { vector<string> results; ASSERT_OK(ScanTablet(j, replicas[0]->tablet_id(), &results)); for (int i = 0; i < kNumRows; i++) { - ASSERT_EQ(Substitute("(int32 c0=$0, int64 c1=$1)", i, i + 1), results[i]); + ASSERT_EQ(Substitute("(int32 c0=$0, int64 $1=$2)", i, + Schema::GetAutoIncrementingColumnName(), i + 1), results[i]); } } }