Hive
  1. Hive
  2. HIVE-2090

Add "DROP DATABASE ... CASCADE/RESTRICT"

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.8.0
    • Component/s: Metastore
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      A "DROP DATABASE ... FORCE" will be useful, when we use a database for isolation when doing some tests. Being able to force cleaning up the database will make test cleaning up easier.

      1. HIVE-2090.7.patch
        71 kB
        Siying Dong
      2. HIVE-2090.6.patch
        74 kB
        Siying Dong
      3. HIVE-2090.5.patch
        71 kB
        Siying Dong
      4. HIVE-2090.4.patch
        71 kB
        Siying Dong
      5. HIVE-2090.3.patch
        71 kB
        Siying Dong
      6. HIVE-2090.2.patch
        40 kB
        Siying Dong
      7. HIVE-2090.1.patch
        37 kB
        Siying Dong

        Activity

        Hide
        Carl Steinbach added a comment -

        It doesn't seem valid to extend the semantics of HiveQL simply to support a use case motivated by Hive's testing infrastructure.

        Also, from a user perspective, how do you plan to explain the difference between "DROP DATABASE x" and "DROP DATABASE x FORCE"? Should I use the former if I'm not really serious about dropping the database?

        Show
        Carl Steinbach added a comment - It doesn't seem valid to extend the semantics of HiveQL simply to support a use case motivated by Hive's testing infrastructure. Also, from a user perspective, how do you plan to explain the difference between "DROP DATABASE x" and "DROP DATABASE x FORCE"? Should I use the former if I'm not really serious about dropping the database?
        Hide
        Siying Dong added a comment -

        Carl, it's just like there is "rm" and there is "rm -r". It's not for supporting Hive's testing. It's to support testing applications that built on top of Hive. Imagine you have a daily ETL query that does some transformation from one table to another. It is natural to create a test database and do all the tests there. In the end, it will be simpler to just run a query to clean whatever on the database.

        Show
        Siying Dong added a comment - Carl, it's just like there is "rm" and there is "rm -r". It's not for supporting Hive's testing. It's to support testing applications that built on top of Hive. Imagine you have a daily ETL query that does some transformation from one table to another. It is natural to create a test database and do all the tests there. In the end, it will be simpler to just run a query to clean whatever on the database.
        Hide
        Carl Steinbach added a comment -

        Is the purpose of this ticket to make "DROP DATABASE x FORCE" behave like the ANSI version of "DROP DATABASE x", e.g. delete x along with any tables in x? As I recall the reason that DROP DATABASE does not currently adhere to ANSI semantics is that we were worried about providing in the absence of a working authorization system. Now that the authorization system exists, it seems like "DROP DATABASE x FORCE" should be the default behavior, and the current behavior should be something that you can enable via a configuration parameter, e.g. hive.exec.drop.ignorenonempty=false.

        Also, I the change to DDLTask.dropDatabase() introduces concurrency problems since you're not operating in a transaction.

        Show
        Carl Steinbach added a comment - Is the purpose of this ticket to make "DROP DATABASE x FORCE" behave like the ANSI version of "DROP DATABASE x", e.g. delete x along with any tables in x? As I recall the reason that DROP DATABASE does not currently adhere to ANSI semantics is that we were worried about providing in the absence of a working authorization system. Now that the authorization system exists, it seems like "DROP DATABASE x FORCE" should be the default behavior, and the current behavior should be something that you can enable via a configuration parameter, e.g. hive.exec.drop.ignorenonempty=false. Also, I the change to DDLTask.dropDatabase() introduces concurrency problems since you're not operating in a transaction.
        Hide
        Siying Dong added a comment -

        Looks like "drop database" have concurrency problem today too if another concurrent query creates table on the database, since it acquires no lock. Don't know how to resolve it. But to this patch, at least I can acquire locks to all tables on the database. It doesn't resolve today's concurrency problem though.

        Show
        Siying Dong added a comment - Looks like "drop database" have concurrency problem today too if another concurrent query creates table on the database, since it acquires no lock. Don't know how to resolve it. But to this patch, at least I can acquire locks to all tables on the database. It doesn't resolve today's concurrency problem though.
        Hide
        John Sichi added a comment -

        ANSI standard is DROP SCHEMA CASCADE/RESTRICT (with default RESTRICT).

        Carl, I think you're referring to MySQL DROP DATABASE, which blows everything away.

        Show
        John Sichi added a comment - ANSI standard is DROP SCHEMA CASCADE/RESTRICT (with default RESTRICT). Carl, I think you're referring to MySQL DROP DATABASE, which blows everything away.
        Hide
        Carl Steinbach added a comment -

        @John: You're right, my mistake. Do people think we should do it the MySQL way or follow the ANSI standard. I'm fine either way.

        Show
        Carl Steinbach added a comment - @John: You're right, my mistake. Do people think we should do it the MySQL way or follow the ANSI standard. I'm fine either way.
        Hide
        John Sichi added a comment -

        I'd say go with default RESTRICT for safety.

        Show
        John Sichi added a comment - I'd say go with default RESTRICT for safety.
        Hide
        Siying Dong added a comment -

        This is an in-progress patch. It fixed the syntax to "CASCADE/RESTRICT" instead of "FORCE". While we had some discussion offline and decided to do the logic in object store level, so I need to make some more changes. We'll open other issues for fixing concurrency and authorization around dropping and creating databases.

        Show
        Siying Dong added a comment - This is an in-progress patch. It fixed the syntax to "CASCADE/RESTRICT" instead of "FORCE". While we had some discussion offline and decided to do the logic in object store level, so I need to make some more changes. We'll open other issues for fixing concurrency and authorization around dropping and creating databases.
        Hide
        He Yongqiang added a comment -

        can you add authorization check for drop database in this jira?

        Show
        He Yongqiang added a comment - can you add authorization check for drop database in this jira?
        Hide
        Siying Dong added a comment -

        Moved the logic of dropping tables to ObjectStore level. The concurrency bug will be handled in separate JIRAs, HIVE-2093 and HIVE-2094.

        Show
        Siying Dong added a comment - Moved the logic of dropping tables to ObjectStore level. The concurrency bug will be handled in separate JIRAs, HIVE-2093 and HIVE-2094 .
        Hide
        Siying Dong added a comment -

        Yongqiang, adding authorization check for dropping and creating databases can take more efforts then this. I'll see whether it is easy to finish it in HIVE-2093 together with concurrency check. It doesn't sound belonging to this JIRA.

        Show
        Siying Dong added a comment - Yongqiang, adding authorization check for dropping and creating databases can take more efforts then this. I'll see whether it is easy to finish it in HIVE-2093 together with concurrency check. It doesn't sound belonging to this JIRA.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/551/
        -----------------------------------------------------------

        Review request for hive.

        Summary
        -------

        https://issues.apache.org/jira/secure/attachment/12475548/HIVE-2090.3.patch

        This addresses bug HIVE-2090.
        https://issues.apache.org/jira/browse/HIVE-2090

        Diffs


        trunk/metastore/if/hive_metastore.thrift 1088810
        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1088810
        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1088810
        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1088810
        trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1088810
        trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1088810
        trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1088810
        trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1088810
        trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1088810
        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1088810
        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 1088810
        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1088810
        trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1088810
        trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1088810
        trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1088810
        trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1088810
        trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropDatabaseDesc.java 1088810
        trunk/ql/src/test/queries/clientpositive/database.q 1088810
        trunk/ql/src/test/results/clientpositive/database.q.out 1088810

        Diff: https://reviews.apache.org/r/551/diff

        Testing
        -------

        Thanks,

        Carl

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/551/ ----------------------------------------------------------- Review request for hive. Summary ------- https://issues.apache.org/jira/secure/attachment/12475548/HIVE-2090.3.patch This addresses bug HIVE-2090 . https://issues.apache.org/jira/browse/HIVE-2090 Diffs trunk/metastore/if/hive_metastore.thrift 1088810 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1088810 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1088810 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1088810 trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1088810 trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1088810 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1088810 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1088810 trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1088810 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1088810 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 1088810 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1088810 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1088810 trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1088810 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1088810 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1088810 trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropDatabaseDesc.java 1088810 trunk/ql/src/test/queries/clientpositive/database.q 1088810 trunk/ql/src/test/results/clientpositive/database.q.out 1088810 Diff: https://reviews.apache.org/r/551/diff Testing ------- Thanks, Carl
        Hide
        Siying Dong added a comment -

        One thing to notice. When dropping all tables on a database, all files on the warehouse root of the DB are deleted. Data from tables/partitions on locations that are not under that root won't be deleted. This is kind of similar to the current approach of dropping table – data in partitions won't be deleted if their locations are not under table's locations.

        Show
        Siying Dong added a comment - One thing to notice. When dropping all tables on a database, all files on the warehouse root of the DB are deleted. Data from tables/partitions on locations that are not under that root won't be deleted. This is kind of similar to the current approach of dropping table – data in partitions won't be deleted if their locations are not under table's locations.
        Hide
        He Yongqiang added a comment -

        2 nitpick:

        1, do not remove the existing constructor "DropDatabaseDesc(String databaseName, boolean ifExists) {"

        2, remove TOK_RESTRICT if it has the same meaning with TOK_CASCADE. Use just one of them. "if (null != ast.getFirstChildWithType(TOK_CASCADE))" seems only checked TOK_CASCADE.

        3. there is no testcase?

        Show
        He Yongqiang added a comment - 2 nitpick: 1, do not remove the existing constructor "DropDatabaseDesc(String databaseName, boolean ifExists) {" 2, remove TOK_RESTRICT if it has the same meaning with TOK_CASCADE. Use just one of them. "if (null != ast.getFirstChildWithType(TOK_CASCADE))" seems only checked TOK_CASCADE. 3. there is no testcase?
        Hide
        He Yongqiang added a comment -

        sorry, just noticed that there is one testcase. pls ignore 3.

        Show
        He Yongqiang added a comment - sorry, just noticed that there is one testcase. pls ignore 3.
        Hide
        He Yongqiang added a comment -

        ok. got that TOK_RESTRICT has different meaning with TOK_CASCADE.

        will start running tests.

        Show
        He Yongqiang added a comment - ok. got that TOK_RESTRICT has different meaning with TOK_CASCADE. will start running tests.
        Hide
        He Yongqiang added a comment -

        can you just rebase?

        Show
        He Yongqiang added a comment - can you just rebase?
        Hide
        Siying Dong added a comment -

        add constructor DropdatabaseDesc(String databaseName, boolean ifExists) and rebase.

        Show
        Siying Dong added a comment - add constructor DropdatabaseDesc(String databaseName, boolean ifExists) and rebase.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/551/#review393
        -----------------------------------------------------------

        trunk/metastore/if/hive_metastore.thrift
        <https://reviews.apache.org/r/551/#comment743>

        Does changing the Thrift IDL like this break backwards compatibility with older clients?

        trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
        <https://reviews.apache.org/r/551/#comment744>

        Missing "@param force..."

        • Carl

        On 2011-04-06 01:01:26, Carl Steinbach wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/551/

        -----------------------------------------------------------

        (Updated 2011-04-06 01:01:26)

        Review request for hive.

        Summary

        -------

        https://issues.apache.org/jira/secure/attachment/12475548/HIVE-2090.3.patch

        This addresses bug HIVE-2090.

        https://issues.apache.org/jira/browse/HIVE-2090

        Diffs

        -----

        trunk/metastore/if/hive_metastore.thrift 1088810

        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1088810

        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1088810

        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1088810

        trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1088810

        trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1088810

        trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1088810

        trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1088810

        trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1088810

        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1088810

        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 1088810

        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1088810

        trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1088810

        trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1088810

        trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1088810

        trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1088810

        trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropDatabaseDesc.java 1088810

        trunk/ql/src/test/queries/clientpositive/database.q 1088810

        trunk/ql/src/test/results/clientpositive/database.q.out 1088810

        Diff: https://reviews.apache.org/r/551/diff

        Testing

        -------

        Thanks,

        Carl

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/551/#review393 ----------------------------------------------------------- trunk/metastore/if/hive_metastore.thrift < https://reviews.apache.org/r/551/#comment743 > Does changing the Thrift IDL like this break backwards compatibility with older clients? trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java < https://reviews.apache.org/r/551/#comment744 > Missing "@param force..." Carl On 2011-04-06 01:01:26, Carl Steinbach wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/551/ ----------------------------------------------------------- (Updated 2011-04-06 01:01:26) Review request for hive. Summary ------- https://issues.apache.org/jira/secure/attachment/12475548/HIVE-2090.3.patch This addresses bug HIVE-2090 . https://issues.apache.org/jira/browse/HIVE-2090 Diffs ----- trunk/metastore/if/hive_metastore.thrift 1088810 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1088810 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1088810 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1088810 trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1088810 trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1088810 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1088810 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1088810 trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1088810 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1088810 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 1088810 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1088810 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1088810 trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1088810 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1088810 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1088810 trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropDatabaseDesc.java 1088810 trunk/ql/src/test/queries/clientpositive/database.q 1088810 trunk/ql/src/test/results/clientpositive/database.q.out 1088810 Diff: https://reviews.apache.org/r/551/diff Testing ------- Thanks, Carl
        Hide
        Siying Dong added a comment -

        add parameter description in comment of dropDatabase()

        Show
        Siying Dong added a comment - add parameter description in comment of dropDatabase()
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/551/
        -----------------------------------------------------------

        (Updated 2011-04-13 01:04:53.612739)

        Review request for hive.

        Changes
        -------

        Updating diff with https://issues.apache.org/jira/secure/attachment/12476195/HIVE-2090.4.patch

        Summary
        -------

        https://issues.apache.org/jira/secure/attachment/12475548/HIVE-2090.3.patch

        This addresses bug HIVE-2090.
        https://issues.apache.org/jira/browse/HIVE-2090

        Diffs (updated)


        trunk/metastore/if/hive_metastore.thrift 1091617
        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1091617
        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1091617
        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1091617
        trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1091617
        trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1091617
        trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1091617
        trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1091617
        trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1091617
        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1091617
        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 1091617
        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropDatabaseDesc.java 1091617
        trunk/ql/src/test/queries/clientpositive/database.q 1091617
        trunk/ql/src/test/results/clientpositive/database.q.out 1091617

        Diff: https://reviews.apache.org/r/551/diff

        Testing
        -------

        Thanks,

        Carl

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/551/ ----------------------------------------------------------- (Updated 2011-04-13 01:04:53.612739) Review request for hive. Changes ------- Updating diff with https://issues.apache.org/jira/secure/attachment/12476195/HIVE-2090.4.patch Summary ------- https://issues.apache.org/jira/secure/attachment/12475548/HIVE-2090.3.patch This addresses bug HIVE-2090 . https://issues.apache.org/jira/browse/HIVE-2090 Diffs (updated) trunk/metastore/if/hive_metastore.thrift 1091617 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1091617 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1091617 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1091617 trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1091617 trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1091617 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1091617 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1091617 trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1091617 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1091617 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 1091617 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropDatabaseDesc.java 1091617 trunk/ql/src/test/queries/clientpositive/database.q 1091617 trunk/ql/src/test/results/clientpositive/database.q.out 1091617 Diff: https://reviews.apache.org/r/551/diff Testing ------- Thanks, Carl
        Hide
        He Yongqiang added a comment -

        carl, can you help review and commit this patch?

        Show
        He Yongqiang added a comment - carl, can you help review and commit this patch?
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/551/
        -----------------------------------------------------------

        (Updated 2011-04-14 00:17:09.433334)

        Review request for hive.

        Changes
        -------

        Updating diff with https://issues.apache.org/jira/secure/attachment/12476199/HIVE-2090.5.patch

        Summary (updated)
        -------

        https://issues.apache.org/jira/secure/attachment/12476199/HIVE-2090.5.patch

        This addresses bug HIVE-2090.
        https://issues.apache.org/jira/browse/HIVE-2090

        Diffs (updated)


        trunk/metastore/if/hive_metastore.thrift 1091617
        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1091617
        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1091617
        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1091617
        trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1091617
        trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1091617
        trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1091617
        trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1091617
        trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1091617
        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1091617
        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 1091617
        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropDatabaseDesc.java 1091617
        trunk/ql/src/test/queries/clientpositive/database.q 1091617
        trunk/ql/src/test/results/clientpositive/database.q.out 1091617

        Diff: https://reviews.apache.org/r/551/diff

        Testing
        -------

        Thanks,

        Carl

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/551/ ----------------------------------------------------------- (Updated 2011-04-14 00:17:09.433334) Review request for hive. Changes ------- Updating diff with https://issues.apache.org/jira/secure/attachment/12476199/HIVE-2090.5.patch Summary (updated) ------- https://issues.apache.org/jira/secure/attachment/12476199/HIVE-2090.5.patch This addresses bug HIVE-2090 . https://issues.apache.org/jira/browse/HIVE-2090 Diffs (updated) trunk/metastore/if/hive_metastore.thrift 1091617 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1091617 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1091617 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1091617 trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1091617 trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1091617 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1091617 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1091617 trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1091617 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1091617 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 1091617 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropDatabaseDesc.java 1091617 trunk/ql/src/test/queries/clientpositive/database.q 1091617 trunk/ql/src/test/results/clientpositive/database.q.out 1091617 Diff: https://reviews.apache.org/r/551/diff Testing ------- Thanks, Carl
        Hide
        Carl Steinbach added a comment -

        @Yongqiang: Will do. I updated the diff on reviewboard and will have comments ready by tomorrow.

        In the meantime, can we get an answer about the concerns I raised with modifying the Thrift IDL? I think adding a parameter to drop_database() breaks backwards compatibility with older versions since Thrift doesn't support method overloading. Can anyone confirm that this is accurate?

        Show
        Carl Steinbach added a comment - @Yongqiang: Will do. I updated the diff on reviewboard and will have comments ready by tomorrow. In the meantime, can we get an answer about the concerns I raised with modifying the Thrift IDL? I think adding a parameter to drop_database() breaks backwards compatibility with older versions since Thrift doesn't support method overloading. Can anyone confirm that this is accurate?
        Hide
        Siying Dong added a comment -

        Thrift should handle it automatically by adding a default value to missing parameter. I'll make change to make the new parameter "optional" anyway. I'll upload a new patch soon.

        Show
        Siying Dong added a comment - Thrift should handle it automatically by adding a default value to missing parameter. I'll make change to make the new parameter "optional" anyway. I'll upload a new patch soon.
        Hide
        Carl Steinbach added a comment -

        So it sounds like downversion clients can talk to upversion servers, but what about the reverse? If the client sends a message that includes the force parameter, will a downversion server simply ignore the additional parameter?

        Show
        Carl Steinbach added a comment - So it sounds like downversion clients can talk to upversion servers, but what about the reverse? If the client sends a message that includes the force parameter, will a downversion server simply ignore the additional parameter?
        Hide
        Siying Dong added a comment -

        According to this doc: http://diwakergupta.github.com/thrift-missing-guide/ , adding an 'optional' keyword will solve the problem. However, when I tried to add it, it shows '[WARNING:/data/users/sdong/www/open-source-hive3/metastore/if/hive_metastore.thrift:212] optional keyword is ignored in argument lists.'. My guess is that it is automatically handled by Thrift. I'll double check with more people about it.

        Show
        Siying Dong added a comment - According to this doc: http://diwakergupta.github.com/thrift-missing-guide/ , adding an 'optional' keyword will solve the problem. However, when I tried to add it, it shows ' [WARNING:/data/users/sdong/www/open-source-hive3/metastore/if/hive_metastore.thrift:212] optional keyword is ignored in argument lists.'. My guess is that it is automatically handled by Thrift. I'll double check with more people about it.
        Hide
        Siying Dong added a comment -

        Shall we support the reverse? when Thrift clients have been upgraded but Thrift servers didn't?

        Show
        Siying Dong added a comment - Shall we support the reverse? when Thrift clients have been upgraded but Thrift servers didn't?
        Hide
        Siying Dong added a comment -

        I just checked with someone familiar with Thrift. For a higher version client calling RPC to a slower version server, the extra parameter will just be simply ignored.

        Show
        Siying Dong added a comment - I just checked with someone familiar with Thrift. For a higher version client calling RPC to a slower version server, the extra parameter will just be simply ignored.
        Hide
        Carl Steinbach added a comment -

        @Siying: The examples in that document seem only to apply to Thrift structs. It doesn't explicitly say that this also works with parameter lists. Can you try testing out the two scenarios (downversion client/upversion server and downversion server/upversion client) and let us know if they work?

        The section at the end that discusses Versioning/Compatibility talks about adding extra fields to structs, so maybe if this doesn't work we should try to transition to a Thrift API where each method takes a single struct as input, and just wrap the list of input parameters in that struct.

        Show
        Carl Steinbach added a comment - @Siying: The examples in that document seem only to apply to Thrift structs. It doesn't explicitly say that this also works with parameter lists. Can you try testing out the two scenarios (downversion client/upversion server and downversion server/upversion client) and let us know if they work? The section at the end that discusses Versioning/Compatibility talks about adding extra fields to structs, so maybe if this doesn't work we should try to transition to a Thrift API where each method takes a single struct as input, and just wrap the list of input parameters in that struct.
        Hide
        Siying Dong added a comment -

        @Carl, I just tried both compatibility cases. Both cases work.

        Show
        Siying Dong added a comment - @Carl, I just tried both compatibility cases. Both cases work.
        Hide
        Carl Steinbach added a comment -

        @Siying: Have you attached the most recent version of this patch? I thought you were going to modify the IDL so that the force parameter is listed as optional and has a default value explicitly set.

        Show
        Carl Steinbach added a comment - @Siying: Have you attached the most recent version of this patch? I thought you were going to modify the IDL so that the force parameter is listed as optional and has a default value explicitly set.
        Hide
        Siying Dong added a comment -

        @Carl, with or without the parameter, thrift generates the same codes with a warning:

        [echo] Executing ...l/thrift/bin/thrift on metastore/if/hive_metastore.thrift
        [exec] .../metastore/if/hive_metastore.thrift:212] optional keyword is ignored in argument lists.

        I guess it doesn't makes a different. You think it's better to keep the warning or keep the 'optional' keyword?

        Show
        Siying Dong added a comment - @Carl, with or without the parameter, thrift generates the same codes with a warning: [echo] Executing ...l/thrift/bin/thrift on metastore/if/hive_metastore.thrift [exec] .../metastore/if/hive_metastore.thrift:212] optional keyword is ignored in argument lists. I guess it doesn't makes a different. You think it's better to keep the warning or keep the 'optional' keyword?
        Hide
        Carl Steinbach added a comment -

        @Siying: Please see my comments on reviewboard
        https://reviews.apache.org/r/551/

        Show
        Carl Steinbach added a comment - @Siying: Please see my comments on reviewboard https://reviews.apache.org/r/551/
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/551/#review462
        -----------------------------------------------------------

        trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java
        <https://reviews.apache.org/r/551/#comment886>

        Doesn't look like this list is referenced anywhere. Please remove.

        trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g
        <https://reviews.apache.org/r/551/#comment887>

        The message should be "restrict or cascade clause", not "if force clause". You may also want to consider splitting this into two different rules so that we can output either "restrict clause" or "cascade clause".

        trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropDatabaseDesc.java
        <https://reviews.apache.org/r/551/#comment888>

        Please replace instances of "force" with "cascade" in this file and everywhere else so that the code is aligned with the HQL grammar.

        trunk/ql/src/test/queries/clientpositive/database.q
        <https://reviews.apache.org/r/551/#comment889>

        Please add a brief comment for each block explaining what you are testing (see earlier examples in this same test).

        trunk/ql/src/test/queries/clientpositive/database.q
        <https://reviews.apache.org/r/551/#comment890>

        Please add a negative test case that attempts to drop a non-empty DB with the RESTRICT clause set. You can probably copy the HQL code in database_drop_not_empty.q

        • Carl

        On 2011-04-14 00:17:09, Carl Steinbach wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/551/

        -----------------------------------------------------------

        (Updated 2011-04-14 00:17:09)

        Review request for hive.

        Summary

        -------

        https://issues.apache.org/jira/secure/attachment/12476199/HIVE-2090.5.patch

        This addresses bug HIVE-2090.

        https://issues.apache.org/jira/browse/HIVE-2090

        Diffs

        -----

        trunk/metastore/if/hive_metastore.thrift 1091617

        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1091617

        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1091617

        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1091617

        trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1091617

        trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1091617

        trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1091617

        trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1091617

        trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1091617

        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1091617

        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 1091617

        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1091617

        trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1091617

        trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1091617

        trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1091617

        trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1091617

        trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropDatabaseDesc.java 1091617

        trunk/ql/src/test/queries/clientpositive/database.q 1091617

        trunk/ql/src/test/results/clientpositive/database.q.out 1091617

        Diff: https://reviews.apache.org/r/551/diff

        Testing

        -------

        Thanks,

        Carl

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/551/#review462 ----------------------------------------------------------- trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java < https://reviews.apache.org/r/551/#comment886 > Doesn't look like this list is referenced anywhere. Please remove. trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g < https://reviews.apache.org/r/551/#comment887 > The message should be "restrict or cascade clause", not "if force clause". You may also want to consider splitting this into two different rules so that we can output either "restrict clause" or "cascade clause". trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropDatabaseDesc.java < https://reviews.apache.org/r/551/#comment888 > Please replace instances of "force" with "cascade" in this file and everywhere else so that the code is aligned with the HQL grammar. trunk/ql/src/test/queries/clientpositive/database.q < https://reviews.apache.org/r/551/#comment889 > Please add a brief comment for each block explaining what you are testing (see earlier examples in this same test). trunk/ql/src/test/queries/clientpositive/database.q < https://reviews.apache.org/r/551/#comment890 > Please add a negative test case that attempts to drop a non-empty DB with the RESTRICT clause set. You can probably copy the HQL code in database_drop_not_empty.q Carl On 2011-04-14 00:17:09, Carl Steinbach wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/551/ ----------------------------------------------------------- (Updated 2011-04-14 00:17:09) Review request for hive. Summary ------- https://issues.apache.org/jira/secure/attachment/12476199/HIVE-2090.5.patch This addresses bug HIVE-2090 . https://issues.apache.org/jira/browse/HIVE-2090 Diffs ----- trunk/metastore/if/hive_metastore.thrift 1091617 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1091617 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1091617 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1091617 trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1091617 trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1091617 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1091617 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1091617 trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1091617 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1091617 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 1091617 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropDatabaseDesc.java 1091617 trunk/ql/src/test/queries/clientpositive/database.q 1091617 trunk/ql/src/test/results/clientpositive/database.q.out 1091617 Diff: https://reviews.apache.org/r/551/diff Testing ------- Thanks, Carl
        Hide
        Carl Steinbach added a comment -

        [exec] .../metastore/if/hive_metastore.thrift:212] optional keyword is ignored in argument lists.

        Looks like this was added here:

        commit 7816572784c7ddafc2c4350b221469af12d198fc
        Author: Mark Slee <mcslee@apache.org>
        Date:   Mon Sep 10 22:08:49 2007 +0000
        
        Modify Thrift parser to disallow optional/required keywords in argument lists
            
        Reviewed By: dreiss
            
        Test Plan: Toss an optional/required in an arg list,
        check that it's ignored and compiler spits a warning
            
            
        git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665255 
        13f79535-47bb-0310-9956-ffa450edef68
        

        The comments seem to indicate that Thrift does not actually support optional parameters or default parameter values for optional parameters, which implies that we're depending on undocumented and unintended behavior that can change at any time. Can you please ask Mark or David about this checkin and see what they say?

        Thanks!

        Show
        Carl Steinbach added a comment - [exec] .../metastore/if/hive_metastore.thrift:212] optional keyword is ignored in argument lists. Looks like this was added here: commit 7816572784c7ddafc2c4350b221469af12d198fc Author: Mark Slee <mcslee@apache.org> Date: Mon Sep 10 22:08:49 2007 +0000 Modify Thrift parser to disallow optional/required keywords in argument lists Reviewed By: dreiss Test Plan: Toss an optional/required in an arg list, check that it's ignored and compiler spits a warning git-svn-id: https: //svn.apache.org/repos/asf/incubator/thrift/trunk@665255 13f79535-47bb-0310-9956-ffa450edef68 The comments seem to indicate that Thrift does not actually support optional parameters or default parameter values for optional parameters, which implies that we're depending on undocumented and unintended behavior that can change at any time. Can you please ask Mark or David about this checkin and see what they say? Thanks!
        Hide
        David Reiss added a comment -

        Jumping in here with minimal context. This looks safe. See section 5 (specifically 5.3) of the original Thrift whitepaper. If the parameter is missing on the wire (because the client has not been updated), the server will see the default value (false in C++/Java, None/null in Python/PHP).

        We explicitly disallow the optional keyword in argument lists, because people thought it would allow them to continue writing drop_database("foo", true) even after updating their generated code, which is not the case. When clients pull in this update, they will need to update their call sites.

        Show
        David Reiss added a comment - Jumping in here with minimal context. This looks safe. See section 5 (specifically 5.3) of the original Thrift whitepaper. If the parameter is missing on the wire (because the client has not been updated), the server will see the default value (false in C++/Java, None/null in Python/PHP). We explicitly disallow the optional keyword in argument lists, because people thought it would allow them to continue writing drop_database("foo", true) even after updating their generated code, which is not the case. When clients pull in this update, they will need to update their call sites.
        Hide
        Siying Dong added a comment -

        new patch addressed Carl's comments.

        Show
        Siying Dong added a comment - new patch addressed Carl's comments.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/551/
        -----------------------------------------------------------

        (Updated 2011-04-18 19:30:43.636903)

        Review request for hive.

        Summary (updated)
        -------

        https://issues.apache.org/jira/secure/attachment/12476401/HIVE-2090.6.patch

        This addresses bug HIVE-2090.
        https://issues.apache.org/jira/browse/HIVE-2090

        Diffs (updated)


        trunk/metastore/if/hive_metastore.thrift 1091617
        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1091617
        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1091617
        trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1091617
        trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1091617
        trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1091617
        trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1091617
        trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1091617
        trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1091617
        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1091617
        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 1091617
        trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1091617
        trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropDatabaseDesc.java 1091617
        trunk/ql/src/test/queries/clientnegative/database_drop_not_empty_restrict.q PRE-CREATION
        trunk/ql/src/test/queries/clientpositive/database.q 1091617
        trunk/ql/src/test/results/clientnegative/database_drop_not_empty_restrict.q.out PRE-CREATION
        trunk/ql/src/test/results/clientpositive/database.q.out 1091617

        Diff: https://reviews.apache.org/r/551/diff

        Testing
        -------

        Thanks,

        Carl

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/551/ ----------------------------------------------------------- (Updated 2011-04-18 19:30:43.636903) Review request for hive. Summary (updated) ------- https://issues.apache.org/jira/secure/attachment/12476401/HIVE-2090.6.patch This addresses bug HIVE-2090 . https://issues.apache.org/jira/browse/HIVE-2090 Diffs (updated) trunk/metastore/if/hive_metastore.thrift 1091617 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1091617 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1091617 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1091617 trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1091617 trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1091617 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1091617 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1091617 trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1091617 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1091617 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 1091617 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1091617 trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropDatabaseDesc.java 1091617 trunk/ql/src/test/queries/clientnegative/database_drop_not_empty_restrict.q PRE-CREATION trunk/ql/src/test/queries/clientpositive/database.q 1091617 trunk/ql/src/test/results/clientnegative/database_drop_not_empty_restrict.q.out PRE-CREATION trunk/ql/src/test/results/clientpositive/database.q.out 1091617 Diff: https://reviews.apache.org/r/551/diff Testing ------- Thanks, Carl
        Hide
        Carl Steinbach added a comment -

        +1. Will commit if tests pass.

        Show
        Carl Steinbach added a comment - +1. Will commit if tests pass.
        Hide
        Carl Steinbach added a comment -

        @Siying: I'm getting errors in DDLSemanticAnalyzer when I try to apply the patch. Can you please rebase it to trunk? Thanks!

        Show
        Carl Steinbach added a comment - @Siying: I'm getting errors in DDLSemanticAnalyzer when I try to apply the patch. Can you please rebase it to trunk? Thanks!
        Hide
        Siying Dong added a comment -

        rebase

        Show
        Siying Dong added a comment - rebase
        Hide
        Carl Steinbach added a comment -

        I'm running tests now.

        Show
        Carl Steinbach added a comment - I'm running tests now.
        Hide
        Carl Steinbach added a comment -

        Committed to trunk. Thanks Siying!

        Show
        Carl Steinbach added a comment - Committed to trunk. Thanks Siying!

          People

          • Assignee:
            Siying Dong
            Reporter:
            Siying Dong
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development