Details

    • Type: New Feature
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.10.0
    • Component/s: Catalog
    • Labels:

      Description

      See the title. PostgreSQL is also an widely used open source DBMS. Like TAJO-179, we need to support postgresql catalog store.

      1. postgresql_oct.16.diff
        16 kB
        Jihun Kang
      2. postgresql.diff
        16 kB
        Jihun Kang
      3. TAJO-233_Hyunsik_20141016.patch
        18 kB
        Hyunsik Choi

        Activity

        Hide
        ykrips Jihun Kang added a comment -

        It is developed using version 9.3-1102 of JDBC4 driver. Postgresql JDBC Driver can provide backward compatibility, so there is no issue on using older version of postgresql server.

        Show
        ykrips Jihun Kang added a comment - It is developed using version 9.3-1102 of JDBC4 driver. Postgresql JDBC Driver can provide backward compatibility, so there is no issue on using older version of postgresql server.
        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/12669670/postgresql.diff
        against master revision 1cff979.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +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 does not introduce any new Findbugs (version 2.0.3) warnings.

        -1 release audit. The applied patch generated 105 release audit warnings.

        +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-server.

        Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/501//testReport/
        Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/501//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt
        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/501//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/12669670/postgresql.diff against master revision 1cff979. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 does not introduce any new Findbugs (version 2.0.3) warnings. -1 release audit. The applied patch generated 105 release audit warnings. +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-server. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/501//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/501//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/501//console This message is automatically generated.
        Hide
        hyunsik Hyunsik Choi added a comment -

        Thank you for your contribution. I schedule this issue to 0.9.1 because 0.9.0 would be released soon. I'll review it soon.

        Show
        hyunsik Hyunsik Choi added a comment - Thank you for your contribution. I schedule this issue to 0.9.1 because 0.9.0 would be released soon. I'll review it soon.
        Hide
        hyunsik Hyunsik Choi added a comment -

        I'm reviewing the patch. I'll give comments soon.

        Show
        hyunsik Hyunsik Choi added a comment - I'm reviewing the patch. I'll give comments soon.
        Hide
        hyunsik Hyunsik Choi added a comment -

        The patch looks good to me. Thank you for your work. For most features, your patch works well. But, the patch causes the following error when partitioned table is handled:

        Caused by: com.google.protobuf.InvalidProtocolBufferException: Protocol message end-group tag did not match expected tag.
        	at com.google.protobuf.InvalidProtocolBufferException.invalidEndTag(InvalidProtocolBufferException.java:94)
        	at com.google.protobuf.CodedInputStream.checkLastTagWas(CodedInputStream.java:124)
        	at com.google.protobuf.AbstractParser.parsePartialFrom(AbstractParser.java:143)
        	at com.google.protobuf.AbstractParser.parseFrom(AbstractParser.java:176)
        	at com.google.protobuf.AbstractParser.parseFrom(AbstractParser.java:188)
        	at com.google.protobuf.AbstractParser.parseFrom(AbstractParser.java:193)
        	at com.google.protobuf.AbstractParser.parseFrom(AbstractParser.java:49)
        	at org.apache.tajo.catalog.proto.CatalogProtos$SchemaProto.parseFrom(CatalogProtos.java:1806)
        	at org.apache.tajo.catalog.store.AbstractDBStore.resultToPartitionMethodProto(AbstractDBStore.java:1952)
        	at org.apache.tajo.catalog.store.AbstractDBStore.getTable(AbstractDBStore.java:1295)
        

        TestCatalog.java is very good testing tool for catalog store. Please see the wiki page at https://cwiki.apache.org/confluence/display/TAJO/Unit+Tests. I've added an example JVM option for PostgreSQLStore. You can use TestCatalog with your PostgreSQLStore by using the following JVM option.

        -Dtajo.catalog.store.class=org.apache.tajo.catalog.store.PostgreSQLStore -Dtajo.catalog.connection.id=username -Dtajo.catalog.connection.password=password -Dtajo.catalog.uri=jdbc:postgresql://localhost/databasename
        
        Show
        hyunsik Hyunsik Choi added a comment - The patch looks good to me. Thank you for your work. For most features, your patch works well. But, the patch causes the following error when partitioned table is handled: Caused by: com.google.protobuf.InvalidProtocolBufferException: Protocol message end-group tag did not match expected tag. at com.google.protobuf.InvalidProtocolBufferException.invalidEndTag(InvalidProtocolBufferException.java:94) at com.google.protobuf.CodedInputStream.checkLastTagWas(CodedInputStream.java:124) at com.google.protobuf.AbstractParser.parsePartialFrom(AbstractParser.java:143) at com.google.protobuf.AbstractParser.parseFrom(AbstractParser.java:176) at com.google.protobuf.AbstractParser.parseFrom(AbstractParser.java:188) at com.google.protobuf.AbstractParser.parseFrom(AbstractParser.java:193) at com.google.protobuf.AbstractParser.parseFrom(AbstractParser.java:49) at org.apache.tajo.catalog.proto.CatalogProtos$SchemaProto.parseFrom(CatalogProtos.java:1806) at org.apache.tajo.catalog.store.AbstractDBStore.resultToPartitionMethodProto(AbstractDBStore.java:1952) at org.apache.tajo.catalog.store.AbstractDBStore.getTable(AbstractDBStore.java:1295) TestCatalog.java is very good testing tool for catalog store. Please see the wiki page at https://cwiki.apache.org/confluence/display/TAJO/Unit+Tests . I've added an example JVM option for PostgreSQLStore. You can use TestCatalog with your PostgreSQLStore by using the following JVM option. -Dtajo.catalog.store.class=org.apache.tajo.catalog.store.PostgreSQLStore -Dtajo.catalog.connection.id=username -Dtajo.catalog.connection.password=password -Dtajo.catalog.uri=jdbc:postgresql://localhost/databasename
        Hide
        ykrips Jihun Kang added a comment - - edited

        Hello, Hyunsik Choi.
        It comes from the PostgreSQL JDBC Driver strictly check the data type of columns. "expression_schema" column in "partition_methods" table stores data by "VARCHAR(1024)" data type and "resultToPartitionMethodProto" function retrieves this data by using "getBytes" function. On PostgreSQL server, "getBytes" and other byte type related functions only work on bytea data type. Since PostgreSQL 9, bytea format has been changed so we need to check if major version of PostgreSQL is 9 or not for selecting "escape" format of bytea. I would like to get your opinion on this issue whether we use bytea or blob data type for this column.

        Show
        ykrips Jihun Kang added a comment - - edited Hello, Hyunsik Choi . It comes from the PostgreSQL JDBC Driver strictly check the data type of columns. "expression_schema" column in "partition_methods" table stores data by "VARCHAR(1024)" data type and "resultToPartitionMethodProto" function retrieves this data by using "getBytes" function. On PostgreSQL server, "getBytes" and other byte type related functions only work on bytea data type. Since PostgreSQL 9, bytea format has been changed so we need to check if major version of PostgreSQL is 9 or not for selecting "escape" format of bytea. I would like to get your opinion on this issue whether we use bytea or blob data type for this column.
        Hide
        ykrips Jihun Kang added a comment -

        I have changed the data type of "EXPRESSION_SCHEMA" column from VARCHAR(1024) to BYTEA. I feel that it is a better solution for this issue.

        Show
        ykrips Jihun Kang added a comment - I have changed the data type of "EXPRESSION_SCHEMA" column from VARCHAR(1024) to BYTEA. I feel that it is a better solution for this issue.
        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/12675242/postgresql_oct.16.diff
        against master revision release-0.9.0-rc0-3-gd56737b.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +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 does not introduce any new Findbugs (version 2.0.3) warnings.

        -1 release audit. The applied patch generated 354 release audit warnings.

        +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-server.

        Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/515//testReport/
        Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/515//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt
        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/515//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/12675242/postgresql_oct.16.diff against master revision release-0.9.0-rc0-3-gd56737b. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 does not introduce any new Findbugs (version 2.0.3) warnings. -1 release audit. The applied patch generated 354 release audit warnings. +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-server. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/515//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/515//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/515//console This message is automatically generated.
        Hide
        hyunsik Hyunsik Choi added a comment - - edited

        +1

        The patch looks nice to me. Thank you for suggesting a nice solution. I also agree with your choice.

        I just changed PostgreSQLStore used in TestCatalog to take given username and password in jvm option. It's a trivial change. I'll commit it shortly.

        tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java
        -    // MySQLStore/MariaDB requires password
        -    if (driverClass.equals(MySQLStore.class.getCanonicalName()) || driverClass.equals(MariaDBStore.class.getCanonicalName())) {
        +    // MySQLStore/MariaDB/PostgreSQL requires username (and password).
        +    if (isConnectionIdRequired(driverClass)) {
               if (connectionId == null) {
                 throw new CatalogException(String.format("%s driver requires %s", driverClass, CatalogConstants.CONNECTION_ID));
               }
        @@ -114,6 +115,12 @@ public class TestCatalog {
               catalog.dropTable(table);
             }
          }
        +
        +  public static boolean isConnectionIdRequired(String driverClass) {
        +    return driverClass.equals(MySQLStore.class.getCanonicalName()) ||
        +           driverClass.equals(MariaDBStore.class.getCanonicalName()) ||
        +           driverClass.equals(PostgreSQLStore.class.getCanonicalName());
        +  }
        
        Show
        hyunsik Hyunsik Choi added a comment - - edited +1 The patch looks nice to me. Thank you for suggesting a nice solution. I also agree with your choice. I just changed PostgreSQLStore used in TestCatalog to take given username and password in jvm option. It's a trivial change. I'll commit it shortly. tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java - // MySQLStore/MariaDB requires password - if (driverClass.equals(MySQLStore.class.getCanonicalName()) || driverClass.equals(MariaDBStore.class.getCanonicalName())) { + // MySQLStore/MariaDB/PostgreSQL requires username (and password). + if (isConnectionIdRequired(driverClass)) { if (connectionId == null) { throw new CatalogException(String.format("%s driver requires %s", driverClass, CatalogConstants.CONNECTION_ID)); } @@ -114,6 +115,12 @@ public class TestCatalog { catalog.dropTable(table); } } + + public static boolean isConnectionIdRequired(String driverClass) { + return driverClass.equals(MySQLStore.class.getCanonicalName()) || + driverClass.equals(MariaDBStore.class.getCanonicalName()) || + driverClass.equals(PostgreSQLStore.class.getCanonicalName()); + }
        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/12675392/TAJO-233_Hyunsik_20141016.patch
        against master revision release-0.9.0-rc0-3-gd56737b.

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

        +1 tests included. The patch appears to include 1 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 does not introduce any new Findbugs (version 2.0.3) warnings.

        -1 release audit. The applied patch generated 190 release audit warnings.

        +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-server.

        Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/516//testReport/
        Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/516//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt
        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/516//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/12675392/TAJO-233_Hyunsik_20141016.patch against master revision release-0.9.0-rc0-3-gd56737b. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 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 does not introduce any new Findbugs (version 2.0.3) warnings. -1 release audit. The applied patch generated 190 release audit warnings. +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-server. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/516//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/516//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/516//console This message is automatically generated.
        Hide
        hyunsik Hyunsik Choi added a comment -

        I just committed it to master branch. Thank you Jihun for your contribution.

        Show
        hyunsik Hyunsik Choi added a comment - I just committed it to master branch. Thank you Jihun for your contribution.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #414 (See https://builds.apache.org/job/Tajo-master-build/414/)
        TAJO-233: Support PostgreSQL CatalogStore. (Jihun Kang via hyunsik) (hyunsik: rev 2e32f5a6271d6952f77839ff639e11537af03ebf)

        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/tablespaces.sql
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/columns.sql
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/table_properties.sql
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/partitions.sql
        • CHANGES
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/tables.sql
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/partition_methods.sql
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/databases.sql
        • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/PostgreSQLStore.java
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/stats.sql
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/indexes.sql
        • tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #414 (See https://builds.apache.org/job/Tajo-master-build/414/ ) TAJO-233 : Support PostgreSQL CatalogStore. (Jihun Kang via hyunsik) (hyunsik: rev 2e32f5a6271d6952f77839ff639e11537af03ebf) tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/tablespaces.sql tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/columns.sql tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/table_properties.sql tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/partitions.sql CHANGES tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/tables.sql tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/partition_methods.sql tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/databases.sql tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/PostgreSQLStore.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/stats.sql tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/indexes.sql tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-CODEGEN-build #56 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/56/)
        TAJO-233: Support PostgreSQL CatalogStore. (Jihun Kang via hyunsik) (hyunsik: rev 2e32f5a6271d6952f77839ff639e11537af03ebf)

        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/partitions.sql
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/databases.sql
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/table_properties.sql
        • tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/indexes.sql
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/columns.sql
        • tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/PostgreSQLStore.java
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/partition_methods.sql
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/tablespaces.sql
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/stats.sql
        • CHANGES
        • tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/tables.sql
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-CODEGEN-build #56 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/56/ ) TAJO-233 : Support PostgreSQL CatalogStore. (Jihun Kang via hyunsik) (hyunsik: rev 2e32f5a6271d6952f77839ff639e11537af03ebf) tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/partitions.sql tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/databases.sql tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/table_properties.sql tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/indexes.sql tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/columns.sql tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/PostgreSQLStore.java tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/partition_methods.sql tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/tablespaces.sql tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/stats.sql CHANGES tajo-catalog/tajo-catalog-server/src/main/resources/schemas/postgresql/tables.sql

          People

          • Assignee:
            ykrips Jihun Kang
            Reporter:
            hyunsik Hyunsik Choi
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development