CASSANDRA-19664 first approach - for copying Basic Table definition #3718
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| /* | ||
| * 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.Indexes; | ||
| 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.schema.TableParams; | ||
| import org.apache.cassandra.schema.Triggers; | ||
| import org.apache.cassandra.schema.UserFunctions; | ||
| 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; | ||
|
|
||
| /** | ||
| * {@code CREATE TABLE [IF NOT EXISTS] <newtable> LIKE <oldtable> WITH <property> = <value>} | ||
|
There was a problem hiding this comment. @blerer this is a brief introduction of cql to CopyTableStatement, and I have already create CASSANDRA-20120 to create CEP-43's doc |
||
| */ | ||
| public final class CopyTableStatement extends AlterSchemaStatement | ||
Maxwell-Guo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| private final String sourceKeyspace; | ||
| private final String sourceTableName; | ||
| private final String targetKeyspace; | ||
| private final String targetTableName; | ||
| private final boolean ifNotExists; | ||
| private final TableAttributes attrs; | ||
|
|
||
| public CopyTableStatement(String sourceKeyspace, | ||
| String targetKeyspace, | ||
| String sourceTableName, | ||
| String targetTableName, | ||
| boolean ifNotExists, | ||
| TableAttributes attrs) | ||
| { | ||
| super(targetKeyspace); | ||
| this.sourceKeyspace = sourceKeyspace; | ||
| this.targetKeyspace = targetKeyspace; | ||
| this.sourceTableName = sourceTableName; | ||
| this.targetTableName = targetTableName; | ||
| this.ifNotExists = ifNotExists; | ||
| this.attrs = attrs; | ||
| } | ||
|
|
||
| @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); | ||
| } | ||
|
|
||
| @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 (null == sourceTableMeta) | ||
| throw ire("Souce Table '%s'.'%s' doesn't exist", sourceKeyspace, sourceTableName); | ||
|
|
||
| if (sourceTableMeta.isIndex()) | ||
| throw ire("Cannot use CREATE TABLE LIKE on a index table '%s'.'%s'.", sourceKeyspace, sourceTableName); | ||
|
|
||
| if (sourceTableMeta.isView()) | ||
| throw ire("Cannot use CREATE 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)) | ||
Maxwell-Guo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| if(ifNotExists) | ||
| return schema; | ||
|
|
||
| throw new AlreadyExistsException(targetKeyspace, targetTableName); | ||
| } | ||
| // todo support udt for differenet ks latter | ||
| if (!sourceKeyspace.equalsIgnoreCase(targetKeyspace) && !sourceKeyspaceMeta.types.isEmpty()) | ||
| throw ire("Cannot use CREATE TABLE LIKE across different keyspace when source table have UDTs."); | ||
|
|
||
| String sourceCQLString = sourceTableMeta.toCqlString(false, false, true, false); | ||
|
There was a problem hiding this comment. we ignore the dropped columns, if the seconds input argument is true that means the dropped column will be consider . |
||
| // add all user functions to be able to give a good error message to the user if the alter references | ||
| // a function from another keyspace | ||
| UserFunctions.Builder ufBuilder = UserFunctions.builder().add(); | ||
| for (KeyspaceMetadata ksm : schema) | ||
| ufBuilder.add(ksm.userFunctions); | ||
|
|
||
| TableMetadata.Builder targetBuilder = CreateTableStatement.parse(sourceCQLString, | ||
| targetKeyspace, | ||
| targetTableName, | ||
| sourceKeyspaceMeta.types, | ||
| ufBuilder.build()) | ||
| .indexes(Indexes.none()) | ||
| .triggers(Triggers.none()); | ||
|
|
||
| TableParams originalParams = targetBuilder.build().params; | ||
| TableParams newTableParams = attrs.asAlteredTableParams(originalParams); | ||
| TableMetadata table = targetBuilder.params(newTableParams) | ||
| .id(TableId.get(metadata)) | ||
| .build(); | ||
| table.validate(); | ||
|
|
||
| if (targetKeyspaceMeta.replicationStrategy.hasTransientReplicas() | ||
|
There was a problem hiding this comment. @Maxwell-Guo This is interesting corner-case. I think that if there was not a possibility to specify different source keyspace from the target one, then we would not need this ... Hm. I think it needs to be there then. There was a problem hiding this comment. My idea at the time was that copying a table is also an act of creating a new table, the only difference is that target table has same schema with source table. We have no way to control the target keyspace because the keyspaces of the two tables may be the same or completely different. So I kept it. |
||
| && table.params.readRepair != ReadRepairStrategy.NONE) | ||
| { | ||
| throw ire("read_repair must be set to 'NONE' for transiently replicated keyspaces"); | ||
| } | ||
|
|
||
| if (!table.params.compression.isEnabled()) | ||
|
There was a problem hiding this comment. So this says that: "if the compression is not enabled on the source table, then be sure that guardrail enables uncompressed sstables". While it does make sense when creating the original table for the first time, does it still make sense to do when we are copying it? If we disabled the compression on source table, then I think that we already do have a guardrail for that, correct? But if we change the guardrail in runtime after the original table was created (e.g. via JMX), then copying such table will not be possible anymore? So, whether we can copy a table or not when the source table has disabled compression only depends on what the guardrail is set to in the runtime. I prefer to not check this. I would rather create a copy of the source table no matter what and we can propagate back to the client (the message will appear in cqlsh when copying table) that if guardrail is different, then a user might notified about that, but this is optional. There was a problem hiding this comment.
Yes, that is why I kept it. The interval between creating two tables may be long, and it is uncertain whether the user has any new ideas about table compression during this period. Therefore, for the behavior of copying the table, except for copying the table schema(column, datatype, mask, table params), creating udt, index, trigger (if needed), and do some validations on udt、indexes and triggers, I have aligned other behaviors and validations with creating a new table. There was a problem hiding this comment. Thinking about it again, I think how it is done is OK, because we can also do |
||
| Guardrails.uncompressedTablesEnabled.ensureEnabled(state); | ||
|
|
||
| return schema.withAddedOrUpdated(targetKeyspaceMeta.withSwapped(targetKeyspaceMeta.tables.with(table))); | ||
| } | ||
|
|
||
| public final static class Raw extends CQLStatement.Raw | ||
| { | ||
| private final QualifiedName oldName; | ||
| private final QualifiedName newName; | ||
| private final boolean ifNotExists; | ||
| public final TableAttributes attrs = new TableAttributes(); | ||
|
|
||
| public Raw(QualifiedName newName, QualifiedName oldName, boolean ifNotExists) | ||
| { | ||
| this.newName = newName; | ||
| this.oldName = oldName; | ||
| this.ifNotExists = ifNotExists; | ||
| } | ||
|
|
||
| @Override | ||
| public CQLStatement prepare(ClientState state) | ||
| { | ||
| String oldKeyspace = oldName.hasKeyspace() ? oldName.getKeyspace() : state.getKeyspace(); | ||
| String newKeyspace = newName.hasKeyspace() ? newName.getKeyspace() : state.getKeyspace(); | ||
| return new CopyTableStatement(oldKeyspace, newKeyspace, oldName.getName(), newName.getName(), ifNotExists, attrs); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -479,12 +479,21 @@ public String defaultCompactValueName() | |
| } | ||
| } | ||
|
|
||
| public static TableMetadata.Builder parse(String cql, String keyspace, String table, Types types, UserFunctions userFunctions) | ||
|
There was a problem hiding this comment. I feel that I am missing something. Why did you change that code? There was a problem hiding this comment. @blerer the original version of that method (which calls this one) is still there. We just create this one in order to be able to specify custom table name with types and functions. If we parse CQL for old table, we just want to be able to put there table name etc for new table directly upon parsing. |
||
| { | ||
| Raw createTable = CQLFragmentParser.parseAny(CqlParser::createTableStatement, cql, "CREATE TABLE") | ||
| .keyspace(keyspace); | ||
|
|
||
| if (table != null) | ||
| createTable.table(table); | ||
|
|
||
| return createTable.prepare(null) // works around a messy ClientState/QueryProcessor class init deadlock | ||
| .builder(types, userFunctions); | ||
| } | ||
|
|
||
| public static TableMetadata.Builder parse(String cql, String keyspace) | ||
| { | ||
| return parse(cql, keyspace, null, Types.none(), UserFunctions.none()); | ||
| } | ||
|
|
||
| public final static class Raw extends CQLStatement.Raw | ||
|
|
@@ -538,6 +547,12 @@ public Raw keyspace(String keyspace) | |
| return this; | ||
| } | ||
|
|
||
| public Raw table(String table) | ||
| { | ||
| name.setName(table, true); | ||
| return this; | ||
| } | ||
|
|
||
| public String table() | ||
| { | ||
| return name.getName(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason i used a new statement here is that :
1、The original createtablestatment code is too bloated. If the semantics of like are added, the code needs to be changed a lot.
2、In addition, I think this new copystatement can do things other than table cloning, and can also support semantic capabilities such as “CREATE TABLE AS”. Because they are similar to table copying functions, they are called copystatement here.
Besides, I have also prepared a patch, which is mainly to modify the pares of create table, that is, to implement the function of create table like in create table statement. If you think it is necessary, I can create a new PR to replace the current one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is another pr here that implement CASSANDRA-19964, but use create table's parase path .
I am not sure which one is better , so I attach both of them.