[
https://issues.apache.org/jira/browse/CASSANDRA-8261?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14239977#comment-14239977
]
Tyler Hobbs commented on CASSANDRA-8261:
----------------------------------------
Overall it seems pretty reasonable so far. My review comments are mostly about
renaming things for clarity, because SchemaTables currently has a lot of
similarly-named things that are a bit confusing at first. Some terms (like
"serialize(d)") seem to be used to mean different things.
SchemaTables:
* {{serializedTables/UserTypes/UserFunctions()}}: need a short java doc, maybe
rename to {{getTableRowsForKeyspace()}}, etc
* {{readSchemaFromDisk()}}: maybe rename to {{readSchemaFromSytemTables()}}
* {{addToSchemaMutation()}}: I suggest renaming the overloaded methods that
apply to tables, columns, and triggers to: {{addTableToSchemaMutation()}},
{{addColumnToSchemaMutation()}}, and {{addTriggerToSchemaMutation()}}
* {{addToDropFromSchemaMutation()}}: rename overloaded methods to:
{{addTableDropToSchemaMutation()}}, {{addColumnDropToSchemaMutation()}}, etc.
The current name is really confusing :)
* {{toSchemaMutation()}}, {{toUpdateSchemaMutation()}},
{{toDropSchemaMutation()}}: maybe rename to {{makeDropSchemaMutation()}}, etc
* {{getSchema()}}: could use a javadoc
* {{serializedSchema()}}: rename to {{getSchemaTableRows()}}, improve javadoc
* {{serializeSchema()}}: rename to {{schemaTablesToMutation()}}, add javadoc
* {{serializeSchema(map, table)}}: rename to {{addSchemaTableToMutation()}},
add javadoc
SystemKeyspaces:
* There's a TODO about making BuiltIndexesTable private, and a related FIXME in
{{SchemaTables.toDropFromSchemaMutation()}}
> Clean up schema metadata classes
> --------------------------------
>
> Key: CASSANDRA-8261
> URL: https://issues.apache.org/jira/browse/CASSANDRA-8261
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Aleksey Yeschenko
> Assignee: Aleksey Yeschenko
> Priority: Minor
> Fix For: 3.0
>
> Attachments: 8261-isolate-hadcoded-system-tables.txt,
> 8261-isolate-serialization-code.txt, 8261-isolate-thrift-code.txt
>
>
> While working on CASSANDRA-6717, I've made some general cleanup changes to
> schema metadata classes - distracted from the core purpose. Also, being
> distracted from it by other things, every time I come back to it gives me a
> bit of a rebase hell.
> Thus I'm isolating those changes into a separate issue here, hoping to commit
> them one by one, before I go back and finalize CASSANDRA-6717.
> The changes include:
> - moving all the toThrift/fromThrift conversion code to ThriftConversion,
> where it belongs
> - moving the complied system CFMetaData objects away from CFMetaData (to
> SystemKeyspace and TracesKeyspace)
> - isolating legacy toSchema/fromSchema code into a separate class
> (LegacySchemaTables - former DefsTables)
> - refactoring CFMetaData/KSMetaData fields to match CQL CREATE TABLE syntax,
> and encapsulating more things in
> CompactionOptions/CompressionOptions/ReplicationOptions classes
> - moving the definition classes to the new 'schema' package
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
