[YSQL] #255: Separate YCQL, YSQL and YEDIS namespaces

Summary: Separate the namespaces for YCQL, YSQL and YEDIS so that - namespaces for YSQL databases are not show in system_schema.keyspaces in YCQL - YSQL databases can be created without name conflict with YCQL keyspaces - YCQL client cannot create a table in a namespace for a YSQL table Test Plan: TestPgMisc.testNamespaceSeparation Reviewers: mihnea, neil, oleg Reviewed By: oleg Subscribers: neha, kannan, bogdan, yql, bharat Differential Revision: https://phabricator.dev.yugabyte.com/D6263
yugabyte · Mar 10, 2019 · 0037132 · 0037132
1 parent c7cfb84
commit 0037132
Show file tree

Hide file tree

Showing 24 changed files with 351 additions and 85 deletions.
diff --git a/java/yb-client/src/main/java/org/yb/client/AsyncYBClient.java b/java/yb-client/src/main/java/org/yb/client/AsyncYBClient.java
@@ -54,6 +54,7 @@
 
 import org.jboss.netty.buffer.ChannelBuffer;
 import org.yb.Common;
+import org.yb.Common.YQLDatabase;
 import org.yb.Schema;
 import org.yb.annotations.InterfaceAudience;
 import org.yb.annotations.InterfaceStability;
@@ -375,6 +376,20 @@ public Deferred<CreateKeyspaceResponse> createKeyspace(String keyspace)
     return sendRpcToTablet(request);
   }
 
