Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.11.0
    • Component/s: Catalog
    • Labels:
      None

      Description

      This patch will implement ALTER TABLE ADD PARTITION method and ALTER TABLE DROP PARTITION method to CatalogStore.

      1. TAJO-1284_3.patch
        97 kB
        Jaehwa Jung
      2. TAJO-1284_2.patch
        82 kB
        Jaehwa Jung
      3. TAJO-1284.patch
        82 kB
        Jaehwa Jung
      4. TAJO-1284_2.sql
        1 kB
        Jaehwa Jung
      5. TAJO-1284_2.png
        46 kB
        Jaehwa Jung
      6. TAJO-1260.sql
        1 kB
        Jaehwa Jung
      7. TAJO-1260.png
        45 kB
        Jaehwa Jung

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          ABORTED: Integrated in Tajo-master-CODEGEN-build #270 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/270/)
          TAJO-1284: Add alter partition method to CatalogStore. (jaehwa) (blrunner: rev cad54428dd803cdb8caf3d51a9ba7e1d83a99b01)

          • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/postgresql.xml
          • tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/NoSuchPartitionException.java
          • tajo-core/src/main/java/org/apache/tajo/master/exec/NonForwardQueryResultSystemScanner.java
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/AlreadyExistsPartitionException.java
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/dictionary/PartitionKeysTableDescriptor.java
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/dictionary/PartitionsTableDescriptor.java
          • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mysql/partitions.sql
          • CHANGES
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/AlterTableDesc.java
          • tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/dictionary/InfoSchemaMetadataDictionary.java
          • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/derby/derby.xml
          • tajo-jdbc/src/test/java/org/apache/tajo/jdbc/util/TestQueryStringDecoder.java
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java
          • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mariadb/partitions.sql
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/AbstractDBStore.java
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/AbstractMySQLMariaDBStore.java
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/CatalogStore.java
          • tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogService.java
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/AlterTableType.java
          • tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/test/java/org/apache/tajo/catalog/store/TestHCatalogStore.java
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/partition/PartitionDesc.java
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/MemStore.java
          • tajo-catalog/tajo-catalog-common/src/main/proto/CatalogProtos.proto
          • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/oracle/oracle.xml
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/partition/PartitionKey.java
          • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mariadb/partition_keys.sql
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java
          • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mysql/partition_keys.sql
          • tajo-catalog/tajo-catalog-client/src/main/proto/CatalogProtocol.proto
          Show
          hudson Hudson added a comment - ABORTED: Integrated in Tajo-master-CODEGEN-build #270 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/270/ ) TAJO-1284 : Add alter partition method to CatalogStore. (jaehwa) (blrunner: rev cad54428dd803cdb8caf3d51a9ba7e1d83a99b01) tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/postgresql.xml tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/NoSuchPartitionException.java tajo-core/src/main/java/org/apache/tajo/master/exec/NonForwardQueryResultSystemScanner.java tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/AlreadyExistsPartitionException.java tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/dictionary/PartitionKeysTableDescriptor.java tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/dictionary/PartitionsTableDescriptor.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mysql/partitions.sql CHANGES tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/AlterTableDesc.java tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/dictionary/InfoSchemaMetadataDictionary.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/derby/derby.xml tajo-jdbc/src/test/java/org/apache/tajo/jdbc/util/TestQueryStringDecoder.java tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mariadb/partitions.sql tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/AbstractDBStore.java tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/AbstractMySQLMariaDBStore.java tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/CatalogStore.java tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogService.java tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/AlterTableType.java tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/test/java/org/apache/tajo/catalog/store/TestHCatalogStore.java tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/partition/PartitionDesc.java tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/MemStore.java tajo-catalog/tajo-catalog-common/src/main/proto/CatalogProtos.proto tajo-catalog/tajo-catalog-server/src/main/resources/schemas/oracle/oracle.xml tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/partition/PartitionKey.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mariadb/partition_keys.sql tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mysql/partition_keys.sql tajo-catalog/tajo-catalog-client/src/main/proto/CatalogProtocol.proto
          Hide
          hudson Hudson added a comment -

          ABORTED: Integrated in Tajo-master-build #632 (See https://builds.apache.org/job/Tajo-master-build/632/)
          TAJO-1284: Add alter partition method to CatalogStore. (jaehwa) (blrunner: rev cad54428dd803cdb8caf3d51a9ba7e1d83a99b01)

          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/dictionary/PartitionKeysTableDescriptor.java
          • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mysql/partition_keys.sql
          • tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/test/java/org/apache/tajo/catalog/store/TestHCatalogStore.java
          • tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java
          • tajo-catalog/tajo-catalog-client/src/main/proto/CatalogProtocol.proto
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/AlterTableType.java
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/MemStore.java
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/CatalogStore.java
          • tajo-core/src/main/java/org/apache/tajo/master/exec/NonForwardQueryResultSystemScanner.java
          • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mariadb/partitions.sql
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/dictionary/InfoSchemaMetadataDictionary.java
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/partition/PartitionDesc.java
          • tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/NoSuchPartitionException.java
          • tajo-catalog/tajo-catalog-common/src/main/proto/CatalogProtos.proto
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/dictionary/PartitionsTableDescriptor.java
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/partition/PartitionKey.java
          • tajo-jdbc/src/test/java/org/apache/tajo/jdbc/util/TestQueryStringDecoder.java
          • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/postgresql.xml
          • CHANGES
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogService.java
          • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mysql/partitions.sql
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/AlreadyExistsPartitionException.java
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/AbstractDBStore.java
          • tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java
          • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/oracle/oracle.xml
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/AlterTableDesc.java
          • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mariadb/partition_keys.sql
          • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java
          • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/derby/derby.xml
          • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/AbstractMySQLMariaDBStore.java
          Show
          hudson Hudson added a comment - ABORTED: Integrated in Tajo-master-build #632 (See https://builds.apache.org/job/Tajo-master-build/632/ ) TAJO-1284 : Add alter partition method to CatalogStore. (jaehwa) (blrunner: rev cad54428dd803cdb8caf3d51a9ba7e1d83a99b01) tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/dictionary/PartitionKeysTableDescriptor.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mysql/partition_keys.sql tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/test/java/org/apache/tajo/catalog/store/TestHCatalogStore.java tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java tajo-catalog/tajo-catalog-client/src/main/proto/CatalogProtocol.proto tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/AlterTableType.java tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/MemStore.java tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/CatalogStore.java tajo-core/src/main/java/org/apache/tajo/master/exec/NonForwardQueryResultSystemScanner.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mariadb/partitions.sql tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/dictionary/InfoSchemaMetadataDictionary.java tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/partition/PartitionDesc.java tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/NoSuchPartitionException.java tajo-catalog/tajo-catalog-common/src/main/proto/CatalogProtos.proto tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/dictionary/PartitionsTableDescriptor.java tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/partition/PartitionKey.java tajo-jdbc/src/test/java/org/apache/tajo/jdbc/util/TestQueryStringDecoder.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/postgresql.xml CHANGES tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogService.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mysql/partitions.sql tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/AlreadyExistsPartitionException.java tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/AbstractDBStore.java tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/oracle/oracle.xml tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/AlterTableDesc.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/mariadb/partition_keys.sql tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/derby/derby.xml tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/AbstractMySQLMariaDBStore.java
          Hide
          blrunner Jaehwa Jung added a comment -

          Jihoon Son

          Thank you for your detailed review.
          I've just committed it to the master branch.

          Show
          blrunner Jaehwa Jung added a comment - Jihoon Son Thank you for your detailed review. I've just committed it to the master branch.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/tajo/pull/448

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/448
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on the pull request:

          https://github.com/apache/tajo/pull/448#issuecomment-86779591

          Ok.
          Here is my +1.
          Ship it!

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/448#issuecomment-86779591 Ok. Here is my +1. Ship it!
          Hide
          tajoqa Tajo QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12707172/TAJO-1284_3.patch
          against master revision release-0.9.0-rc0-218-g12f30c5.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

          +1 checkstyle. The patch generated 0 code style errors.

          -1 findbugs. The patch appears to cause Findbugs (version 2.0.3) to fail.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-client tajo-catalog/tajo-catalog-common tajo-catalog/tajo-catalog-drivers/tajo-hcatalog tajo-catalog/tajo-catalog-server tajo-core tajo-jdbc.

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/660//testReport/
          Findbugs results: https://builds.apache.org/job/PreCommit-TAJO-Build/660//findbugsResult
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/660//console

          This message is automatically generated.

          Show
          tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12707172/TAJO-1284_3.patch against master revision release-0.9.0-rc0-218-g12f30c5. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to cause Findbugs (version 2.0.3) to fail. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-client tajo-catalog/tajo-catalog-common tajo-catalog/tajo-catalog-drivers/tajo-hcatalog tajo-catalog/tajo-catalog-server tajo-core tajo-jdbc. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/660//testReport/ Findbugs results: https://builds.apache.org/job/PreCommit-TAJO-Build/660//findbugsResult Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/660//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user blrunner commented on the pull request:

          https://github.com/apache/tajo/pull/448#issuecomment-85927642

          @jihoonson

          Thanks for your detailed review.
          I updated the patch according to your opinion.
          And I will add more unit test cases for TestAlterTable with another issue.
          Please refer the following site:
          https://issues.apache.org/jira/browse/TAJO-1345

          Show
          githubbot ASF GitHub Bot added a comment - Github user blrunner commented on the pull request: https://github.com/apache/tajo/pull/448#issuecomment-85927642 @jihoonson Thanks for your detailed review. I updated the patch according to your opinion. And I will add more unit test cases for TestAlterTable with another issue. Please refer the following site: https://issues.apache.org/jira/browse/TAJO-1345
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user blrunner commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/448#discussion_r27090442

          — Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java —
          @@ -48,11 +48,16 @@
          public static final String TB_STATISTICS = "STATS";
          public static final String TB_PARTITION_METHODS = "PARTITION_METHODS";
          public static final String TB_PARTTIONS = "PARTITIONS";
          + public static final String TB_PARTTION_KEYS = "PARTITION_KEYS";

          public static final String COL_TABLESPACE_PK = "SPACE_ID";
          public static final String COL_DATABASES_PK = "DB_ID";
          public static final String COL_TABLES_PK = "TID";
          public static final String COL_TABLES_NAME = "TABLE_NAME";
          +
          + public static final String COL_PARTITIONS_PK = "PID";
          — End diff –

          @jihoonson

          I misunderstood your first comments.
          I'll rename the primary key name PARTITION_ID.

          Show
          githubbot ASF GitHub Bot added a comment - Github user blrunner commented on a diff in the pull request: https://github.com/apache/tajo/pull/448#discussion_r27090442 — Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java — @@ -48,11 +48,16 @@ public static final String TB_STATISTICS = "STATS"; public static final String TB_PARTITION_METHODS = "PARTITION_METHODS"; public static final String TB_PARTTIONS = "PARTITIONS"; + public static final String TB_PARTTION_KEYS = "PARTITION_KEYS"; public static final String COL_TABLESPACE_PK = "SPACE_ID"; public static final String COL_DATABASES_PK = "DB_ID"; public static final String COL_TABLES_PK = "TID"; public static final String COL_TABLES_NAME = "TABLE_NAME"; + + public static final String COL_PARTITIONS_PK = "PID"; — End diff – @jihoonson I misunderstood your first comments. I'll rename the primary key name PARTITION_ID.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user blrunner commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/448#discussion_r27083258

          — Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java —
          @@ -48,11 +48,16 @@
          public static final String TB_STATISTICS = "STATS";
          public static final String TB_PARTITION_METHODS = "PARTITION_METHODS";
          public static final String TB_PARTTIONS = "PARTITIONS";
          + public static final String TB_PARTTION_KEYS = "PARTITION_KEYS";

          public static final String COL_TABLESPACE_PK = "SPACE_ID";
          public static final String COL_DATABASES_PK = "DB_ID";
          public static final String COL_TABLES_PK = "TID";
          public static final String COL_TABLES_NAME = "TABLE_NAME";
          +
          + public static final String COL_PARTITIONS_PK = "PID";
          — End diff –

          Okay, I also think that it is more simple and more intuitive.
          And how about rename existing variables as following:

          • COL_TABLESPACE_PK --> TABLESPACE_ID
          Show
          githubbot ASF GitHub Bot added a comment - Github user blrunner commented on a diff in the pull request: https://github.com/apache/tajo/pull/448#discussion_r27083258 — Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java — @@ -48,11 +48,16 @@ public static final String TB_STATISTICS = "STATS"; public static final String TB_PARTITION_METHODS = "PARTITION_METHODS"; public static final String TB_PARTTIONS = "PARTITIONS"; + public static final String TB_PARTTION_KEYS = "PARTITION_KEYS"; public static final String COL_TABLESPACE_PK = "SPACE_ID"; public static final String COL_DATABASES_PK = "DB_ID"; public static final String COL_TABLES_PK = "TID"; public static final String COL_TABLES_NAME = "TABLE_NAME"; + + public static final String COL_PARTITIONS_PK = "PID"; — End diff – Okay, I also think that it is more simple and more intuitive. And how about rename existing variables as following: COL_TABLESPACE_PK --> TABLESPACE_ID
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/448#discussion_r27083099

          — Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java —
          @@ -48,11 +48,16 @@
          public static final String TB_STATISTICS = "STATS";
          public static final String TB_PARTITION_METHODS = "PARTITION_METHODS";
          public static final String TB_PARTTIONS = "PARTITIONS";
          + public static final String TB_PARTTION_KEYS = "PARTITION_KEYS";

          public static final String COL_TABLESPACE_PK = "SPACE_ID";
          public static final String COL_DATABASES_PK = "DB_ID";
          public static final String COL_TABLES_PK = "TID";
          public static final String COL_TABLES_NAME = "TABLE_NAME";
          +
          + public static final String COL_PARTITIONS_PK = "PID";
          — End diff –

          IMO, ```PARTITION_ID``` looks good.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/448#discussion_r27083099 — Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java — @@ -48,11 +48,16 @@ public static final String TB_STATISTICS = "STATS"; public static final String TB_PARTITION_METHODS = "PARTITION_METHODS"; public static final String TB_PARTTIONS = "PARTITIONS"; + public static final String TB_PARTTION_KEYS = "PARTITION_KEYS"; public static final String COL_TABLESPACE_PK = "SPACE_ID"; public static final String COL_DATABASES_PK = "DB_ID"; public static final String COL_TABLES_PK = "TID"; public static final String COL_TABLES_NAME = "TABLE_NAME"; + + public static final String COL_PARTITIONS_PK = "PID"; — End diff – IMO, ```PARTITION_ID``` looks good.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user blrunner commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/448#discussion_r27082506

          — Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java —
          @@ -48,11 +48,16 @@
          public static final String TB_STATISTICS = "STATS";
          public static final String TB_PARTITION_METHODS = "PARTITION_METHODS";
          public static final String TB_PARTTIONS = "PARTITIONS";
          + public static final String TB_PARTTION_KEYS = "PARTITION_KEYS";

          public static final String COL_TABLESPACE_PK = "SPACE_ID";
          public static final String COL_DATABASES_PK = "DB_ID";
          public static final String COL_TABLES_PK = "TID";
          public static final String COL_TABLES_NAME = "TABLE_NAME";
          +
          + public static final String COL_PARTITIONS_PK = "PID";
          — End diff –

          I followed existing naming rule of CatalogConstants. I think there are three options as follows.

          • COL_PARTITIONS_PK
          • COL_PARTITIONS_ID
          • PARTITIONS_ID

          What's your opinion about above options?

          Show
          githubbot ASF GitHub Bot added a comment - Github user blrunner commented on a diff in the pull request: https://github.com/apache/tajo/pull/448#discussion_r27082506 — Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java — @@ -48,11 +48,16 @@ public static final String TB_STATISTICS = "STATS"; public static final String TB_PARTITION_METHODS = "PARTITION_METHODS"; public static final String TB_PARTTIONS = "PARTITIONS"; + public static final String TB_PARTTION_KEYS = "PARTITION_KEYS"; public static final String COL_TABLESPACE_PK = "SPACE_ID"; public static final String COL_DATABASES_PK = "DB_ID"; public static final String COL_TABLES_PK = "TID"; public static final String COL_TABLES_NAME = "TABLE_NAME"; + + public static final String COL_PARTITIONS_PK = "PID"; — End diff – I followed existing naming rule of CatalogConstants. I think there are three options as follows. COL_PARTITIONS_PK COL_PARTITIONS_ID PARTITIONS_ID What's your opinion about above options?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on the pull request:

          https://github.com/apache/tajo/pull/448#issuecomment-85500561

          @blrunner thanks for your nice patch.
          I left some comments.
          Would you add a table for partition keys to ```information_schema```?
          In addition, it would be great if you add more tests. TestAlterTable looks good for testing.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/448#issuecomment-85500561 @blrunner thanks for your nice patch. I left some comments. Would you add a table for partition keys to ```information_schema```? In addition, it would be great if you add more tests. TestAlterTable looks good for testing.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/448#discussion_r27026522

          — Diff: tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/postgresql.xml —
          @@ -151,7 +151,6 @@ xsi:schemaLocation="http://tajo.apache.org/catalogstore ../DBMSSchemaDefinition.
          PID INT NOT NULL PRIMARY KEY,
          TID INT NOT NULL,
          PARTITION_NAME VARCHAR(128),

          • ORDINAL_POSITION INT NOT NULL,
              • End diff –

          This schema is not compatible with that of other databases.
          It looks that some changes are missed.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/448#discussion_r27026522 — Diff: tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/postgresql.xml — @@ -151,7 +151,6 @@ xsi:schemaLocation="http://tajo.apache.org/catalogstore ../DBMSSchemaDefinition. PID INT NOT NULL PRIMARY KEY, TID INT NOT NULL, PARTITION_NAME VARCHAR(128), ORDINAL_POSITION INT NOT NULL, End diff – This schema is not compatible with that of other databases. It looks that some changes are missed.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/448#discussion_r27025505

          — Diff: tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/MemStore.java —
          @@ -498,37 +498,16 @@ public void dropPartitionMethod(String databaseName, String tableName) throws Ca
          }

          @Override

          • public void addPartitions(CatalogProtos.PartitionsProto partitionDescList) throws CatalogException {
            + public List<CatalogProtos.PartitionDescProto> getPartitions(String databaseName, String tableName) throws CatalogException { throw new RuntimeException("not supported!"); }

          @Override

          • public void addPartition(String databaseName, String tableName, CatalogProtos.PartitionDescProto
          • partitionDescProto) throws CatalogException {
            + public CatalogProtos.PartitionDescProto getPartition(String databaseName, String tableName,
            + String partitionName) throws CatalogException {
            throw new RuntimeException("not supported!");
              • End diff –

          MemStore is used for tests, so we need to implement this function.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/448#discussion_r27025505 — Diff: tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/MemStore.java — @@ -498,37 +498,16 @@ public void dropPartitionMethod(String databaseName, String tableName) throws Ca } @Override public void addPartitions(CatalogProtos.PartitionsProto partitionDescList) throws CatalogException { + public List<CatalogProtos.PartitionDescProto> getPartitions(String databaseName, String tableName) throws CatalogException { throw new RuntimeException("not supported!"); } @Override public void addPartition(String databaseName, String tableName, CatalogProtos.PartitionDescProto partitionDescProto) throws CatalogException { + public CatalogProtos.PartitionDescProto getPartition(String databaseName, String tableName, + String partitionName) throws CatalogException { throw new RuntimeException("not supported!"); End diff – MemStore is used for tests, so we need to implement this function.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/448#discussion_r27025498

          — Diff: tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/MemStore.java —
          @@ -498,37 +498,16 @@ public void dropPartitionMethod(String databaseName, String tableName) throws Ca
          }

          @Override

          • public void addPartitions(CatalogProtos.PartitionsProto partitionDescList) throws CatalogException {
            + public List<CatalogProtos.PartitionDescProto> getPartitions(String databaseName, String tableName) throws CatalogException {
            throw new RuntimeException("not supported!");
              • End diff –

          MemStore is used for tests, so we need to implement this function.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/448#discussion_r27025498 — Diff: tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/MemStore.java — @@ -498,37 +498,16 @@ public void dropPartitionMethod(String databaseName, String tableName) throws Ca } @Override public void addPartitions(CatalogProtos.PartitionsProto partitionDescList) throws CatalogException { + public List<CatalogProtos.PartitionDescProto> getPartitions(String databaseName, String tableName) throws CatalogException { throw new RuntimeException("not supported!"); End diff – MemStore is used for tests, so we need to implement this function.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/448#discussion_r27023446

          — Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/partition/PartitionDesc.java —
          @@ -50,15 +73,21 @@ public PartitionDesc(CatalogProtos.PartitionDescProto proto) {
          if(proto.hasPartitionName())

          { this.partitionName = proto.getPartitionName(); }
          • this.ordinalPosition = proto.getOrdinalPosition();
          • if(proto.hasPartitionValue()) {
          • this.partitionValue = proto.getPartitionValue();
            +
            + this.partitionKeys = new ArrayList<PartitionKey>();
            + for(CatalogProtos.PartitionKeyProto keyProto : proto.getPartitionKeysList()) { + PartitionKey partitionKey = new PartitionKey(keyProto); + this.partitionKeys.add(partitionKey); }

            +
            if(proto.hasPath())

            { this.path = proto.getPath(); }

            }

          + public String getPartitionName()

          { return partitionName; }

          + public void setPartitionName(String partitionName)

          { this.partitionName = partitionName; }

          — End diff –

          Is this function different with the below setName() function?
          In addition, please move brackets according to our coding convention.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/448#discussion_r27023446 — Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/partition/PartitionDesc.java — @@ -50,15 +73,21 @@ public PartitionDesc(CatalogProtos.PartitionDescProto proto) { if(proto.hasPartitionName()) { this.partitionName = proto.getPartitionName(); } this.ordinalPosition = proto.getOrdinalPosition(); if(proto.hasPartitionValue()) { this.partitionValue = proto.getPartitionValue(); + + this.partitionKeys = new ArrayList<PartitionKey>(); + for(CatalogProtos.PartitionKeyProto keyProto : proto.getPartitionKeysList()) { + PartitionKey partitionKey = new PartitionKey(keyProto); + this.partitionKeys.add(partitionKey); } + if(proto.hasPath()) { this.path = proto.getPath(); } } + public String getPartitionName() { return partitionName; } + public void setPartitionName(String partitionName) { this.partitionName = partitionName; } — End diff – Is this function different with the below setName() function? In addition, please move brackets according to our coding convention.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/448#discussion_r27023351

          — Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java —
          @@ -48,11 +48,16 @@
          public static final String TB_STATISTICS = "STATS";
          public static final String TB_PARTITION_METHODS = "PARTITION_METHODS";
          public static final String TB_PARTTIONS = "PARTITIONS";
          + public static final String TB_PARTTION_KEYS = "PARTITION_KEYS";

          public static final String COL_TABLESPACE_PK = "SPACE_ID";
          public static final String COL_DATABASES_PK = "DB_ID";
          public static final String COL_TABLES_PK = "TID";
          public static final String COL_TABLES_NAME = "TABLE_NAME";
          +
          + public static final String COL_PARTITIONS_PK = "PID";
          — End diff –

          I think that it would be better if the name is more intuitive.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/448#discussion_r27023351 — Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java — @@ -48,11 +48,16 @@ public static final String TB_STATISTICS = "STATS"; public static final String TB_PARTITION_METHODS = "PARTITION_METHODS"; public static final String TB_PARTTIONS = "PARTITIONS"; + public static final String TB_PARTTION_KEYS = "PARTITION_KEYS"; public static final String COL_TABLESPACE_PK = "SPACE_ID"; public static final String COL_DATABASES_PK = "DB_ID"; public static final String COL_TABLES_PK = "TID"; public static final String COL_TABLES_NAME = "TABLE_NAME"; + + public static final String COL_PARTITIONS_PK = "PID"; — End diff – I think that it would be better if the name is more intuitive.
          Hide
          tajoqa Tajo QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12706554/TAJO-1284_2.patch
          against master revision release-0.9.0-rc0-214-g8d0146b.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

          +1 checkstyle. The patch generated 0 code style errors.

          -1 findbugs. The patch appears to cause Findbugs (version 2.0.3) to fail.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-client tajo-catalog/tajo-catalog-common tajo-catalog/tajo-catalog-drivers/tajo-hcatalog tajo-catalog/tajo-catalog-server tajo-core.

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/645//testReport/
          Findbugs results: https://builds.apache.org/job/PreCommit-TAJO-Build/645//findbugsResult
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/645//console

          This message is automatically generated.

          Show
          tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12706554/TAJO-1284_2.patch against master revision release-0.9.0-rc0-214-g8d0146b. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to cause Findbugs (version 2.0.3) to fail. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-client tajo-catalog/tajo-catalog-common tajo-catalog/tajo-catalog-drivers/tajo-hcatalog tajo-catalog/tajo-catalog-server tajo-core. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/645//testReport/ Findbugs results: https://builds.apache.org/job/PreCommit-TAJO-Build/645//findbugsResult Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/645//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user blrunner commented on the pull request:

          https://github.com/apache/tajo/pull/448#issuecomment-85052400

          Thanks @jihoonson
          I've just fixed the patch by your comments.
          If you review the patch continuously, it would be pleasure to me.

          Show
          githubbot ASF GitHub Bot added a comment - Github user blrunner commented on the pull request: https://github.com/apache/tajo/pull/448#issuecomment-85052400 Thanks @jihoonson I've just fixed the patch by your comments. If you review the patch continuously, it would be pleasure to me.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on the pull request:

          https://github.com/apache/tajo/pull/448#issuecomment-85016516

          I'm still reviewing. I'll finish soon.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/448#issuecomment-85016516 I'm still reviewing. I'll finish soon.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/448#discussion_r26939171

          — Diff: tajo-catalog/tajo-catalog-common/src/main/proto/CatalogProtos.proto —
          @@ -293,10 +293,21 @@ message PartitionMethodProto {
          }

          message PartitionDescProto {

          • optional string partitionName = 2;
          • required int32 ordinalPosition = 3;
          • optional string partitionValue = 4;
          • optional string path = 5;
            + required string partitionName = 1;
            + repeated PartitionKeyProto partitionKeys = 32;
              • End diff –

          This will not induce any problems, but it would be better to change to 2.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/448#discussion_r26939171 — Diff: tajo-catalog/tajo-catalog-common/src/main/proto/CatalogProtos.proto — @@ -293,10 +293,21 @@ message PartitionMethodProto { } message PartitionDescProto { optional string partitionName = 2; required int32 ordinalPosition = 3; optional string partitionValue = 4; optional string path = 5; + required string partitionName = 1; + repeated PartitionKeyProto partitionKeys = 32; End diff – This will not induce any problems, but it would be better to change to 2.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/448#discussion_r26939127

          — Diff: tajo-catalog/tajo-catalog-common/src/main/proto/CatalogProtos.proto —
          @@ -281,8 +282,7 @@ message SortSpecProto {

          message PartitionsProto {

          • required TableIdentifierProto tableIdentifier = 1;
          • repeated PartitionDescProto partition = 2;
            + repeated PartitionDescProto partition = 21;
              • End diff –

          This will not induce any problems, but it would be better to change to 2.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/448#discussion_r26939127 — Diff: tajo-catalog/tajo-catalog-common/src/main/proto/CatalogProtos.proto — @@ -281,8 +282,7 @@ message SortSpecProto { message PartitionsProto { required TableIdentifierProto tableIdentifier = 1; repeated PartitionDescProto partition = 2; + repeated PartitionDescProto partition = 21; End diff – This will not induce any problems, but it would be better to change to 2.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on a diff in the pull request:

          https://github.com/apache/tajo/pull/448#discussion_r26938947

          — Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/partition/PartitionKey.java —
          @@ -0,0 +1,148 @@
          +/**
          + * Licensed to the Apache Software Foundation (ASF) under one
          + * or more contributor license agreements. See the NOTICE file
          + * distributed with this work for additional information
          + * regarding copyright ownership. The ASF licenses this file
          + * to you under the Apache License, Version 2.0 (the
          + * "License"); you may not use this file except in compliance
          + * with the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +
          +package org.apache.tajo.catalog.partition;
          +
          +import com.google.common.base.Objects;
          +import com.google.gson.annotations.Expose;
          +import org.apache.tajo.catalog.json.CatalogGsonHelper;
          +import org.apache.tajo.catalog.proto.CatalogProtos;
          +import org.apache.tajo.common.ProtoObject;
          +import org.apache.tajo.json.GsonObject;
          +
          +
          +/**
          + * This presents column name and partition value pairs of column partitioned table.
          + *
          + * For example, consider you have a partitioned table as follows:
          + *
          + * create external table table1 (id text, name text) PARTITION BY COLUMN (dt text, phone text,
          + * gender text) USING RCFILE LOCATION '/tajo/data/table1';
          + *
          + * Then, its data will be stored on HDFS as follows:
          + * - /tajo/data/table1/dt=20150301/phone=1300/gender=m
          + * - /tajo/data/table1/dt=20150301/phone=1300/gender=f
          + * - /tajo/data/table1/dt=20150302/phone=1500/gender=m
          + * - /tajo/data/table1/dt=20150302/phone=1500/gender=f
          + *
          + * In such as above, first directory can be presented with this class as follows:
          + * The first pair: column name = dt, partition value = 20150301
          + * The second pair: column name = phone, partition value = 1300
          + * The thris pair: column name = gender, partition value = m
          + *
          + */
          +public class PartitionKey implements ProtoObject<CatalogProtos.PartitionKeyProto>, Cloneable, GsonObject {
          + @Expose protected String columnName; // required
          + @Expose protected String partitionValue; // required
          +
          + private CatalogProtos.PartitionKeyProto.Builder builder = CatalogProtos.PartitionKeyProto.newBuilder();
          +
          + public PartitionKey()

          { + }

          +
          + public PartitionKey(String columnName, String partitionValue)

          { + this.columnName = columnName; + this.partitionValue = partitionValue; + }

          +
          + public PartitionKey(PartitionKey partition)

          { + this.columnName = partition.columnName; + this.partitionValue = partition.partitionValue; + }

          +
          + public PartitionKey(CatalogProtos.PartitionKeyProto proto) {
          + if (proto.hasColumnName())

          { + this.columnName = proto.getColumnName(); + }

          + if (proto.hasPartitionValue())

          { + this.partitionValue = proto.getPartitionValue(); + }

          + }
          +
          + public String getPartitionValue()

          { + return partitionValue; + }

          +
          + public void setPartitionValue(String partitionValue)

          { + this.partitionValue = partitionValue; + }

          +
          + public String getColumnName()

          { return columnName; }

          +
          + public void setColumnName(String columnName)

          { this.columnName = columnName; }

          +
          + public int hashCode()

          { + return Objects.hashCode(partitionValue, columnName); + }

          +
          + public boolean equals(Object o) {
          + if (o instanceof PartitionKey) {
          + PartitionKey another = (PartitionKey) o;
          + boolean eq = ((columnName != null && another.columnName != null
          — End diff –

          How about to use TUtil.checkEquals()?

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/448#discussion_r26938947 — Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/partition/PartitionKey.java — @@ -0,0 +1,148 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.tajo.catalog.partition; + +import com.google.common.base.Objects; +import com.google.gson.annotations.Expose; +import org.apache.tajo.catalog.json.CatalogGsonHelper; +import org.apache.tajo.catalog.proto.CatalogProtos; +import org.apache.tajo.common.ProtoObject; +import org.apache.tajo.json.GsonObject; + + +/** + * This presents column name and partition value pairs of column partitioned table. + * + * For example, consider you have a partitioned table as follows: + * + * create external table table1 (id text, name text) PARTITION BY COLUMN (dt text, phone text, + * gender text) USING RCFILE LOCATION '/tajo/data/table1'; + * + * Then, its data will be stored on HDFS as follows: + * - /tajo/data/table1/dt=20150301/phone=1300/gender=m + * - /tajo/data/table1/dt=20150301/phone=1300/gender=f + * - /tajo/data/table1/dt=20150302/phone=1500/gender=m + * - /tajo/data/table1/dt=20150302/phone=1500/gender=f + * + * In such as above, first directory can be presented with this class as follows: + * The first pair: column name = dt, partition value = 20150301 + * The second pair: column name = phone, partition value = 1300 + * The thris pair: column name = gender, partition value = m + * + */ +public class PartitionKey implements ProtoObject<CatalogProtos.PartitionKeyProto>, Cloneable, GsonObject { + @Expose protected String columnName; // required + @Expose protected String partitionValue; // required + + private CatalogProtos.PartitionKeyProto.Builder builder = CatalogProtos.PartitionKeyProto.newBuilder(); + + public PartitionKey() { + } + + public PartitionKey(String columnName, String partitionValue) { + this.columnName = columnName; + this.partitionValue = partitionValue; + } + + public PartitionKey(PartitionKey partition) { + this.columnName = partition.columnName; + this.partitionValue = partition.partitionValue; + } + + public PartitionKey(CatalogProtos.PartitionKeyProto proto) { + if (proto.hasColumnName()) { + this.columnName = proto.getColumnName(); + } + if (proto.hasPartitionValue()) { + this.partitionValue = proto.getPartitionValue(); + } + } + + public String getPartitionValue() { + return partitionValue; + } + + public void setPartitionValue(String partitionValue) { + this.partitionValue = partitionValue; + } + + public String getColumnName() { return columnName; } + + public void setColumnName(String columnName) { this.columnName = columnName; } + + public int hashCode() { + return Objects.hashCode(partitionValue, columnName); + } + + public boolean equals(Object o) { + if (o instanceof PartitionKey) { + PartitionKey another = (PartitionKey) o; + boolean eq = ((columnName != null && another.columnName != null — End diff – How about to use TUtil.checkEquals()?
          Hide
          tajoqa Tajo QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12706393/TAJO-1284.patch
          against master revision release-0.9.0-rc0-213-g3aaff38.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

          +1 checkstyle. The patch generated 0 code style errors.

          -1 findbugs. The patch appears to cause Findbugs (version 2.0.3) to fail.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-client tajo-catalog/tajo-catalog-common tajo-catalog/tajo-catalog-drivers/tajo-hcatalog tajo-catalog/tajo-catalog-server tajo-core.

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/635//testReport/
          Findbugs results: https://builds.apache.org/job/PreCommit-TAJO-Build/635//findbugsResult
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/635//console

          This message is automatically generated.

          Show
          tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12706393/TAJO-1284.patch against master revision release-0.9.0-rc0-213-g3aaff38. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to cause Findbugs (version 2.0.3) to fail. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-client tajo-catalog/tajo-catalog-common tajo-catalog/tajo-catalog-drivers/tajo-hcatalog tajo-catalog/tajo-catalog-server tajo-core. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/635//testReport/ Findbugs results: https://builds.apache.org/job/PreCommit-TAJO-Build/635//findbugsResult Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/635//console This message is automatically generated.
          Hide
          blrunner Jaehwa Jung added a comment -

          I updated the index for related tables and Tajo can get related partition directories with just one SELECT statement.

          For example, consider you have a partitioned table for three columns (i.e., col1, col2, col3).
          Then, you can write WHERE statement as follows:

          WHERE col1 is not null and col2 = 3 and col3 is not null
          

          In such as above, Tajo can get the partition directories as follows:

          SELECT DISTINCT A.PATH
          FROM PARTITIONS A, (
            SELECT B.PID
            FROM PARTITION_KEYS B
            WHERE B.PID > 0 
            AND (
              (B.COLUMN_NAME = 'col1' AND B.PARTITION_VALUE IS NOT NULL) 
              OR (B.COLUMN_NAME = 'col2' AND B.PARTITION_VALUE = 3) 
              OR (B.COLUMN_NAME = 'col3' AND B.PARTITION_VALUE IS NOT NULL)
            )
          ) B
          WHERE A.PID > 0
          AND A.TID = 33
          AND A.PID = B.PID
          

          For the reference, I'll create new issue for supporting this function.

          Show
          blrunner Jaehwa Jung added a comment - I updated the index for related tables and Tajo can get related partition directories with just one SELECT statement. For example, consider you have a partitioned table for three columns (i.e., col1, col2, col3). Then, you can write WHERE statement as follows: WHERE col1 is not null and col2 = 3 and col3 is not null In such as above, Tajo can get the partition directories as follows: SELECT DISTINCT A.PATH FROM PARTITIONS A, ( SELECT B.PID FROM PARTITION_KEYS B WHERE B.PID > 0 AND ( (B.COLUMN_NAME = 'col1' AND B.PARTITION_VALUE IS NOT NULL) OR (B.COLUMN_NAME = 'col2' AND B.PARTITION_VALUE = 3) OR (B.COLUMN_NAME = 'col3' AND B.PARTITION_VALUE IS NOT NULL) ) ) B WHERE A.PID > 0 AND A.TID = 33 AND A.PID = B.PID For the reference, I'll create new issue for supporting this function.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user blrunner opened a pull request:

          https://github.com/apache/tajo/pull/448

          TAJO-1284: Add alter partition method to CatalogStore.

          This patch will implement ALTER TABLE ADD PARTITION method and ALTER TABLE DROP PARTITION method to CatalogStore.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/blrunner/tajo TAJO-1284-NEW

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/tajo/pull/448.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #448


          commit 3b2c8c10176a84e2c1b07325920f0e289d52d641
          Author: JaeHwa Jung <blrunner@apache.org>
          Date: 2015-03-22T14:49:45Z

          TAJO-1284: Add alter partition method to CatalogStore.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user blrunner opened a pull request: https://github.com/apache/tajo/pull/448 TAJO-1284 : Add alter partition method to CatalogStore. This patch will implement ALTER TABLE ADD PARTITION method and ALTER TABLE DROP PARTITION method to CatalogStore. You can merge this pull request into a Git repository by running: $ git pull https://github.com/blrunner/tajo TAJO-1284 -NEW Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/448.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #448 commit 3b2c8c10176a84e2c1b07325920f0e289d52d641 Author: JaeHwa Jung <blrunner@apache.org> Date: 2015-03-22T14:49:45Z TAJO-1284 : Add alter partition method to CatalogStore.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user blrunner closed the pull request at:

          https://github.com/apache/tajo/pull/382

          Show
          githubbot ASF GitHub Bot added a comment - Github user blrunner closed the pull request at: https://github.com/apache/tajo/pull/382
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user blrunner commented on the pull request:

          https://github.com/apache/tajo/pull/382#issuecomment-84616248

          I'll open new PR.

          Show
          githubbot ASF GitHub Bot added a comment - Github user blrunner commented on the pull request: https://github.com/apache/tajo/pull/382#issuecomment-84616248 I'll open new PR.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user blrunner commented on the pull request:

          https://github.com/apache/tajo/pull/382#issuecomment-74193544

          I removed existing addPartition and addPartitions at CatalogStore. Because it seemed like an inconvenient methods to implement and maintain. So, I implemented it to alter table method. For reference, I implemented it to HCatalogStore, But I didn't implement getAllPartitions and get Partitions because of HiveMetastore performance.

          Show
          githubbot ASF GitHub Bot added a comment - Github user blrunner commented on the pull request: https://github.com/apache/tajo/pull/382#issuecomment-74193544 I removed existing addPartition and addPartitions at CatalogStore. Because it seemed like an inconvenient methods to implement and maintain. So, I implemented it to alter table method. For reference, I implemented it to HCatalogStore, But I didn't implement getAllPartitions and get Partitions because of HiveMetastore performance.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user blrunner opened a pull request:

          https://github.com/apache/tajo/pull/382

          TAJO-1284: Add alter partition method to CatalogStore.

          This patch will implement ALTER TABLE ADD PARTITION method and ALTER TABLE DROP PARTITION method to CatalogStore.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/blrunner/tajo TAJO-1284

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/tajo/pull/382.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #382


          commit 2d8f957f2e88e2ea9946adb6232e4905f74a58c2
          Author: JaeHwa Jung <blrunner@apache.org>
          Date: 2015-02-12T09:04:17Z

          TAJO-1284: Add alter partition method to CatalogStore.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user blrunner opened a pull request: https://github.com/apache/tajo/pull/382 TAJO-1284 : Add alter partition method to CatalogStore. This patch will implement ALTER TABLE ADD PARTITION method and ALTER TABLE DROP PARTITION method to CatalogStore. You can merge this pull request into a Git repository by running: $ git pull https://github.com/blrunner/tajo TAJO-1284 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/382.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #382 commit 2d8f957f2e88e2ea9946adb6232e4905f74a58c2 Author: JaeHwa Jung <blrunner@apache.org> Date: 2015-02-12T09:04:17Z TAJO-1284 : Add alter partition method to CatalogStore.
          Hide
          blrunner Jaehwa Jung added a comment -

          Thanks Hyunsik Choi

          Sure, the new name looks good to me. I've just renamed my working table name.

          And I also was considering other partition types, such as range partition, list partition. But it's not too late to support other partition types after stabilizing the column partition. If we are prepared to implement other partition types, we can add other columns to partition table of catalog. Or we can design new tables. In this case, we need to support a sql script file for upgrading catalog server.

          Show
          blrunner Jaehwa Jung added a comment - Thanks Hyunsik Choi Sure, the new name looks good to me. I've just renamed my working table name. And I also was considering other partition types, such as range partition, list partition. But it's not too late to support other partition types after stabilizing the column partition. If we are prepared to implement other partition types, we can add other columns to partition table of catalog. Or we can design new tables. In this case, we need to support a sql script file for upgrading catalog server.
          Hide
          hyunsik Hyunsik Choi added a comment -

          In our roadmap, we will support range partition too. But, the proposed schema does not consider other kinds of partition types. Of course, this consideration is not mandatory. Nevertheless, we may need some room to support other partition types.

          Show
          hyunsik Hyunsik Choi added a comment - In our roadmap, we will support range partition too. But, the proposed schema does not consider other kinds of partition types. Of course, this consideration is not mandatory. Nevertheless, we may need some room to support other partition types.
          Hide
          hyunsik Hyunsik Choi added a comment -

          Thank you for your work. The work looks nice.

          I have one suggestion. I'd like to propose the rename from PARTiTION_PARAMS to PARTITION_KEYS. In my opinion, PARTITION_KEYS is more intuitive with regard to its purpose.

          Show
          hyunsik Hyunsik Choi added a comment - Thank you for your work. The work looks nice. I have one suggestion. I'd like to propose the rename from PARTiTION_PARAMS to PARTITION_KEYS. In my opinion, PARTITION_KEYS is more intuitive with regard to its purpose.
          Hide
          blrunner Jaehwa Jung added a comment -

          I planed to use existing table named PARTITIONS. And I just created a new table named PARTITION_PARARMS. For reference, I moved value column of PARTITIONS table to PARTITION_PARARMS table.

          Show
          blrunner Jaehwa Jung added a comment - I planed to use existing table named PARTITIONS. And I just created a new table named PARTITION_PARARMS. For reference, I moved value column of PARTITIONS table to PARTITION_PARARMS table.
          Hide
          blrunner Jaehwa Jung added a comment - - edited

          I designed two tables as follows:

          • COLUMN_PARTITIONS: Maintains each partition name and partition path. If an user add one partition, Tajo will insert just one row at this table.
          • COLUMN_PARTITION_PARAMS: Maintains each partition key and values by a row. If an user add one partition which consists of two keys, Tajo will insert two rows at this table.

          I uploaded two files that could help comprehension. For reference, I just created a script file for MySQL users.

          Show
          blrunner Jaehwa Jung added a comment - - edited I designed two tables as follows: COLUMN_PARTITIONS: Maintains each partition name and partition path. If an user add one partition, Tajo will insert just one row at this table. COLUMN_PARTITION_PARAMS: Maintains each partition key and values by a row. If an user add one partition which consists of two keys, Tajo will insert two rows at this table. I uploaded two files that could help comprehension. For reference, I just created a script file for MySQL users.

            People

            • Assignee:
              blrunner Jaehwa Jung
              Reporter:
              blrunner Jaehwa Jung
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development