blerer commented on code in PR #3718: URL: https://github.com/apache/cassandra/pull/3718#discussion_r1867366551
########## src/java/org/apache/cassandra/cql3/statements/schema/CopyTableStatement.java: ########## @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.cassandra.cql3.statements.schema; + +import org.apache.cassandra.audit.AuditLogContext; +import org.apache.cassandra.audit.AuditLogEntryType; +import org.apache.cassandra.auth.Permission; +import org.apache.cassandra.cql3.CQLStatement; +import org.apache.cassandra.cql3.QualifiedName; +import org.apache.cassandra.db.guardrails.Guardrails; +import org.apache.cassandra.exceptions.AlreadyExistsException; +import org.apache.cassandra.schema.KeyspaceMetadata; +import org.apache.cassandra.schema.Keyspaces; +import org.apache.cassandra.schema.TableId; +import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.service.ClientState; +import org.apache.cassandra.service.reads.repair.ReadRepairStrategy; +import org.apache.cassandra.tcm.ClusterMetadata; +import org.apache.cassandra.transport.Event.SchemaChange; + +public final class CopyTableStatement extends AlterSchemaStatement +{ + private final String sourceKeyspace; + private final String sourceTableName; + private final String targetKeyspace; + private final String targetTableName; + + public CopyTableStatement(String sourceKeyspace, + String targetKeyspace, + String sourceTableName, + String targetTableName) + { + super(targetKeyspace); + this.sourceKeyspace = sourceKeyspace; + this.targetKeyspace = targetKeyspace; + this.sourceTableName = sourceTableName; + this.targetTableName = targetTableName; + } + + @Override + SchemaChange schemaChangeEvent(Keyspaces.KeyspacesDiff diff) + { + return new SchemaChange(SchemaChange.Change.CREATED, SchemaChange.Target.TABLE, targetKeyspace, targetTableName); + } + + @Override + public void authorize(ClientState client) + { + client.ensureTablePermission(sourceKeyspace, sourceTableName, Permission.SELECT); + client.ensureAllTablesPermission(targetKeyspace, Permission.CREATE); + } + + @Override + public AuditLogContext getAuditLogContext() + { + return new AuditLogContext(AuditLogEntryType.CREATE_TABLE_LIKE, targetKeyspace, targetTableName); + } + + public String toString() + { + return String.format("CREATE TABLE (%s, %s) LIKE (%s, %s)", targetKeyspace, targetTableName, sourceKeyspace, sourceTableName); + } + + @Override Review Comment: Is there a real need to override the method? ########## src/java/org/apache/cassandra/cql3/QueryProcessor.java: ########## @@ -144,7 +144,6 @@ public class QueryProcessor implements QueryHandler SystemKeyspace.removePreparedStatement(md5Digest); } }).build(); - Review Comment: nit: We should probably leave the blank line. ########## test/unit/org/apache/cassandra/cql3/CQLTester.java: ########## @@ -1628,6 +1673,12 @@ protected final String formatQuery(String keyspace, String query) return currentTable == null ? query : String.format(query, keyspace + "." + currentTable); } + protected final String formatQuery(String sourceKeyspace, String sourceTable, String targetKeyspace, String query) Review Comment: I would inline that method into `createTableLike`. It is too specific ########## src/java/org/apache/cassandra/cql3/statements/schema/CopyTableStatement.java: ########## @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.cassandra.cql3.statements.schema; + +import org.apache.cassandra.audit.AuditLogContext; +import org.apache.cassandra.audit.AuditLogEntryType; +import org.apache.cassandra.auth.Permission; +import org.apache.cassandra.cql3.CQLStatement; +import org.apache.cassandra.cql3.QualifiedName; +import org.apache.cassandra.db.guardrails.Guardrails; +import org.apache.cassandra.exceptions.AlreadyExistsException; +import org.apache.cassandra.schema.KeyspaceMetadata; +import org.apache.cassandra.schema.Keyspaces; +import org.apache.cassandra.schema.TableId; +import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.service.ClientState; +import org.apache.cassandra.service.reads.repair.ReadRepairStrategy; +import org.apache.cassandra.tcm.ClusterMetadata; +import org.apache.cassandra.transport.Event.SchemaChange; + +public final class CopyTableStatement extends AlterSchemaStatement +{ + private final String sourceKeyspace; + private final String sourceTableName; + private final String targetKeyspace; + private final String targetTableName; + + public CopyTableStatement(String sourceKeyspace, + String targetKeyspace, + String sourceTableName, + String targetTableName) + { + super(targetKeyspace); + this.sourceKeyspace = sourceKeyspace; + this.targetKeyspace = targetKeyspace; + this.sourceTableName = sourceTableName; + this.targetTableName = targetTableName; + } + + @Override + SchemaChange schemaChangeEvent(Keyspaces.KeyspacesDiff diff) + { + return new SchemaChange(SchemaChange.Change.CREATED, SchemaChange.Target.TABLE, targetKeyspace, targetTableName); + } + + @Override + public void authorize(ClientState client) + { + client.ensureTablePermission(sourceKeyspace, sourceTableName, Permission.SELECT); + client.ensureAllTablesPermission(targetKeyspace, Permission.CREATE); + } + + @Override + public AuditLogContext getAuditLogContext() + { + return new AuditLogContext(AuditLogEntryType.CREATE_TABLE_LIKE, targetKeyspace, targetTableName); + } + + public String toString() + { + return String.format("CREATE TABLE (%s, %s) LIKE (%s, %s)", targetKeyspace, targetTableName, sourceKeyspace, sourceTableName); + } + + @Override + public void validate(ClientState state) + { + super.validate(state); + } + + @Override + public Keyspaces apply(ClusterMetadata metadata) + { + Keyspaces schema = metadata.schema.getKeyspaces(); + KeyspaceMetadata sourceKeyspaceMeta = schema.getNullable(sourceKeyspace); + TableMetadata sourceTableMeta = sourceKeyspaceMeta.getTableNullable(sourceTableName); + + if (null == sourceKeyspaceMeta) + throw ire("Source Keyspace '%s' doesn't exist", sourceKeyspace); + + if (!sourceKeyspaceMeta.hasTable(sourceTableName)) + { + throw ire("Souce Table '%s'.'%s' doesn't exist", sourceKeyspace, sourceTableName); + } + + if (sourceTableMeta.isIndex()) + throw ire("Cannot use CTREAT TABLE LIKE on a index table '%s'.'%s'.", sourceKeyspace, sourceTableName); Review Comment: Typo CTREAT -> CREATE ########## src/java/org/apache/cassandra/cql3/statements/schema/CopyTableStatement.java: ########## @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.cassandra.cql3.statements.schema; + +import org.apache.cassandra.audit.AuditLogContext; +import org.apache.cassandra.audit.AuditLogEntryType; +import org.apache.cassandra.auth.Permission; +import org.apache.cassandra.cql3.CQLStatement; +import org.apache.cassandra.cql3.QualifiedName; +import org.apache.cassandra.db.guardrails.Guardrails; +import org.apache.cassandra.exceptions.AlreadyExistsException; +import org.apache.cassandra.schema.KeyspaceMetadata; +import org.apache.cassandra.schema.Keyspaces; +import org.apache.cassandra.schema.TableId; +import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.service.ClientState; +import org.apache.cassandra.service.reads.repair.ReadRepairStrategy; +import org.apache.cassandra.tcm.ClusterMetadata; +import org.apache.cassandra.transport.Event.SchemaChange; + +public final class CopyTableStatement extends AlterSchemaStatement Review Comment: It would be nice to have some documentation explaining to which CQL statement this statement is corresponding. ########## src/java/org/apache/cassandra/cql3/statements/schema/CopyTableStatement.java: ########## @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.cassandra.cql3.statements.schema; + +import org.apache.cassandra.audit.AuditLogContext; +import org.apache.cassandra.audit.AuditLogEntryType; +import org.apache.cassandra.auth.Permission; +import org.apache.cassandra.cql3.CQLStatement; +import org.apache.cassandra.cql3.QualifiedName; +import org.apache.cassandra.db.guardrails.Guardrails; +import org.apache.cassandra.exceptions.AlreadyExistsException; +import org.apache.cassandra.schema.KeyspaceMetadata; +import org.apache.cassandra.schema.Keyspaces; +import org.apache.cassandra.schema.TableId; +import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.service.ClientState; +import org.apache.cassandra.service.reads.repair.ReadRepairStrategy; +import org.apache.cassandra.tcm.ClusterMetadata; +import org.apache.cassandra.transport.Event.SchemaChange; + +public final class CopyTableStatement extends AlterSchemaStatement +{ + private final String sourceKeyspace; + private final String sourceTableName; + private final String targetKeyspace; + private final String targetTableName; + + public CopyTableStatement(String sourceKeyspace, + String targetKeyspace, + String sourceTableName, + String targetTableName) + { + super(targetKeyspace); + this.sourceKeyspace = sourceKeyspace; + this.targetKeyspace = targetKeyspace; + this.sourceTableName = sourceTableName; + this.targetTableName = targetTableName; + } + + @Override + SchemaChange schemaChangeEvent(Keyspaces.KeyspacesDiff diff) + { + return new SchemaChange(SchemaChange.Change.CREATED, SchemaChange.Target.TABLE, targetKeyspace, targetTableName); + } + + @Override + public void authorize(ClientState client) + { + client.ensureTablePermission(sourceKeyspace, sourceTableName, Permission.SELECT); + client.ensureAllTablesPermission(targetKeyspace, Permission.CREATE); + } + + @Override + public AuditLogContext getAuditLogContext() + { + return new AuditLogContext(AuditLogEntryType.CREATE_TABLE_LIKE, targetKeyspace, targetTableName); + } + + public String toString() + { + return String.format("CREATE TABLE (%s, %s) LIKE (%s, %s)", targetKeyspace, targetTableName, sourceKeyspace, sourceTableName); + } + + @Override + public void validate(ClientState state) + { + super.validate(state); + } + + @Override + public Keyspaces apply(ClusterMetadata metadata) + { + Keyspaces schema = metadata.schema.getKeyspaces(); + KeyspaceMetadata sourceKeyspaceMeta = schema.getNullable(sourceKeyspace); + TableMetadata sourceTableMeta = sourceKeyspaceMeta.getTableNullable(sourceTableName); + + if (null == sourceKeyspaceMeta) + throw ire("Source Keyspace '%s' doesn't exist", sourceKeyspace); + + if (!sourceKeyspaceMeta.hasTable(sourceTableName)) + { + throw ire("Souce Table '%s'.'%s' doesn't exist", sourceKeyspace, sourceTableName); + } + + if (sourceTableMeta.isIndex()) + throw ire("Cannot use CTREAT TABLE LIKE on a index table '%s'.'%s'.", sourceKeyspace, sourceTableName); + + if (sourceTableMeta.isView()) + throw ire("Cannot use CTREAT TABLE LIKE on a materialized view '%s'.'%s'.", sourceKeyspace, sourceTableName); + + KeyspaceMetadata targetKeyspaceMeta = schema.getNullable(targetKeyspace); + if (null == targetKeyspaceMeta) + throw ire("Target Keyspace '%s' doesn't exist", targetKeyspace); + + if (targetKeyspaceMeta.hasTable(targetTableName)) Review Comment: How about `IF NOT EXISTS` ########## test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java: ########## @@ -386,4 +386,47 @@ public void testWarnings() throws Throwable res = executeNet("REVOKE SELECT, MODIFY ON KEYSPACE revoke_yeah FROM " + user); assertWarningsContain(res.getExecutionInfo().getWarnings(), "Role '" + user + "' was not granted MODIFY on <keyspace revoke_yeah>"); } + + @Test + public void testCreateTableLikeAuthorize() throws Throwable + { + useSuperUser(); + + // two keyspaces + executeNet("CREATE KEYSPACE ks1 WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'}"); + executeNet("CREATE KEYSPACE ks2 WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'}"); + executeNet("CREATE TABLE ks1.sourcetb (id int PRIMARY KEY, val text)"); + executeNet("CREATE USER '" + user + "' WITH PASSWORD '" + pass + "'"); + + // same keyspace + // has no select permission on source table + ResultSet res = executeNet("REVOKE SELECT ON TABLE ks1.sourcetb FROM " + user); + assertWarningsContain(res.getExecutionInfo().getWarnings(), "Role '" + user + "' was not granted SELECT on <table ks1.sourcetb>"); + + useUser(user, pass); + assertUnauthorizedQuery("User user has no SELECT permission on <table ks1.sourcetb> or any of its parents", + "CREATE TABLE ks1.targetTb like ks1.sourcetb"); Review Comment: Alternating between uppercase and lower case make the statement hard to read. It would be better if the "like" was also uppercase. Same thing for the other queries bellow. ########## src/java/org/apache/cassandra/cql3/statements/schema/CopyTableStatement.java: ########## @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.cassandra.cql3.statements.schema; + +import org.apache.cassandra.audit.AuditLogContext; +import org.apache.cassandra.audit.AuditLogEntryType; +import org.apache.cassandra.auth.Permission; +import org.apache.cassandra.cql3.CQLStatement; +import org.apache.cassandra.cql3.QualifiedName; +import org.apache.cassandra.db.guardrails.Guardrails; +import org.apache.cassandra.exceptions.AlreadyExistsException; +import org.apache.cassandra.schema.KeyspaceMetadata; +import org.apache.cassandra.schema.Keyspaces; +import org.apache.cassandra.schema.TableId; +import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.service.ClientState; +import org.apache.cassandra.service.reads.repair.ReadRepairStrategy; +import org.apache.cassandra.tcm.ClusterMetadata; +import org.apache.cassandra.transport.Event.SchemaChange; + +public final class CopyTableStatement extends AlterSchemaStatement +{ + private final String sourceKeyspace; + private final String sourceTableName; + private final String targetKeyspace; + private final String targetTableName; + + public CopyTableStatement(String sourceKeyspace, + String targetKeyspace, + String sourceTableName, + String targetTableName) + { + super(targetKeyspace); + this.sourceKeyspace = sourceKeyspace; + this.targetKeyspace = targetKeyspace; + this.sourceTableName = sourceTableName; + this.targetTableName = targetTableName; + } + + @Override + SchemaChange schemaChangeEvent(Keyspaces.KeyspacesDiff diff) + { + return new SchemaChange(SchemaChange.Change.CREATED, SchemaChange.Target.TABLE, targetKeyspace, targetTableName); + } + + @Override + public void authorize(ClientState client) + { + client.ensureTablePermission(sourceKeyspace, sourceTableName, Permission.SELECT); + client.ensureAllTablesPermission(targetKeyspace, Permission.CREATE); + } + + @Override + public AuditLogContext getAuditLogContext() + { + return new AuditLogContext(AuditLogEntryType.CREATE_TABLE_LIKE, targetKeyspace, targetTableName); + } + + public String toString() + { + return String.format("CREATE TABLE (%s, %s) LIKE (%s, %s)", targetKeyspace, targetTableName, sourceKeyspace, sourceTableName); + } + + @Override + public void validate(ClientState state) + { + super.validate(state); + } + + @Override + public Keyspaces apply(ClusterMetadata metadata) + { + Keyspaces schema = metadata.schema.getKeyspaces(); + KeyspaceMetadata sourceKeyspaceMeta = schema.getNullable(sourceKeyspace); + TableMetadata sourceTableMeta = sourceKeyspaceMeta.getTableNullable(sourceTableName); + + if (null == sourceKeyspaceMeta) + throw ire("Source Keyspace '%s' doesn't exist", sourceKeyspace); + + if (!sourceKeyspaceMeta.hasTable(sourceTableName)) + { + throw ire("Souce Table '%s'.'%s' doesn't exist", sourceKeyspace, sourceTableName); + } + + if (sourceTableMeta.isIndex()) + throw ire("Cannot use CTREAT TABLE LIKE on a index table '%s'.'%s'.", sourceKeyspace, sourceTableName); + + if (sourceTableMeta.isView()) + throw ire("Cannot use CTREAT TABLE LIKE on a materialized view '%s'.'%s'.", sourceKeyspace, sourceTableName); Review Comment: Typo CTREAT -> CREATE ########## test/unit/org/apache/cassandra/cql3/CQLTester.java: ########## @@ -1518,6 +1560,13 @@ protected static void assertNoWarningContains(List<String> warnings, String mess } } + protected void expectedFailure(final Class<? extends RequestValidationException> exceptionType, String statement, String errorMsg) + { + + assertThatExceptionOfType(exceptionType) + .isThrownBy(() -> createTableMayThrow(statement)) .withMessageContaining(errorMsg); Review Comment: The method seems to do the same as `assertInvalidThrowMessage(String errorMessage, Class<? extends Throwable> exception, String query, Object... values)` ########## test/unit/org/apache/cassandra/cql3/CQLTester.java: ########## @@ -1078,6 +1082,30 @@ protected String createTable(String keyspace, String query, String tableName) return currentTable; } + protected String createTableLike(String query, String sourceTable) + { + return createTableLike(query, sourceTable, KEYSPACE, null, KEYSPACE); + } + + protected String createTableLike(String query, String sourceTable, String sourceKeyspace, String targetKeyspace) + { + return createTableLike(query, sourceTable, sourceKeyspace, null, targetKeyspace); + } + + protected String createTableLike(String query, String sourceTable, String sourceKeyspace, String targetTable, String targetKeyspace) + { + if (!tables.contains(sourceTable)) + { + throw new IllegalArgumentException("Source table " + sourceTable + " is not exists"); Review Comment: _does_ not exist ########## test/unit/org/apache/cassandra/cql3/statements/DescribeStatementTest.java: ########## @@ -822,6 +823,61 @@ public void testDescribeWithCustomIndex() throws Throwable row(KEYSPACE_PER_TEST, "index", indexWithOptions, expectedIndexStmtWithOptions)); } + @Test + public void testDescribeCreateLikeTable() throws Throwable + { + requireNetwork(); + DatabaseDescriptor.setDynamicDataMaskingEnabled(true); + String souceTable = createTable(KEYSPACE_PER_TEST, + "CREATE TABLE %s (" + + " pk1 text, " + + " pk2 int MASKED WITH DEFAULT, " + + " ck1 int, " + + " ck2 double," + + " s1 float static, " + + " v1 int, " + + " v2 int, " + + "PRIMARY KEY ((pk1, pk2), ck1, ck2 ))"); + String targetTable = createTableLike("create table %s like %s", souceTable, KEYSPACE_PER_TEST, KEYSPACE_PER_TEST); + Pair<TableMetadata, TableMetadata> pair = assertTableMetaEquals(KEYSPACE_PER_TEST, KEYSPACE_PER_TEST, souceTable, targetTable); Review Comment: It could be replaced by: ``` String souceTable = createTable(KEYSPACE_PER_TEST, "CREATE TABLE %s (" + " pk1 text, " + " pk2 int MASKED WITH DEFAULT, " + " ck1 int, " + " ck2 double," + " s1 float static, " + " v1 int, " + " v2 int, " + "PRIMARY KEY ((pk1, pk2), ck1, ck2 ))"); TableMetadata sourceTableMetadata = currentTableMetadata(); String targetTable = createTableLike("create table %s like %s", souceTable, KEYSPACE_PER_TEST, KEYSPACE_PER_TEST); TableMetadata targetTableMetadata = currentTableMetadata(); assertTableMetaEquals(sourceTableMetadata, targetTableMetadata); ``` and `assertTableMetaEquals` can be moved to `DescribeStatementTest` and replaced by: ``` protected void assertTableMetaEquals(TableMetadata source, TableMetadata target) { assertNotNull(source); assertNotNull(target); assertTrue(source.equalsWithoutTableNameAndDropCns(target)); assertNotEquals(source.id, target.id); assertNotEquals(source.name, target.name); } ``` It will then allow to remove the `getTableMetadata` methods in CQLTester. ########## test/unit/org/apache/cassandra/cql3/CQLTester.java: ########## @@ -1078,6 +1082,30 @@ protected String createTable(String keyspace, String query, String tableName) return currentTable; } + protected String createTableLike(String query, String sourceTable) Review Comment: The method is not used. I would remove it. ########## test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java: ########## @@ -386,4 +386,47 @@ public void testWarnings() throws Throwable res = executeNet("REVOKE SELECT, MODIFY ON KEYSPACE revoke_yeah FROM " + user); assertWarningsContain(res.getExecutionInfo().getWarnings(), "Role '" + user + "' was not granted MODIFY on <keyspace revoke_yeah>"); } + + @Test + public void testCreateTableLikeAuthorize() throws Throwable + { + useSuperUser(); + + // two keyspaces + executeNet("CREATE KEYSPACE ks1 WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'}"); + executeNet("CREATE KEYSPACE ks2 WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'}"); + executeNet("CREATE TABLE ks1.sourcetb (id int PRIMARY KEY, val text)"); + executeNet("CREATE USER '" + user + "' WITH PASSWORD '" + pass + "'"); + + // same keyspace + // has no select permission on source table + ResultSet res = executeNet("REVOKE SELECT ON TABLE ks1.sourcetb FROM " + user); + assertWarningsContain(res.getExecutionInfo().getWarnings(), "Role '" + user + "' was not granted SELECT on <table ks1.sourcetb>"); + + useUser(user, pass); + assertUnauthorizedQuery("User user has no SELECT permission on <table ks1.sourcetb> or any of its parents", + "CREATE TABLE ks1.targetTb like ks1.sourcetb"); + + // has select permission on source table no create permission on target keyspace + useSuperUser(); + executeNet("GRANT SELECT ON TABLE ks1.sourcetb TO " + user); + res = executeNet("REVOKE CREATE ON KEYSPACE ks1 FROM " + user); + assertWarningsContain(res.getExecutionInfo().getWarnings(), "Role '" + user + "' was not granted CREATE on <keyspace ks1>"); + + useUser(user, pass); + assertUnauthorizedQuery("User user has no CREATE permission on <all tables in ks1> or any of its parents", + "CREATE TABLE ks1.targetTb like ks1.sourcetb"); + + // different keyspaces + // has select permission on source table no create permission on target keyspace + useSuperUser(); + executeNet("GRANT SELECT ON TABLE ks1.sourcetb TO " + user); + res = executeNet("REVOKE CREATE ON KEYSPACE ks2 FROM " + user); + assertWarningsContain(res.getExecutionInfo().getWarnings(), "Role '" + user + "' was not granted CREATE on <keyspace ks2>"); + + useUser(user, pass); + assertUnauthorizedQuery("User user has no CREATE permission on <all tables in ks2> or any of its parents", + "CREATE TABLE ks2.targetTb like ks1.sourcetb"); + Review Comment: Can you also add some test where the source keyspace or table does not exist to see if the authorization framework handle them correctly. ########## src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java: ########## @@ -479,12 +479,21 @@ public String defaultCompactValueName() } } + public static TableMetadata.Builder parse(String cql, String keyspace, String table, Types types, UserFunctions userFunctions) Review Comment: I feel that I am missing something. Why did you change that code? ########## test/unit/org/apache/cassandra/cql3/CQLTester.java: ########## @@ -1543,6 +1578,16 @@ protected static ResultMessage schemaChange(String query) } } + protected TableMetadata getTableMetadata(String table) + { + return Schema.instance.getTableMetadata(KEYSPACE, table); + } + + protected TableMetadata getTableMetadata(String keyspace, String table) + { + return Schema.instance.getTableMetadata(keyspace, table); + } + Review Comment: Can be removed after the changes suggested in `DescribeStatementTest` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org For additional commands, e-mail: pr-h...@cassandra.apache.org
