> On June 7, 2014, 5:28 a.m., Hyunsik Choi wrote: > > tajo-catalog/tajo-catalog-common/src/main/proto/CatalogProtos.proto, line 63 > > <https://reviews.apache.org/r/20852/diff/5/?file=597829#file597829line63> > > > > I can understand EQUAL type predicate. In the case of EQUAL, the > > partition directory name would be the same to the partition column values. > > It will be the same to the existing column partition approach. > > > > But, in other cases, how we can determine the directory name? For > > partition, we should use the partition expression to divide the data. But, > > your proposed predicates except for EQUAL would result in TRUE or FALSE. It > > is hard to get some partition key from them. > > > > If partition expression is function, it would make sense. > > Alvin Henrick wrote: > Hi Hyunsik, Yes you are right the EQUAL is used for creating and dropping > partition but other operator will be used only when dropping partition. > > When Creating ==> COLUMNNAME EQUAL VALUE . The other > operator will never be used. > > > When Dropping ==> We will use COLUMNNAME (EQUAL , LTH , GTH > so on...) VALUE to drop the partition so I was under the assumption that it > should evaluate to TRUE or FALSE. > > > I am creating the partition expression and storing it in the > Meta store.Please have a look at AbstractDBStore. > > The simple partition expression will look like. ( COLUMNNAME1 > EQAUL VALUE1 ) > > The composite partition expression will look like. ( > COLUMNNAME1 EQAUL VALUE1 ,COLUMNNAME2 EQUAL VALUE2) > > I may be completely wrong here. Would need your help :) > > > Thanks! > Warm Regards, > Alvin.
I leave my comments: * In my view point, 'column partition' should deal with simple and composite partition. * ColumnPredicatePartition should not be a concreate class of PartitionMethodDescExpr because 'EQUAL' already is dealt by 'column partition' and non 'EQUAL' predicates are used for only dropping partition. * Instead, the predicate should be used as a partition drop predicate in AlterTable. * The PartitionType 'COL_PRED' would be not necessary. I have more comments. I'll directly add the comments on diff. - Hyunsik ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20852/#review44957 ----------------------------------------------------------- On May 29, 2014, 6:30 a.m., Alvin Henrick wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20852/ > ----------------------------------------------------------- > > (Updated May 29, 2014, 6:30 a.m.) > > > Review request for Tajo and Hyunsik Choi. > > > Bugs: TAJO-744 > https://issues.apache.org/jira/browse/TAJO-744 > > > Repository: tajo > > > Description > ------- > > TAJO-744 review request. The work is still in progress but wanted to get some > feedback. > > > Diffs > ----- > > tajo-algebra/src/main/java/org/apache/tajo/algebra/AlterTable.java 0f56bc2 > tajo-algebra/src/main/java/org/apache/tajo/algebra/AlterTableOpType.java > 67b28a2 > > tajo-algebra/src/main/java/org/apache/tajo/algebra/ColumnPredicatePartition.java > PRE-CREATION > tajo-algebra/src/main/java/org/apache/tajo/algebra/CreateTable.java f60b571 > tajo-algebra/src/main/java/org/apache/tajo/algebra/JsonHelper.java 7d853d6 > > tajo-algebra/src/main/java/org/apache/tajo/algebra/PartitionMethodDescExpr.java > PRE-CREATION > tajo-algebra/src/main/java/org/apache/tajo/algebra/PartitionType.java > PRE-CREATION > > tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/AlterTableDesc.java > fcaa4c3 > > tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/AlterTableType.java > 0b7639c > > tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java > d823f25 > > tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java > 85ea516 > > tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/PartitionPredicate.java > PRE-CREATION > > tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/PartitionPredicateSchema.java > PRE-CREATION > > tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/partition/PartitionPredicateMethodDesc.java > PRE-CREATION > tajo-catalog/tajo-catalog-common/src/main/proto/CatalogProtos.proto e70ed2b > > tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java > 7924af1 > > tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/AbstractDBStore.java > 0d22486 > > tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/CatalogStore.java > 5de9633 > > tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/DerbyStore.java > d6f9fc3 > > tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/MemStore.java > ca99160 > > tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/MySQLStore.java > 849afc8 > > tajo-catalog/tajo-catalog-server/src/main/resources/schemas/derby/partition_methods.sql > 4ad4c60 > > tajo-catalog/tajo-catalog-server/src/main/resources/schemas/derby/partition_methods_store.sql > PRE-CREATION > > tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mysql/partition_methods.sql > 060c4c8 > > tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mysql/partition_methods_store.sql > PRE-CREATION > > tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java > 453a54d > tajo-core/src/main/antlr4/org/apache/tajo/engine/parser/SQLParser.g4 > 0076794 > tajo-core/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java > e7de8f6 > tajo-core/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java > 0780d4f > > tajo-core/src/main/java/org/apache/tajo/engine/planner/logical/AlterTableNode.java > 76a47d0 > tajo-core/src/main/java/org/apache/tajo/master/GlobalEngine.java 3b81ce2 > tajo-core/src/test/java/org/apache/tajo/engine/parser/TestSQLAnalyzer.java > 2010502 > tajo-core/src/test/java/org/apache/tajo/engine/query/TestAlterTable.java > 57d8b32 > > tajo-core/src/test/resources/queries/TestAlterTable/alter_table_add_partition_ddl.sql > PRE-CREATION > > tajo-core/src/test/resources/queries/TestAlterTable/alter_table_drop_partition_ddl.sql > PRE-CREATION > > Diff: https://reviews.apache.org/r/20852/diff/ > > > Testing > ------- > > > Thanks, > > Alvin Henrick > >
