----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19685/#review38890 -----------------------------------------------------------
The latest patch looks nice for me. You are right. I missed the implementation of AbstractDBStore and HCatalogStore. Think you for reminding me :) As you concerned, we cannot use ALTER TABLE in practice if they are not implemented. The below link would be helpful for you. With some JVM options, you can perform unit tests in TestCatalog with Derby, or MySQL. Also, you can verify all integration tests with HCatalogStore. https://cwiki.apache.org/confluence/display/TAJO/Unit+Tests If you complete the implementation of AbstractDBStore, I'll also test your patch in various CatalogStores like Derby, MySQL, and Hive meta. Best Regards, Hyunsik Choi - Hyunsik Choi On March 28, 2014, 6:10 a.m., Alvin Henrick wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19685/ > ----------------------------------------------------------- > > (Updated March 28, 2014, 6:10 a.m.) > > > Review request for Tajo and Hyunsik Choi. > > > Bugs: TAJO-480 > https://issues.apache.org/jira/browse/TAJO-480 > > > Repository: tajo > > > Description > ------- > > 1) Implemented Alter Table. > 2) Made changes to various component like algebra,parser,catalog,planner and > engine etc. > 3) Only implemented MemStore for review.Before I proceed any further felt > like it needs to be reviewed. > 4) Need to implement AbstractDBStore.WIP > 5) Need to improve exception handling and logging.WIP. > > Please feel free to advice comment. Appreciate your help in reviewing the > code. > > Thanks! > Warm Regards, > Alvin. > > > HOLD ON GUYS!!!! I SEE THE DATABASE concept was introduced and it is causing > issues when patch is applied.I will soon publish the new PATCH. > I APOLOGIZE for inconvenience. > > > Diffs > ----- > > tajo-algebra/src/main/java/org/apache/tajo/algebra/AlterTable.java > PRE-CREATION > tajo-algebra/src/main/java/org/apache/tajo/algebra/AlterTableOpType.java > PRE-CREATION > tajo-algebra/src/main/java/org/apache/tajo/algebra/ColumnDefinition.java > PRE-CREATION > tajo-algebra/src/main/java/org/apache/tajo/algebra/CreateTable.java 42dcc68 > tajo-algebra/src/main/java/org/apache/tajo/algebra/OpType.java c4a007a > > tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java > 6fb385e > > tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/CatalogService.java > d69ed7e > tajo-catalog/tajo-catalog-client/src/main/proto/CatalogProtocol.proto > 06b69c1 > > tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/AlterTableDesc.java > PRE-CREATION > > tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/AlterTableType.java > PRE-CREATION > > tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java > 92df86a > > tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/ColumnNameAlreadyExistException.java > PRE-CREATION > tajo-catalog/tajo-catalog-common/src/main/proto/CatalogProtos.proto 14fb39f > > tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java > a171fb4 > > tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/AbstractDBStore.java > d10f545 > > tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/CatalogStore.java > 3f8686d > > tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/MemStore.java > ef98ee2 > > tajo-core/tajo-core-backend/src/main/antlr4/org/apache/tajo/engine/parser/SQLLexer.g4 > 6eccd12 > > tajo-core/tajo-core-backend/src/main/antlr4/org/apache/tajo/engine/parser/SQLParser.g4 > c0edf09 > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/parser/HiveQLAnalyzer.java > afd0ce9 > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java > 30d4c2e > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/AlgebraVisitor.java > 84cdc08 > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/BaseAlgebraVisitor.java > 62cee57 > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/BasicLogicalPlanVisitor.java > 772e5fb > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanPreprocessor.java > 1ac416f > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanVisitor.java > 76454b9 > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java > c479f1c > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/PlannerUtil.java > fbd65a9 > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/SimpleAlgebraVisitor.java > dc7b7a2 > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/AlterTableNode.java > PRE-CREATION > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/NodeType.java > 2b453fb > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/GlobalEngine.java > fe3caeb > > tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/QueryTestCaseBase.java > 65a0d47 > > tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestAlterTable.java > PRE-CREATION > > tajo-core/tajo-core-backend/src/test/resources/dataset/TestAlterTable/table1.tbl > PRE-CREATION > > tajo-core/tajo-core-backend/src/test/resources/queries/TestAlterTable/alter_table_add_new_column_ddl.sql > PRE-CREATION > > tajo-core/tajo-core-backend/src/test/resources/queries/TestAlterTable/alter_table_rename_column_ddl.sql > PRE-CREATION > > tajo-core/tajo-core-backend/src/test/resources/queries/TestAlterTable/alter_table_rename_table_ddl.sql > PRE-CREATION > > tajo-core/tajo-core-backend/src/test/resources/queries/TestAlterTable/table1_ddl.sql > PRE-CREATION > > Diff: https://reviews.apache.org/r/19685/diff/ > > > Testing > ------- > > > Thanks, > > Alvin Henrick > >
