Hive
  1. Hive
  2. HIVE-4433

Fix C++ Thrift bindings broken in HIVE-4322

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Not a Problem
    • Affects Version/s: 0.12.0
    • Fix Version/s: 0.12.0
    • Component/s: Metastore, Thrift API
    • Labels:
      None

      Issue Links

        Activity

        Hide
        Carl Steinbach added a comment -

        HIVE-4322 added a SkewedValueList structure to the Metastore Thrift IDL, along with a map field where this structure is used as a key:

        // Workaround for HIVE-4322struct SkewedValueList {
          1: list<string> skewedValueList
        }
        
        // this object holds all the information about skewed table
        struct SkewedInfo {
          1: list<string> skewedColNames, // skewed column names
          2: list<list<string>> skewedColValues, //skewed values
          3: map<SkewedValueList, string> skewedColValueLocationMaps, //skewed value to location mappings
        }
        

        It turns out that this change breaks the C++ Thrift bindings (and possibly also the bindings for other languages).

        According to the Thrift documentation here it's a really bad idea to use a struct or container type as a map key:

        N.B.: For maximal compatibility, the key type for map should be a basic type rather than a struct or container type. There are some languages which do not support more complex key types in their native map types. In addition the JSON protocol only supports key types that are base types.

        I haven't had much time to look into this, but my hunch is that we probably need to backout HIVE-4322.

        Show
        Carl Steinbach added a comment - HIVE-4322 added a SkewedValueList structure to the Metastore Thrift IDL, along with a map field where this structure is used as a key: // Workaround for HIVE-4322struct SkewedValueList { 1: list<string> skewedValueList } // this object holds all the information about skewed table struct SkewedInfo { 1: list<string> skewedColNames, // skewed column names 2: list<list<string>> skewedColValues, //skewed values 3: map<SkewedValueList, string> skewedColValueLocationMaps, //skewed value to location mappings } It turns out that this change breaks the C++ Thrift bindings (and possibly also the bindings for other languages). According to the Thrift documentation here it's a really bad idea to use a struct or container type as a map key: N.B.: For maximal compatibility, the key type for map should be a basic type rather than a struct or container type. There are some languages which do not support more complex key types in their native map types. In addition the JSON protocol only supports key types that are base types. I haven't had much time to look into this, but my hunch is that we probably need to backout HIVE-4322 .
        Hide
        Carl Steinbach added a comment -

        Marking this as a blocker for 0.11.0.

        Show
        Carl Steinbach added a comment - Marking this as a blocker for 0.11.0.
        Hide
        Samuel Yuan added a comment -

        The problem is that we were originally using a list as the map key, which is worse (at least for Python). Other alternatives were considered, but since the list could contain arbitrary strings, replacing it with a struct wrapper seemed to be the best solution.

        Would you mind posting a log of what's breaking in C++?

        Show
        Samuel Yuan added a comment - The problem is that we were originally using a list as the map key, which is worse (at least for Python). Other alternatives were considered, but since the list could contain arbitrary strings, replacing it with a struct wrapper seemed to be the best solution. Would you mind posting a log of what's breaking in C++?
        Hide
        Carl Steinbach added a comment -

        You can reproduce the error by running the following command:

        % ant compile-cpp -Dthrift.home=$THRIFT_HOME

        Here's the error message I get:

             [exec] g++ -m64 -DARCH64 -dynamiclib /Users/carl/Work/repos/hive-test/build/metastore/objs/ThriftHiveMetastore.o /Users/carl/Work/repos/hive-test/build/metastore/objs/hive_metastore_constants.o /Users/carl/Work/repos/hive-test/build/metastore/objs/hive_metastore_types.o /Users/carl/Work/repos/hive-test/build/service/objs/ThriftHive.o /Users/carl/Work/repos/hive-test/build/service/objs/hive_service_constants.o /Users/carl/Work/repos/hive-test/build/service/objs/hive_service_types.o /Users/carl/Work/repos/hive-test/build/ql/objs/queryplan_types.o /Users/carl/Work/repos/hive-test/build/ql/objs/queryplan_constants.o /Users/carl/Work/repos/hive-test/build/odbc/objs/hiveclient.o /Users/carl/Work/repos/hive-test/build/odbc/objs/HiveResultSet.o /Users/carl/Work/repos/hive-test/build/odbc/objs/HiveColumnDesc.o /Users/carl/Work/repos/hive-test/build/odbc/objs/HiveRowSet.o /Users/carl/Work/repos/hive-test/build/odbc/objs/hiveclienthelper.o -L/usr/local/lib -lthrift -L/usr/local/lib -lfb303 -o /Users/carl/Work/repos/hive-test/build/odbc/lib/libhiveclient.so.1.0.0 \
             [exec]         && ln -sf libhiveclient.so.1.0.0 /Users/carl/Work/repos/hive-test/build/odbc/lib/libhiveclient.so
             [exec] Undefined symbols for architecture x86_64:
             [exec]   "Apache::Hadoop::Hive::SkewedValueList::operator<(Apache::Hadoop::Hive::SkewedValueList const&) const", referenced from:
             [exec]       std::less<Apache::Hadoop::Hive::SkewedValueList>::operator()(Apache::Hadoop::Hive::SkewedValueList const&, Apache::Hadoop::Hive::SkewedValueList const&) constin hive_metastore_types.o
             [exec] ld: symbol(s) not found for architecture x86_64
             [exec] collect2: ld returned 1 exit status
             [exec] make: *** [/Users/carl/Work/repos/hive-test/build/odbc/lib/libhiveclient.so.1.0.0] Error 1
        
        BUILD FAILED
        /Users/carl/Work/repos/hive-test/build.xml:242: The following error occurred while executing this line:
        /Users/carl/Work/repos/hive-test/build.xml:109: The following error occurred while executing this line:
        /Users/carl/Work/repos/hive-test/build.xml:111: The following error occurred while executing this line:
        /Users/carl/Work/repos/hive-test/odbc/build.xml:69: exec returned: 2
        

        The problem is that STL maps require the '<' operator to be defined for the key type, and SkewedValueList doesn't satisfy that requirement.

        Show
        Carl Steinbach added a comment - You can reproduce the error by running the following command: % ant compile-cpp -Dthrift.home=$THRIFT_HOME Here's the error message I get: [exec] g++ -m64 -DARCH64 -dynamiclib /Users/carl/Work/repos/hive-test/build/metastore/objs/ThriftHiveMetastore.o /Users/carl/Work/repos/hive-test/build/metastore/objs/hive_metastore_constants.o /Users/carl/Work/repos/hive-test/build/metastore/objs/hive_metastore_types.o /Users/carl/Work/repos/hive-test/build/service/objs/ThriftHive.o /Users/carl/Work/repos/hive-test/build/service/objs/hive_service_constants.o /Users/carl/Work/repos/hive-test/build/service/objs/hive_service_types.o /Users/carl/Work/repos/hive-test/build/ql/objs/queryplan_types.o /Users/carl/Work/repos/hive-test/build/ql/objs/queryplan_constants.o /Users/carl/Work/repos/hive-test/build/odbc/objs/hiveclient.o /Users/carl/Work/repos/hive-test/build/odbc/objs/HiveResultSet.o /Users/carl/Work/repos/hive-test/build/odbc/objs/HiveColumnDesc.o /Users/carl/Work/repos/hive-test/build/odbc/objs/HiveRowSet.o /Users/carl/Work/repos/hive-test/build/odbc/objs/hiveclienthelper.o -L/usr/local/lib -lthrift -L/usr/local/lib -lfb303 -o /Users/carl/Work/repos/hive-test/build/odbc/lib/libhiveclient.so.1.0.0 \ [exec] && ln -sf libhiveclient.so.1.0.0 /Users/carl/Work/repos/hive-test/build/odbc/lib/libhiveclient.so [exec] Undefined symbols for architecture x86_64: [exec] "Apache::Hadoop::Hive::SkewedValueList::operator<(Apache::Hadoop::Hive::SkewedValueList const&) const", referenced from: [exec] std::less<Apache::Hadoop::Hive::SkewedValueList>::operator()(Apache::Hadoop::Hive::SkewedValueList const&, Apache::Hadoop::Hive::SkewedValueList const&) constin hive_metastore_types.o [exec] ld: symbol(s) not found for architecture x86_64 [exec] collect2: ld returned 1 exit status [exec] make: *** [/Users/carl/Work/repos/hive-test/build/odbc/lib/libhiveclient.so.1.0.0] Error 1 BUILD FAILED /Users/carl/Work/repos/hive-test/build.xml:242: The following error occurred while executing this line: /Users/carl/Work/repos/hive-test/build.xml:109: The following error occurred while executing this line: /Users/carl/Work/repos/hive-test/build.xml:111: The following error occurred while executing this line: /Users/carl/Work/repos/hive-test/odbc/build.xml:69: exec returned: 2 The problem is that STL maps require the '<' operator to be defined for the key type, and SkewedValueList doesn't satisfy that requirement.
        Hide
        Ashutosh Chauhan added a comment -

        Carl Steinbach The original culprit HIVE-4322 is not on 0.11. So, I think this cannot be marked as blocker for 0.11. Do you agree?

        Show
        Ashutosh Chauhan added a comment - Carl Steinbach The original culprit HIVE-4322 is not on 0.11. So, I think this cannot be marked as blocker for 0.11. Do you agree?
        Hide
        Carl Steinbach added a comment -

        Good point. I changed the fixversion to 0.12.0

        Show
        Carl Steinbach added a comment - Good point. I changed the fixversion to 0.12.0
        Hide
        Samuel Yuan added a comment -

        I'm thinking it's possible to work around this by defining '<' since it's present in the auto-generated header file. Given that other language bindings might also have been broken by HIVE-4322 though it's probably better to change the map key to a primitive type instead. I have filed HIVE-4492 to revert the original change.

        Show
        Samuel Yuan added a comment - I'm thinking it's possible to work around this by defining '<' since it's present in the auto-generated header file. Given that other language bindings might also have been broken by HIVE-4322 though it's probably better to change the map key to a primitive type instead. I have filed HIVE-4492 to revert the original change.
        Hide
        Ashutosh Chauhan added a comment -

        After HIVE-4492 rollback, this is no longer an issue.

        Show
        Ashutosh Chauhan added a comment - After HIVE-4492 rollback, this is no longer an issue.

          People

          • Assignee:
            Samuel Yuan
            Reporter:
            Carl Steinbach
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development