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 7fafdd280 KUDU-1945 Auto-incrementing column feature flag
7fafdd280 is described below

commit 7fafdd2803a2c747faa6789a1921d51186edfb6e
Author: Marton Greber <greber...@gmail.com>
AuthorDate: Wed Feb 15 22:34:15 2023 +0100

    KUDU-1945 Auto-incrementing column feature flag
    
    This patch adds the flag "--master_support_auto_incrementing_column" to
    guard the auto-incrementing column feature. It prohibits users to create
    table with non-unique primary key. The default value for the flag is set
    to true in this patch. The verification happens on the RPC level. If the
    server has the feature turned off, or the server is older compared to
    the client - doesn't have the auto-incrementing feature -, the request
    gets rejected in both cases.
    
    Change-Id: I39c3dde3705c25c36d3ad787c0db6ed03f6c2cfd
    Reviewed-on: http://gerrit.cloudera.org:8080/19523
    Tested-by: Kudu Jenkins
    Reviewed-by: Wenzhe Zhou <wz...@cloudera.com>
    Reviewed-by: Alexey Serbin <ale...@apache.org>
---
 .../org/apache/kudu/client/CreateTableOptions.java |  4 ++++
 .../java/org/apache/kudu/client/TestKuduTable.java | 19 ++++++++++++++++++
 src/kudu/client/client-internal.cc                 |  6 +++++-
 src/kudu/client/client-internal.h                  |  3 ++-
 src/kudu/client/client-test.cc                     | 23 ++++++++++++++++++++--
 src/kudu/client/client.cc                          |  4 +++-
 src/kudu/master/master.proto                       |  2 ++
 src/kudu/master/master_service.cc                  |  8 ++++++++
 8 files changed, 64 insertions(+), 5 deletions(-)

diff --git 
a/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java 
b/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
index 5e3547eea..d577a590f 100644
--- 
a/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
+++ 
b/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
@@ -337,6 +337,10 @@ public class CreateTableOptions {
 
   List<Integer> getRequiredFeatureFlags(Schema schema) {
     List<Integer> requiredFeatureFlags = new ArrayList<>();
+    if (schema.hasAutoIncrementingColumn()) {
+      requiredFeatureFlags.add(
+              
Integer.valueOf(Master.MasterFeatures.AUTO_INCREMENTING_COLUMN_VALUE));
+    }
     if (schema.hasImmutableColumns()) {
       requiredFeatureFlags.add(
               
Integer.valueOf(Master.MasterFeatures.IMMUTABLE_COLUMN_ATTRIBUTE_VALUE));
diff --git 
a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java 
b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
index 6f5469241..035b68a18 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
@@ -24,6 +24,7 @@ import static 
org.apache.kudu.client.KuduPredicate.ComparisonOp.LESS;
 import static org.apache.kudu.client.KuduPredicate.ComparisonOp.LESS_EQUAL;
 import static org.apache.kudu.test.ClientTestUtil.createBasicSchemaInsert;
 import static 
org.apache.kudu.test.ClientTestUtil.createSchemaWithImmutableColumns;
+import static org.apache.kudu.test.ClientTestUtil.createSchemaWithNonUniqueKey;
 import static org.apache.kudu.test.ClientTestUtil.getBasicCreateTableOptions;
 import static org.apache.kudu.test.ClientTestUtil.getBasicSchema;
 import static 
org.apache.kudu.test.ClientTestUtil.getBasicTableOptionsWithNonCoveredRange;
@@ -2662,4 +2663,22 @@ public class TestKuduTable {
           "More than one columns are set as auto-incrementing columns"));
     }
   }
