----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18180/#review35137 -----------------------------------------------------------
Hi Yongjun. Thank you for your contribution. Bug-fix updated nicely, but codes for drop table have some problems. So, I commented for it. Cheers. tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/AbstractDBStore.java <https://reviews.apache.org/r/18180/#comment65552> If this method would be called, TajoMaster must shut down with CatalogException message written by you. But current version fail at needUpgrade method. So, I recommend you to update your patch as follows: - remove LOG.error - add throw new CatalogException (after dropBaseTable) tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/DerbyStore.java <https://reviews.apache.org/r/18180/#comment65553> I think that drop table code size is long. BaseTableMaps has table names which already was created. How about iterating over a Map EntrySet as follows? for(Map.Entry<String,Boolean> entry: baseTableMaps.entrySet()) { tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/DerbyStore.java <https://reviews.apache.org/r/18180/#comment65555> You'd better remove toUpperCase. Current catalog table names be written in upper case. So, toUpperCase doesn't matter. But if we rename it in lower case, it will be lack. tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/MySQLStore.java <https://reviews.apache.org/r/18180/#comment65554> Please refer my comments for DerbyStore. tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/MySQLStore.java <https://reviews.apache.org/r/18180/#comment65556> Please refer my comments for DerbyStore. - Jung JaeHwa On Feb. 21, 2014, 6:06 a.m., Yongjun Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18180/ > ----------------------------------------------------------- > > (Updated Feb. 21, 2014, 6:06 a.m.) > > > Review request for Tajo. > > > Bugs: TAJO-594 > https://issues.apache.org/jira/browse/TAJO-594 > > > Repository: tajo > > > Description > ------- > > When I test MySQL catalog store instead of Derby, I get an error below. We > have to fix it. > > {noformat} > 2014-02-12 18:27:49,608 ERROR catalog.CatalogServer > (CatalogServer.java:addTable(255)) - Unknown column 'TID' in 'field list' > java.io.IOException: Unknown column 'TID' in 'field list > at > org.apache.tajo.catalog.store.AbstractDBStore.addTable(AbstractDBStore.java:326) > at > org.apache.tajo.catalog.CatalogServer$CatalogProtocolHandler.addTable(CatalogServer.java:253) > at > org.apache.tajo.catalog.AbstractCatalogClient$6.call(AbstractCatalogClient.java:161) > at > org.apache.tajo.catalog.AbstractCatalogClient$6.call(AbstractCatalogClient.java:158) > at org.apache.tajo.rpc.ServerCallable.withRetries(ServerCallable.java:96) > at > org.apache.tajo.catalog.AbstractCatalogClient.addTable(AbstractCatalogClient.java:158) > at > org.apache.tajo.master.GlobalEngine.createTableOnPath(GlobalEngine.java:317) > at org.apache.tajo.master.GlobalEngine.createTable(GlobalEngine.java:280) > at org.apache.tajo.master.GlobalEngine.updateQuery(GlobalEngine.java:221) > at org.apache.tajo.master.GlobalEngine.executeQuery(GlobalEngine.java:142) > at > org.apache.tajo.master.TajoMasterClientService$TajoMasterClientProtocolServiceHandler.submitQuery(TajoMasterClientService.java:158) > at > org.apache.tajo.ipc.TajoMasterClientProtocol$TajoMasterClientProtocolService$2.callBlockingMethod(TajoMasterClientProtocol.java:289) > {noformat} > > > Diffs > ----- > > > tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/AbstractDBStore.java > 8c2c0c3 > > tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/DerbyStore.java > b4bd605 > > tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/MySQLStore.java > d78e734 > > Diff: https://reviews.apache.org/r/18180/diff/ > > > Testing > ------- > > mvn clean install > > environment > * Derby > * MySQL 5.6.10 with lower_case_table_names=1 > > > Thanks, > > Yongjun Park > >
