You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2023/01/26 16:32:42 UTC
[kudu] branch master updated: KUDU-1945 Auto-Incrementing Column, C++ client
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 <gr...@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 <al...@apache.org>
Reviewed-by: Abhishek Chennaka <ac...@cloudera.com>
Tested-by: Alexey Serbin <al...@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]);
}
}
}