+
+  @Test(timeout = 100000)
+  @KuduTestHarness.MasterServerConfig(flags = {
+      "--master_support_auto_incrementing_column=false"
+  })
+  public void testCreateTableWithAutoIncrementingColWhenMasterNotSupport() 
throws Exception {
+    try {
+      CreateTableOptions builder = getBasicCreateTableOptions();
+      client.createTable(tableName, createSchemaWithNonUniqueKey(), builder);
+      fail("shouldn't be able to create a table with auto-incrementing column 
" +
+          "when server side doesn't support required AUTO_INCREMENTING_COLUMN 
feature");
+    } catch (KuduException ex) {
+      final String errmsg = ex.getMessage();
+      assertTrue(errmsg, ex.getStatus().isRemoteError());
+      assertTrue(errmsg, errmsg.matches(
+          ".* server sent error unsupported feature flags"));
+    }
+  }
 }
diff --git a/src/kudu/client/client-internal.cc 
b/src/kudu/client/client-internal.cc
index ca6bd53d8..e4faa05db 100644
--- a/src/kudu/client/client-internal.cc
+++ b/src/kudu/client/client-internal.cc
@@ -319,7 +319,8 @@ Status KuduClient::Data::CreateTable(KuduClient* client,
                                      const MonoTime& deadline,
                                      bool has_range_partition_bounds,
                                      bool has_range_specific_hash_schema,
-                                     bool has_immutable_column_schema) {
+                                     bool has_immutable_column_schema,
+                                     bool has_auto_incrementing_column) {
   vector<uint32_t> features;
   if (has_range_partition_bounds) {
     features.push_back(MasterFeatures::RANGE_PARTITION_BOUNDS);
@@ -330,6 +331,9 @@ Status KuduClient::Data::CreateTable(KuduClient* client,
   if (has_immutable_column_schema) {
     features.push_back(MasterFeatures::IMMUTABLE_COLUMN_ATTRIBUTE);
   }
+  if (has_auto_incrementing_column) {
+    features.push_back(MasterFeatures::AUTO_INCREMENTING_COLUMN);
+  }
   Synchronizer sync;
   AsyncLeaderMasterRpc<CreateTableRequestPB, CreateTableResponsePB> rpc(
       deadline, client, BackoffType::EXPONENTIAL, req, resp,
diff --git a/src/kudu/client/client-internal.h 
b/src/kudu/client/client-internal.h
index b29b77d91..56c0965c0 100644
--- a/src/kudu/client/client-internal.h
+++ b/src/kudu/client/client-internal.h
@@ -108,7 +108,8 @@ class KuduClient::Data {
                             const MonoTime& deadline,
                             bool has_range_partition_bounds,
                             bool has_range_specific_hash_schema,
-                            bool has_immutable_column_schema);
+                            bool has_immutable_column_schema,
+                            bool has_auto_incrementing_column);
 
   static Status IsCreateTableInProgress(KuduClient* client,
                                         master::TableIdentifierPB table,
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index e1a5e451c..41e5f694a 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -147,6 +147,7 @@ DECLARE_bool(fail_dns_resolution);
 DECLARE_bool(location_mapping_by_uuid);
 DECLARE_bool(log_inject_latency);
 DECLARE_bool(master_client_location_assignment_enabled);
+DECLARE_bool(master_support_auto_incrementing_column);
 DECLARE_bool(master_support_connect_to_master_rpc);
 DECLARE_bool(master_support_immutable_column_attribute);
 DECLARE_bool(mock_table_metrics_for_testing);
@@ -9852,7 +9853,6 @@ TEST_F(ReplicationFactorLimitsTest, MaxReplicationFactor) 
{
 class ClientTestAutoIncrementingColumn : public ClientTest {
  public:
   void SetUp() override {
-    // TODO:achennaka Enable Feature flag here once implemented
 
     KuduTest::SetUp();
 
@@ -9924,7 +9924,6 @@ TEST_F(ClientTestAutoIncrementingColumn, ReadAndWrite) {
   }
 }
 
-
 TEST_F(ClientTestAutoIncrementingColumn, ConcurrentWrites) {
   const string kTableName = "concurrent_writes_auto_incrementing_column";
   KuduSchemaBuilder b;
@@ -10139,5 +10138,25 @@ TEST_F(ClientTestAutoIncrementingColumn, 
InsertOperationNegatives) {
                         "with auto-incrementing column");
   }
 }
+
+TEST_F(ClientTestAutoIncrementingColumn, CreateTableFeatureFlag) {
+  FLAGS_master_support_auto_incrementing_column = false;
+  const string kTableName = 
"create_table_with_auto_incrementing_column_feature_flag";
+  KuduSchemaBuilder b;
+  
b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->NonUniquePrimaryKey();
+  ASSERT_OK(b.Build(&schema_));
+
+  unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+  Status s = table_creator->table_name(kTableName)
+                .schema(&schema_)
+                .add_hash_partitions({"key"}, 2)
+                .num_replicas(1)
+                .Create();
+  ASSERT_TRUE(s.IsNotSupported()) << s.ToString();
+  ASSERT_STR_CONTAINS(
+      s.ToString(),
+      Substitute("Error creating table $0 on the master: cluster does not 
support "
+                 "CreateTable with feature(s) AUTO_INCREMENTING_COLUMN", 
kTableName));
+}
 } // namespace client
 } // namespace kudu
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index 21efb537f..0a128f604 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -1060,6 +1060,7 @@ Status KuduTableCreator::Create() {
       break;
     }
   }
+  bool has_auto_incrementing_column = 
data_->schema_->schema_->has_auto_incrementing();
 
   if (data_->table_type_) {
     req.set_table_type(*data_->table_type_);
@@ -1080,7 +1081,8 @@ Status KuduTableCreator::Create() {
                                          deadline,
                                          !data_->range_partitions_.empty(),
                                          has_range_with_custom_hash_schema,
-                                         has_immutable_column_schema),
+                                         has_immutable_column_schema,
+                                         has_auto_incrementing_column),
       Substitute("Error creating table $0 on the master", data_->table_name_));
   // Spin until the table is fully created, if requested.
   if (data_->wait_) {
diff --git a/src/kudu/master/master.proto b/src/kudu/master/master.proto
index d527ec11c..0493be6e2 100644
--- a/src/kudu/master/master.proto
+++ b/src/kudu/master/master.proto
@@ -1176,6 +1176,8 @@ enum MasterFeatures {
   UPSERT_IGNORE = 9;
   // Whether master supports immutable attribute on column schema.
   IMMUTABLE_COLUMN_ATTRIBUTE = 10;
+  // Whether master supports auto incrementing column.
+  AUTO_INCREMENTING_COLUMN = 11;
 }
 
 service MasterService {
diff --git a/src/kudu/master/master_service.cc 
b/src/kudu/master/master_service.cc
index 234008826..f5944aa3b 100644
--- a/src/kudu/master/master_service.cc
+++ b/src/kudu/master/master_service.cc
@@ -118,6 +118,12 @@ DEFINE_bool(master_support_immutable_column_attribute, 
true,
 TAG_FLAG(master_support_immutable_column_attribute, hidden);
 TAG_FLAG(master_support_immutable_column_attribute, runtime);
 
+DEFINE_bool(master_support_auto_incrementing_column, true,
+            "Whether the cluster supports auto-incrementing columns. "
+            "This in turn enables the usage of non-unique primary key.");
+TAG_FLAG(master_support_auto_incrementing_column, unsafe);
+TAG_FLAG(master_support_auto_incrementing_column, experimental);
+TAG_FLAG(master_support_auto_incrementing_column, runtime);
 
 using google::protobuf::Message;
 using kudu::consensus::ReplicaManagementInfoPB;
@@ -956,6 +962,8 @@ bool MasterServiceImpl::SupportsFeature(uint32_t feature) 
const {
       return FLAGS_master_support_upsert_ignore_operations;
     case MasterFeatures::IMMUTABLE_COLUMN_ATTRIBUTE:
       return FLAGS_master_support_immutable_column_attribute;
+    case MasterFeatures::AUTO_INCREMENTING_COLUMN:
+      return FLAGS_master_support_auto_incrementing_column;
     default:
       return false;
   }

Reply via email to