+  /*
+   * Create a keyspace (namespace) for the specified database type.
+   * @param name of the keyspace.
+   */
+  public Deferred<CreateKeyspaceResponse> createKeyspace(String keyspace, YQLDatabase databaseType)
+      throws Exception {
+    checkIsClosed();
+    CreateKeyspaceRequest request = new CreateKeyspaceRequest(this.masterTable,
+                                                              keyspace,
+                                                              databaseType);
+    request.setTimeoutMillis(defaultAdminOperationTimeoutMs);
+    return sendRpcToTablet(request);
+  }
+
   /**
    * Delete a table on the cluster with the specified name.
    * @param keyspace CQL keyspace to which this table belongs

diff --git a/java/yb-client/src/main/java/org/yb/client/CreateKeyspaceRequest.java b/java/yb-client/src/main/java/org/yb/client/CreateKeyspaceRequest.java
@@ -19,6 +19,7 @@
 
 import org.yb.annotations.InterfaceAudience;
 import org.yb.Common.HostPortPB;
+import org.yb.Common.YQLDatabase;
 import org.yb.consensus.Consensus;
 import org.yb.consensus.Metadata;
 import org.yb.consensus.Metadata.RaftPeerPB;
@@ -31,18 +32,27 @@
 @InterfaceAudience.Public
 class CreateKeyspaceRequest extends YRpc<CreateKeyspaceResponse> {
   private String name;
+  private YQLDatabase databaseType;
 
   public CreateKeyspaceRequest(YBTable masterTable, String name) {
     super(masterTable);
     this.name = name;
   }
 
+  public CreateKeyspaceRequest(YBTable masterTable, String name, YQLDatabase databaseType) {
+    super(masterTable);
+    this.name = name;
+    this.databaseType = databaseType;
+  }
+
   @Override
   ChannelBuffer serialize(Message header) {
     assert header.isInitialized();
     final Master.CreateNamespaceRequestPB.Builder builder =
       Master.CreateNamespaceRequestPB.newBuilder();
     builder.setName(this.name);
+    if (this.databaseType != null)
+      builder.setDatabaseType(this.databaseType);
 
     return toChannelBuffer(header, builder.build());
   }

diff --git a/java/yb-client/src/main/java/org/yb/client/YBClient.java b/java/yb-client/src/main/java/org/yb/client/YBClient.java
@@ -42,6 +42,7 @@
 import org.slf4j.LoggerFactory;
 import org.yb.ColumnSchema;
 import org.yb.Common.TableType;
+import org.yb.Common.YQLDatabase;
 import org.yb.Schema;
 import org.yb.Type;
 import org.yb.annotations.InterfaceAudience;
@@ -116,7 +117,8 @@ public static CreateTableOptions getRedisTableOptions(int numTablets) {
   }
 
   public void createRedisNamespace() throws Exception {
-    CreateKeyspaceResponse resp = this.createKeyspace(REDIS_KEYSPACE_NAME);
+    CreateKeyspaceResponse resp = this.createKeyspace(REDIS_KEYSPACE_NAME,
+                                                      YQLDatabase.YQL_DATABASE_REDIS);
     if (resp.hasError()) {
       throw new RuntimeException("Could not create keyspace " + REDIS_KEYSPACE_NAME +
                                  ". Error :" + resp.errorMessage());
@@ -187,6 +189,16 @@ public CreateKeyspaceResponse createKeyspace(String keyspace)
     return d.join(getDefaultAdminOperationTimeoutMs());
   }
 
+  /*
+   * Create a keyspace (namespace) for the specified database type.
+   * @param non-null name of the keyspace.
+   */
+  public CreateKeyspaceResponse createKeyspace(String keyspace, YQLDatabase databaseType)
+      throws Exception {
+    Deferred<CreateKeyspaceResponse> d = asyncClient.createKeyspace(keyspace, databaseType);
+    return d.join(getDefaultAdminOperationTimeoutMs());
+  }
+
   /**
    * Delete a table on the cluster with the specified name.
    * @param keyspace CQL keyspace to which this table belongs

diff --git a/java/yb-pgsql/pom.xml b/java/yb-pgsql/pom.xml
@@ -29,6 +29,10 @@
       <groupId>org.postgresql</groupId>
       <artifactId>postgresql</artifactId>
     </dependency>
+    <dependency>
+      <groupId>com.yugabyte</groupId>
+      <artifactId>cassandra-driver-core</artifactId>
+    </dependency>
     <dependency>
       <groupId>log4j</groupId>
       <artifactId>log4j</artifactId>

diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/BasePgSQLTest.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/BasePgSQLTest.java
@@ -53,6 +53,10 @@ public class BasePgSQLTest extends BaseMiniClusterTest {
   private static final String PG_DATA_FLAG = "PGDATA";
   private static final String YB_ENABLED_IN_PG_ENV_VAR_NAME = "YB_ENABLED_IN_POSTGRES";
 
+  // CQL and Redis settings.
+  protected static boolean startCqlProxy = false;
+  protected static boolean startRedisProxy = false;
+
   protected static Connection connection;
 
   private static File initdbDataDir;
@@ -103,8 +107,8 @@ private Map<String, String> getTServerAndInitDbFlags() {
     if (isTSAN()) {
       flagMap.put("yb_client_admin_operation_timeout_sec", "120");
     }
-    flagMap.put("start_redis_proxy", "false");
-    flagMap.put("start_cql_proxy", "false");
+    flagMap.put("start_cql_proxy", Boolean.toString(startCqlProxy));
+    flagMap.put("start_redis_proxy", Boolean.toString(startRedisProxy));
 
     return flagMap;
   }

diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgMisc.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgMisc.java
@@ -0,0 +1,78 @@
+// Copyright (c) YugaByte, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+// in compliance with the License.  You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software distributed under the License
+// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+// or implied.  See the License for the specific language governing permissions and limitations
+// under the License.
+//
+
+package org.yb.pgsql;
+
+import java.util.*;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.yb.util.YBTestRunnerNonTsanOnly;
+
+import java.sql.Statement;
+
+import com.datastax.driver.core.Cluster;
+import com.datastax.driver.core.ResultSet;
+import com.datastax.driver.core.Session;
+import com.datastax.driver.core.exceptions.QueryValidationException;
+
+import static org.yb.AssertionWrappers.*;
+
+@RunWith(value=YBTestRunnerNonTsanOnly.class)
+public class TestPgMisc extends BasePgSQLTest {
+
+  private static final Logger LOG = LoggerFactory.getLogger(TestPgMisc.class);
+
+  @BeforeClass
+  public static void SetUpBeforeClass() throws Exception {
+    // Starts CQL proxy for the cross Postgres/CQL testNamespaceSeparation test case.
+    BasePgSQLTest.startCqlProxy = true;
+  }
+
+  protected void assertResult(ResultSet rs, Set<String> expectedRows) {
+    Set<String> actualRows = new HashSet<>();
+    for (com.datastax.driver.core.Row row : rs) {
+      actualRows.add(row.toString());
+    }
+    assertEquals(expectedRows, actualRows);
+  }
+
+  @Test
+  public void testNamespaceSeparation() throws Exception {
+    // Create a YCQL session.
+    Session session = Cluster.builder()
+                             .addContactPointsWithPorts(miniCluster.getCQLContactPoints())
+                             .build()
+                             .connect();
+
+    // Verify that namespaces for YSQL databases are not shown in YCQL.
+    assertResult(session.execute("select keyspace_name from system_schema.keyspaces;"),
+                 new HashSet<String>(Arrays.asList("Row[system]",
+                                                   "Row[system_auth]",
+                                                   "Row[system_schema]")));
+
+    // Verify that YSQL table cannot be created in namespaces for YSQL databases.
+    try {
+      session.execute("create table template1.t (a int primary key);");
+      fail("YCQL table created in namespace for YSQL database");
+    } catch (QueryValidationException e) {
+      LOG.info("Expected exception", e);
+    }
+
+    // Verify that YSQL database can be created with the same name as an existing YCQL keyspace.
+    connection.createStatement().execute("create database system;");
+  }
+}
diff --git a/src/yb/benchmarks/yb_load_test_tool.cc b/src/yb/benchmarks/yb_load_test_tool.cc
@@ -125,6 +125,7 @@ using yb::CountDownLatch;
 using yb::Slice;
 using yb::YBPartialRow;
 using yb::TableType;
+using yb::YQLDatabase;
 
 using strings::Substitute;
 
@@ -246,7 +247,8 @@ void SetupYBTable(const shared_ptr<YBClient> &client) {
     keyspace = yb::common::kRedisKeyspaceName;
   }
   const YBTableName table_name(keyspace, FLAGS_table_name);
-  CHECK_OK(client->CreateNamespaceIfNotExists(table_name.namespace_name()));
+  CHECK_OK(client->CreateNamespaceIfNotExists(table_name.namespace_name(),
+                                              YQLDatabase::YQL_DATABASE_REDIS));
 
   if (!YBTableExistsAlready(client, table_name) || DropTableIfNecessary(client, table_name)) {
     CreateTable(table_name, client);

diff --git a/src/yb/client/client.cc b/src/yb/client/client.cc
@@ -561,7 +561,7 @@ Status YBClient::GetTableSchema(const YBTableName& table_name,
 }
 
 Status YBClient::CreateNamespace(const std::string& namespace_name,
-                                 const YQLDatabase database_type,
+                                 const boost::optional<YQLDatabase>& database_type,
                                  const std::string& creator_role_name,
                                  const std::string& namespace_id,
                                  const std::string& source_namespace_id,
@@ -572,8 +572,8 @@ Status YBClient::CreateNamespace(const std::string& namespace_name,
   if (!creator_role_name.empty()) {
     req.set_creator_role_name(creator_role_name);
   }
-  if (database_type != YQL_DATABASE_UNDEFINED) {
-    req.set_database_type(database_type);
+  if (database_type) {
+    req.set_database_type(*database_type);
   }
   if (!namespace_id.empty()) {
     req.set_namespace_id(namespace_id);
@@ -589,30 +589,31 @@ Status YBClient::CreateNamespace(const std::string& namespace_name,
 }
 
 Status YBClient::CreateNamespaceIfNotExists(const std::string& namespace_name,
-                                            YQLDatabase database_type) {
+                                            const boost::optional<YQLDatabase>& database_type) {
   Result<bool> namespace_exists = NamespaceExists(namespace_name);
   RETURN_NOT_OK(namespace_exists);
   return namespace_exists.get() ? Status::OK()
                                 : CreateNamespace(namespace_name, database_type);
 }
 
 Status YBClient::DeleteNamespace(const std::string& namespace_name,
-                                 YQLDatabase database_type) {
+                                 const boost::optional<YQLDatabase>& database_type) {
   DeleteNamespaceRequestPB req;
   DeleteNamespaceResponsePB resp;
   req.mutable_namespace_()->set_name(namespace_name);
-  if (database_type != YQL_DATABASE_UNDEFINED) {
-    req.set_database_type(database_type);
+  if (database_type) {
+    req.set_database_type(*database_type);
   }
   CALL_SYNC_LEADER_MASTER_RPC(req, resp, DeleteNamespace);
   return Status::OK();
 }
 
-Status YBClient::ListNamespaces(YQLDatabase database_type, std::vector<std::string>* namespaces) {
+Status YBClient::ListNamespaces(const boost::optional<YQLDatabase>& database_type,
+                                std::vector<std::string>* namespaces) {
   ListNamespacesRequestPB req;
   ListNamespacesResponsePB resp;
-  if (database_type != YQL_DATABASE_UNDEFINED) {
-    req.set_database_type(database_type);
+  if (database_type) {
+    req.set_database_type(*database_type);
   }
   CALL_SYNC_LEADER_MASTER_RPC(req, resp, ListNamespaces);
 
@@ -673,7 +674,7 @@ Status YBClient::GrantRevokePermission(GrantRevokeStatementType statement_type,
 }
 
 Result<bool> YBClient::NamespaceExists(const std::string& namespace_name,
-                                       YQLDatabase database_type) {
+                                       const boost::optional<YQLDatabase>& database_type) {
   std::vector<std::string> namespaces;
   RETURN_NOT_OK(ListNamespaces(database_type, &namespaces));
 
@@ -1557,7 +1558,7 @@ Status YBTableCreator::Create() {
   // Build request.
   CreateTableRequestPB req;
   req.set_name(data_->table_name_.table_name());
-  req.mutable_namespace_()->set_name(data_->table_name_.resolved_namespace_name());
+  data_->table_name_.SetIntoNamespaceIdentifierPB(req.mutable_namespace_());
   req.set_table_type(data_->table_type_);
 
   if (!data_->creator_role_name_.empty()) {

diff --git a/src/yb/client/client.h b/src/yb/client/client.h
@@ -318,7 +318,7 @@ class YBClient : public std::enable_shared_from_this<YBClient> {
   // TODO(neil) When database_type is undefined, backend will not check error on database type.
   // Except for testing we should use proper database_types for all creations.
   CHECKED_STATUS CreateNamespace(const std::string& namespace_name,
-                                 YQLDatabase database_type = YQL_DATABASE_UNDEFINED,
+                                 const boost::optional<YQLDatabase>& database_type = boost::none,
                                  const std::string& creator_role_name = "",
                                  const std::string& namespace_id = "",
                                  const std::string& source_namespace_id = "",
@@ -329,11 +329,12 @@ class YBClient : public std::enable_shared_from_this<YBClient> {
   // TODO(neil) When database_type is undefined, backend will not check error on database type.
   // Except for testing we should use proper database_types for all creations.
   CHECKED_STATUS CreateNamespaceIfNotExists(const std::string& namespace_name,
-                                            YQLDatabase database_type = YQL_DATABASE_UNDEFINED);
+                                            const boost::optional<YQLDatabase>& database_type =
+                                            boost::none);
 
   // Delete namespace with the given name.
   CHECKED_STATUS DeleteNamespace(const std::string& namespace_name,
-                                 YQLDatabase database_type = YQL_DATABASE_UNDEFINED);
+                                 const boost::optional<YQLDatabase>& database_type = boost::none);
 
   // For Postgres: reserve oids for a Postgres database.
   CHECKED_STATUS ReservePgsqlOids(const std::string& namespace_id,
@@ -353,14 +354,15 @@ class YBClient : public std::enable_shared_from_this<YBClient> {
   // List all namespace names.
   // 'namespaces' is appended to only on success.
   CHECKED_STATUS ListNamespaces(std::vector<std::string>* namespaces) {
-    return ListNamespaces(YQL_DATABASE_UNDEFINED, namespaces);
+    return ListNamespaces(boost::none, namespaces);
   }
-  CHECKED_STATUS ListNamespaces(YQLDatabase database_type, std::vector<std::string>* namespaces);
+  CHECKED_STATUS ListNamespaces(const boost::optional<YQLDatabase>& database_type,
+                                std::vector<std::string>* namespaces);
 
   // Check if the namespace given by 'namespace_name' exists.
   // Result value is set only on success.
   Result<bool> NamespaceExists(const std::string& namespace_name,
-                               YQLDatabase database_type = YQL_DATABASE_UNDEFINED);
+                               const boost::optional<YQLDatabase>& database_type = boost::none);
 
   // Authentication and Authorization
   // Create a new role.

diff --git a/src/yb/client/yb_table_name.cc b/src/yb/client/yb_table_name.cc
@@ -26,13 +26,31 @@ DEFINE_bool(yb_system_namespace_readonly, true, "Set system keyspace read-only."
 using std::string;
 
 void YBTableName::SetIntoTableIdentifierPB(master::TableIdentifierPB* id) const {
+  SetIntoNamespaceIdentifierPB(id->mutable_namespace_());
   id->set_table_name(table_name());
-  id->mutable_namespace_()->set_name(resolved_namespace_name());
 }
 
 void YBTableName::GetFromTableIdentifierPB(const master::TableIdentifierPB& id) {
+  GetFromNamespaceIdentifierPB(id.namespace_());
   table_name_ = id.table_name();
-  namespace_name_ = id.namespace_().name();
+}
+
+void YBTableName::SetIntoNamespaceIdentifierPB(master::NamespaceIdentifierPB* id) const {
+  id->set_name(resolved_namespace_name());
+  if (!namespace_id_.empty()) {
+    id->set_id(namespace_id_);
+  } else {
+    id->clear_id();
+  }
+}
+
+void YBTableName::GetFromNamespaceIdentifierPB(const master::NamespaceIdentifierPB& id) {
+  namespace_name_ = id.name();
+  if (id.has_id()) {
+    namespace_id_ = id.id();
+  } else {
+    namespace_id_.clear();
+  }
 }
 
 bool YBTableName::IsSystemNamespace(const std::string& namespace_name) {