Merge lp:~nwilliams/akiban-server/remove-get-table-definition into lp:~akiban-technologies/akiban-server/trunk

[Nathan Williams]

Remove TableDefinition and creation from SchemaManager.

Hasn't been used since the adapter started generating its own.

[Mike McMahon (mmcm)]

As described.

review: Approve

=== modified file 'src/main/java/com/akiban/server/store/AbstractSchemaManager.java'
--- src/main/java/com/akiban/server/store/AbstractSchemaManager.java	2013-07-25 16:10:58 +0000
+++ src/main/java/com/akiban/server/store/AbstractSchemaManager.java	2013-07-29 14:51:38 +0000
@@ -385,52 +385,6 @@
     }
 
     @Override
-    public TableDefinition getTableDefinition(Session session, TableName tableName) {
-        final Table table = getAis(session).getTable(tableName);
-        if(table == null) {
-            throw new NoSuchTableException(tableName);
-        }
-        final String ddl = new DDLGenerator().createTable(table);
-        return new TableDefinition(table.getTableId(), tableName, ddl);
-    }
-
-    @Override
-    public SortedMap<String, TableDefinition> getTableDefinitions(Session session, String schemaName) {
-        final SortedMap<String, TableDefinition> result = new TreeMap<>();
-        final DDLGenerator gen = new DDLGenerator();
-        for(UserTable table : getAis(session).getUserTables().values()) {
-            final TableName name = table.getName();
-            if(name.getSchemaName().equals(schemaName)) {
-                final String ddl = gen.createTable(table);
-                final TableDefinition def = new TableDefinition(table.getTableId(), name, ddl);
-                result.put(name.getTableName(), def);
-            }
-        }
-        return result;
-    }
-
-    @Override
-    public List<String> schemaStrings(Session session, boolean withISTables) {
-        final AkibanInformationSchema ais = getAis(session);
-        final DDLGenerator generator = new DDLGenerator();
-        final List<String> ddlList = new ArrayList<>();
-        for(Schema schema : ais.getSchemas().values()) {
-            if(!withISTables &&
-                    (TableName.INFORMATION_SCHEMA.equals(schema.getName()) ||
-                            TableName.SECURITY_SCHEMA.equals(schema.getName()) ||
-                            TableName.SYS_SCHEMA.equals(schema.getName()) ||
-                            TableName.SQLJ_SCHEMA.equals(schema.getName()))) {
-                continue;
-            }
-            ddlList.add(String.format(CREATE_SCHEMA_FORMATTER, schema.getName()));
-            for(UserTable table : schema.getUserTables().values()) {
-                ddlList.add(generator.createTable(table));
-            }
-        }
-        return ddlList;
-    }
-
-    @Override
     public void createView(Session session, View view) {
         final AkibanInformationSchema oldAIS = getAis(session);
         checkSystemSchema(view.getName(), false);

=== modified file 'src/main/java/com/akiban/server/store/SchemaManager.java'
--- src/main/java/com/akiban/server/store/SchemaManager.java	2013-07-25 16:10:58 +0000
+++ src/main/java/com/akiban/server/store/SchemaManager.java	2013-07-29 14:51:38 +0000
@@ -142,23 +142,6 @@
     void alterSequence(Session session, TableName sequenceName, Sequence newDefinition);
 
     /**
-     * Generate a TableDefinition, which includes a canonical 'create table' statement,
-     * schema name, table name, and table ID, for the given table.
-     * @param session Session to operate under.
-     * @param tableName The name of the requested table.
-     * @return Filled in TableDefinition.
-     */
-    TableDefinition getTableDefinition(Session session, TableName tableName);
-
-    /**
-     * Generate a 'create table' DDL statement for each table in the given schema.
-     * @param session Session to operate under.
-     * @param schemaName Schema to to query.
-     * @return Map, keyed by table name, of all TableDefinitions.
-     */
-    SortedMap<String, TableDefinition> getTableDefinitions(Session session, String schemaName);
-
-    /**
      * Returns the current and authoritative AIS, containing all metadata about
      * all known for the running system.
      * @param session Session to operate under.
@@ -166,17 +149,6 @@
      */
     AkibanInformationSchema getAis(Session session);
 
-    /**
-     * Generate DDL statements for every schema and table.
-     * The format of the 'create schema' contains if not exists and will occur before
-     * any table in that schema. No other guarantees are given about ordering.
-     *
-     * @param session The Session to operate under.
-     * @param withISTables  true, include 'create table' statements for tables in the I_S.
-     * @return List of every create statement request.
-     */
-    List<String> schemaStrings(Session session, boolean withISTables);
-
     /** Add the given view to the current AIS. */
     void createView(Session session, View view);
 

=== removed file 'src/main/java/com/akiban/server/store/TableDefinition.java'
--- src/main/java/com/akiban/server/store/TableDefinition.java	2013-03-22 20:05:57 +0000
+++ src/main/java/com/akiban/server/store/TableDefinition.java	1970-01-01 00:00:00 +0000
@@ -1,60 +0,0 @@
-/**
- * Copyright (C) 2009-2013 Akiban Technologies, Inc.
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-package com.akiban.server.store;
-
-import com.akiban.ais.model.TableName;
-
-public class TableDefinition {
-    private final int tableId;
-    private final String schemaName;
-    private final String tableName;
-    private final String ddl;
-
-    public TableDefinition(int tableId, String schemaName, String tableName, String ddl) {
-        this.tableId = tableId;
-        this.schemaName = schemaName;
-        this.tableName = tableName;
-        this.ddl = ddl;
-    }
-
-    public TableDefinition(int tableId, TableName name, String ddl) {
-        this(tableId, name.getSchemaName(), name.getTableName(), ddl);
-    }
-
-    public String getDDL() {
-        return ddl;
-    }
-
-    public String getSchemaName() {
-        return schemaName;
-    }
-
-    public String getTableName() {
-        return tableName;
-    }
-
-    public int getTableId() {
-        return tableId;
-    }
-
-    @Override
-    public String toString() {
-        return "TableDefinition[" + tableId + ": "
-                + TableName.create(schemaName, tableName) + ']';
-    }
-}
\ No newline at end of file

=== modified file 'src/test/java/com/akiban/server/service/is/BasicInfoSchemaTablesServiceImplTest.java'
--- src/test/java/com/akiban/server/service/is/BasicInfoSchemaTablesServiceImplTest.java	2013-07-25 16:10:58 +0000
+++ src/test/java/com/akiban/server/service/is/BasicInfoSchemaTablesServiceImplTest.java	2013-07-29 14:51:38 +0000
@@ -39,7 +39,6 @@
 import com.akiban.server.service.security.SecurityService;
 import com.akiban.server.service.session.Session;
 import com.akiban.server.store.SchemaManager;
-import com.akiban.server.store.TableDefinition;
 import com.akiban.server.types.AkType;
 import com.akiban.server.types.ValueSource;
 import org.junit.Before;
@@ -54,7 +53,6 @@
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
-import java.util.SortedMap;
 
 import static com.akiban.qp.memoryadapter.MemoryGroupCursor.GroupScan;
 import static com.akiban.server.types.AkType.*;
@@ -725,21 +723,6 @@
         }
 
         @Override
-        public TableDefinition getTableDefinition(Session session, TableName tableName) {
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public SortedMap<String, TableDefinition> getTableDefinitions(Session session, String schemaName) {
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public List<String> schemaStrings(Session session, boolean withISTables) {
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
         public void createView(Session session, View view) {
             throw new UnsupportedOperationException();
         }

=== modified file 'src/test/java/com/akiban/server/test/it/dxl/AtomicSchemaChangesIT.java'
--- src/test/java/com/akiban/server/test/it/dxl/AtomicSchemaChangesIT.java	2013-03-22 20:05:57 +0000
+++ src/test/java/com/akiban/server/test/it/dxl/AtomicSchemaChangesIT.java	2013-07-29 14:51:38 +0000
@@ -18,16 +18,17 @@
 package com.akiban.server.test.it.dxl;
 
 import com.akiban.ais.model.AkibanInformationSchema;
+import com.akiban.ais.model.UserTable;
 import com.akiban.ais.protobuf.ProtobufWriter;
+import com.akiban.ais.util.DDLGenerator;
 import com.akiban.server.error.UnsupportedIndexSizeException;
-import com.akiban.server.store.SchemaManager;
-import com.akiban.server.store.TableDefinition;
 import com.akiban.server.test.it.ITBase;
 import com.akiban.util.GrowableByteBuffer;
 import org.junit.Assert;
 import org.junit.Test;
 
 import java.util.Map;
+import java.util.TreeMap;
 import java.util.concurrent.Callable;
 
 import static junit.framework.Assert.*;
@@ -181,9 +182,9 @@
 
     private void checkInitialDDL() throws Exception
     {
-        for (Map.Entry<String, TableDefinition> entry : createTableStatements("s").entrySet()) {
+        for (Map.Entry<String, String> entry : createTableStatements("s").entrySet()) {
             String table = entry.getKey();
-            String ddl = entry.getValue().getDDL();
+            String ddl = entry.getValue();
             if (table.equals("parent")) {
                 assertEquals(PARENT_DDL, ddl);
             } else if (table.equals("child")) {
@@ -202,12 +203,17 @@
         return buffer;
     }
 
-    private Map<String, TableDefinition> createTableStatements(final String schema) throws Exception
+    private Map<String, String> createTableStatements(final String schema) throws Exception
     {
-        return transactionallyUnchecked(new Callable<Map<String, TableDefinition>>() {
+        return transactionallyUnchecked(new Callable<Map<String, String>>() {
             @Override
-            public Map<String, TableDefinition> call() throws Exception {
-                return serviceManager().getServiceByClass(SchemaManager.class).getTableDefinitions(session(), schema);
+            public Map<String, String> call() throws Exception {
+                DDLGenerator generator = new DDLGenerator();
+                Map<String, String> map = new TreeMap<>();
+                for(UserTable table : ais().getSchema(schema).getUserTables().values()) {
+                    map.put(table.getName().getTableName(), generator.createTable(table));
+                }
+                return map;
             }
         });
     }

=== modified file 'src/test/java/com/akiban/server/test/it/store/SchemaManagerIT.java'
--- src/test/java/com/akiban/server/test/it/store/SchemaManagerIT.java	2013-07-25 18:32:49 +0000
+++ src/test/java/com/akiban/server/test/it/store/SchemaManagerIT.java	2013-07-29 14:51:38 +0000
@@ -33,18 +33,19 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.SortedMap;
 import java.util.SortedSet;
 import java.util.TreeSet;
 import java.util.concurrent.Callable;
 
 import com.akiban.ais.model.Index;
 import com.akiban.ais.model.Routine;
+import com.akiban.ais.model.Schema;
 import com.akiban.ais.model.Sequence;
 import com.akiban.ais.model.Table;
 import com.akiban.ais.model.TableName;
 import com.akiban.ais.model.aisb2.AISBBasedBuilder;
 import com.akiban.ais.model.aisb2.NewAISBuilder;
+import com.akiban.ais.util.DDLGenerator;
 import com.akiban.qp.expression.IndexKeyRange;
 import com.akiban.qp.memoryadapter.MemoryAdapter;
 import com.akiban.qp.memoryadapter.MemoryGroupCursor;
@@ -60,7 +61,6 @@
 import com.akiban.server.error.NoSuchTableException;
 import com.akiban.server.service.session.Session;
 import com.akiban.server.store.SchemaManager;
-import com.akiban.server.store.TableDefinition;
 import com.akiban.server.store.statistics.IndexStatistics;
 import com.akiban.server.test.it.ITBase;
 import org.junit.Assert;
@@ -115,22 +115,6 @@
         });
     }
 
-    private TableDefinition getTableDef(final String schema, final String table) throws Exception {
-        return transactionally(new Callable<TableDefinition>() {
-            public TableDefinition call() throws Exception {
-                return schemaManager.getTableDefinition(session(), new TableName(schema, table));
-            }
-        });
-    }
-
-    private List<String> getSchemaStringsWithoutAIS() throws Exception {
-        return transactionally(new Callable<List<String>>() {
-            public List<String> call() throws Exception {
-                return schemaManager.schemaStrings(session(), false);
-            }
-        });
-    }
-
     private void safeRestart() throws Exception {
         safeRestartTestServices();
         schemaManager = serviceManager().getSchemaManager();
@@ -155,36 +139,6 @@
         assertTablesInSchema(SCHEMA);
     }
 
-    @Test(expected=NoSuchTableException.class)
-    public void getUnknownTableDefinition() throws Exception {
-        getTableDef("fooschema", "bartable1");
-    }
-
-    // Also tests createDef(), but assertTablesInSchema() uses getDef() so try and test first
-    @Test
-    public void getTableDefinition() throws Exception {
-        createTableDef(SCHEMA, T1_NAME, T1_DDL);
-        final TableDefinition def = getTableDef(SCHEMA, T1_NAME);
-        assertNotNull("Definition exists", def);
-    }
-
-    @Test
-    public void getTableDefinitionsUnknownSchema() throws Exception {
-        final SortedMap<String, TableDefinition> defs = getTableDefinitions("fooschema");
-        assertEquals("no defs", 0, defs.size());
-    }
-
-    @Test
-    public void getTableDefinitionsOneSchema() throws Exception {
-        createTableDef(SCHEMA, T1_NAME, T1_DDL);
-        createTableDef(SCHEMA, T2_NAME, T2_DDL);
-        createTableDef(SCHEMA + "_bob", T1_NAME, T1_DDL);
-        final SortedMap<String, TableDefinition> defs = getTableDefinitions(SCHEMA);
-        assertTrue("contains t1", defs.containsKey(T1_NAME));
-        assertTrue("contains t2", defs.containsKey(T2_NAME));
-        assertEquals("no defs", 2, defs.size());
-    }
-
     @Test
     public void createOneDefinition() throws Exception {
         createTableDef(SCHEMA, T1_NAME, T1_DDL);
@@ -335,45 +289,6 @@
         final long second = ais().getGeneration();
         assertTrue("timestamp changed", first != second);
     }
-    
-    @Test
-    public void schemaStringsSingleTable() throws Exception {
-        // Check 1) basic ordering 2) that the statements are 'canonicalized'
-        final String TABLE_DDL =  "id int not null primary key";
-        final String SCHEMA_DDL = "create schema if not exists `foo`;";
-        final String TABLE_CANONICAL = "create table `foo`.`bar`(`id` int NOT NULL, PRIMARY KEY(`id`)) engine=akibandb DEFAULT CHARSET=utf8 COLLATE=utf8_bin";
-        createTableDef("foo", "bar", TABLE_DDL);
-        final List<String> ddls = getSchemaStringsWithoutAIS();
-        assertEquals("ddl count", 2, ddls.size()); // schema and table
-        assertTrue("create schema", ddls.get(0).startsWith("create schema"));
-        assertEquals("create schema is canonical", SCHEMA_DDL, ddls.get(0));
-        assertTrue("create table second", ddls.get(1).startsWith("create table"));
-        assertEquals("create table is canonical", TABLE_CANONICAL, ddls.get(1));
-    }
-
-    @Test
-    public void schemaStringsSingleGroup() throws Exception {
-        final String SCHEMA = "s1";
-        createTableDef(SCHEMA, T1_NAME, T1_DDL);
-        createTableDef(SCHEMA, T3_CHILD_T1_NAME, T3_CHILD_T1_DDL);
-        Map<String, List<String>> schemaAndTables = new HashMap<>();
-        schemaAndTables.put(SCHEMA, Arrays.asList(T1_NAME, T3_CHILD_T1_NAME));
-        assertSchemaStrings(schemaAndTables);
-    }
-
-    @Test
-    public void schemaStringsMultipleSchemas() throws Exception {
-        final Map<String, List<String>> schemaAndTables = new HashMap<>();
-        schemaAndTables.put("s1", Arrays.asList("t1", "t2"));
-        schemaAndTables.put("s2", Arrays.asList("t3"));
-        schemaAndTables.put("s3", Arrays.asList("t4"));
-        for(Map.Entry<String, List<String>> entry : schemaAndTables.entrySet()) {
-            for(String table : entry.getValue()) {
-                createTableDef(entry.getKey(), table, "id int not null primary key");
-            }
-        }
-        assertSchemaStrings(schemaAndTables);
-    }
 
     @Test
     public void manyTablesAndRestart() throws Exception {
@@ -505,13 +420,14 @@
                 "REFERENCES `index_statistics`(`table_id`, `index_id`)"+
         ") engine=akibandb DEFAULT CHARSET=utf8 COLLATE=utf8_bin";
 
-        TableDefinition statsDef = getTableDef(SCHEMA, STATS_TABLE);
-        assertNotNull("Stats table present", statsDef);
-        assertEquals("Stats DDL", STATS_DDL, statsDef.getDDL());
+        DDLGenerator generator = new DDLGenerator();
+        UserTable statsTable = ais().getUserTable(SCHEMA, STATS_TABLE);
+        assertNotNull("Stats table present", statsTable);
+        assertEquals("Stats DDL", STATS_DDL, generator.createTable(statsTable));
 
-        TableDefinition entryDef = getTableDef(SCHEMA, ENTRY_TABLE);
-        assertNotNull("Entry table present", entryDef);
-        assertEquals("Entry DDL", ENTRY_DDL, entryDef.getDDL());
+        UserTable entryTable = ais().getUserTable(SCHEMA, ENTRY_TABLE);
+        assertNotNull("Entry table present", entryTable);
+        assertEquals("Entry DDL", ENTRY_DDL, generator.createTable(entryTable));
     }
 
     @Test
@@ -789,62 +705,15 @@
         for (String name : tableNames) {
             final Table table = ais.getTable(schema, name);
             assertNotNull(schema + "." + name + " in AIS", table);
-            final TableDefinition def = getTableDefinitions(schema).get(table.getName().getTableName());
-            assertNotNull(schema + "." + name  + " has definition", def);
             expected.add(name);
         }
         final SortedSet<String> actual = new TreeSet<>();
-        for (TableDefinition def : getTableDefinitions(schema).values()) {
-            final Table table = ais.getTable(schema, def.getTableName());
-            assertNotNull(def + " in AIS", table);
-            actual.add(def.getTableName());
+        Schema schemaObj = ais.getSchema(schema);
+        if(schemaObj != null) {
+            actual.addAll(schemaObj.getUserTables().keySet());
         }
         assertEquals("tables in: " + schema, expected, actual);
     }
-    
-    /**
-     * Check that the result of {@link SchemaManager#schemaStrings(Session, boolean)} is correct for
-     * the given tables. The only guarantees are that schemas are created with 'if not exists',
-     * a schema statement comes before any table in it, and a create table statement is fully qualified.
-     * @param schemaAndTables Map of schema names to table names that should exist
-     * @throws Exception For any internal error.
-     */
-    private void assertSchemaStrings(Map<String, List<String>> schemaAndTables) throws Exception {
-        final String CREATE_SCHEMA = "create schema if not exists `";
-        final String CREATE_TABLE = "create table `";
-        final List<String> ddls = getSchemaStringsWithoutAIS();
-        final Set<String> sawSchemas = new HashSet<>();
-        for(String statement : ddls) {
-            if(statement.startsWith(CREATE_SCHEMA)) {
-                final int offset = CREATE_SCHEMA.length();
-                final String schemaName = statement.substring(offset, offset + 2);
-                assertFalse("haven't seen schema "+schemaName,
-                            sawSchemas.contains(schemaName));
-                sawSchemas.add(schemaName);
-            }
-            else if(statement.startsWith(CREATE_TABLE)){
-                final int offset = CREATE_TABLE.length();
-                final String schemaName = statement.substring(offset, offset + 2);
-                assertTrue("schema "+schemaName+" has been seen",
-                           sawSchemas.contains(schemaName));
-                final String tableName = statement.substring(offset+5, offset+7);
-                assertTrue("table "+tableName+" is in schema "+tableName,
-                           schemaAndTables.get(schemaName).contains(tableName));
-            }
-            else {
-                Assert.fail("Unknown statement type: " + statement);
-            }
-        }
-    }
-
-    private SortedMap<String, TableDefinition> getTableDefinitions(final String schema) {
-        return transactionallyUnchecked(new Callable<SortedMap<String, TableDefinition>>() {
-            @Override
-            public SortedMap<String, TableDefinition> call() throws Exception {
-                return schemaManager.getTableDefinitions(session(), schema);
-            }
-        });
-    }
 
     private static UserTable makeSimpleISTable(TableName name) {
         NewAISBuilder builder = AISBBasedBuilder.create(name.getSchemaName());