----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5685/#review10147 -----------------------------------------------------------
Looks good overall. Most of things I commented on have to do with formatting issues and missing ASF license headers. It may be worth running all of this code through an automated formatter like astyle. odbc/src/cpp/HiveResultSet.cpp <https://reviews.apache.org/r/5685/#comment21496> I think we should try to keep the formatting consistent. * Always use braces for blocks (if, while, etc) * open brace always on the same line * "} else {" odbc/src/cpp/HiveResultSet.cpp <https://reviews.apache.org/r/5685/#comment21498> Brace should after the closing paren. odbc/src/cpp/hiveclient.def <https://reviews.apache.org/r/5685/#comment21495> Missing ASF license header. odbc/src/cpp/hiveclienthelper.h <https://reviews.apache.org/r/5685/#comment21494> Formatting: please fix the line break alignment. odbc/src/cpp/if/fb303.thrift <https://reviews.apache.org/r/5685/#comment21491> This file shouldn't be necessary. odbc/src/cpp/if/hive_metastore.thrift <https://reviews.apache.org/r/5685/#comment21492> Please remove and use metastore/if/hive_metastore.thrift instead. odbc/src/cpp/if/hive_service.thrift <https://reviews.apache.org/r/5685/#comment21490> Please remove this file and use the copy here instead: service/if/hive_service.thrift odbc/src/cpp/if/queryplan.thrift <https://reviews.apache.org/r/5685/#comment21489> This file can be removed. We should instead reference the copy in ql/if/queryplan.thrift odbc/src/driver/hiveodbc.h <https://reviews.apache.org/r/5685/#comment21487> Please remove the CDH reference in the comment and bump the version number to 0.10.0 odbc/src/driver/hiveodbc.h <https://reviews.apache.org/r/5685/#comment21488> Formatting: It would be nice if the comments for these structs were left-justified. odbc/src/driver/hiveodbc.c <https://reviews.apache.org/r/5685/#comment21501> Is is possible to reformat this so we get closer to the 100 character line limit? odbc/src/driver/hiveodbc.c <https://reviews.apache.org/r/5685/#comment21502> Formatting. odbc/src/driver/hiveodbc.c <https://reviews.apache.org/r/5685/#comment21503> Remove. odbc/src/driver/hiveodbc_win32_rc.rc <https://reviews.apache.org/r/5685/#comment21505> Missing ASF header. odbc/src/driver/libhiveodbc.def <https://reviews.apache.org/r/5685/#comment21506> Missing ASF header. - Carl Steinbach On June 30, 2012, 4:38 a.m., Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5685/ > ----------------------------------------------------------- > > (Updated June 30, 2012, 4:38 a.m.) > > > Review request for hive and Carl Steinbach. > > > Description > ------- > > Enhanced ODBC driver with limited ODBC3 compliance. This ticket HIVE-3213 > covers the source code, the build changes are tracked by HIVE-3212 > > > This addresses bug HIVE-3213. > https://issues.apache.org/jira/browse/HIVE-3213 > > > Diffs > ----- > > odbc/src/cpp/HiveConnection.h 3b2e2b1 > odbc/src/cpp/HiveResultSet.h 25eabc4 > odbc/src/cpp/HiveResultSet.cpp d3d375e > odbc/src/cpp/HiveRowSet.h ca6e6af > odbc/src/cpp/HiveRowSet.cpp 3de6124 > odbc/src/cpp/Makefile.am PRE-CREATION > odbc/src/cpp/Makefile.in PRE-CREATION > odbc/src/cpp/hiveclient.h f1af670 > odbc/src/cpp/hiveclient.cpp 450eb0b > odbc/src/cpp/hiveclient.def PRE-CREATION > odbc/src/cpp/hiveclienthelper.h 5814a03 > odbc/src/cpp/hiveconstants.h 72f1049 > odbc/src/cpp/if/fb303.thrift PRE-CREATION > odbc/src/cpp/if/hive_metastore.thrift PRE-CREATION > odbc/src/cpp/if/hive_service.thrift PRE-CREATION > odbc/src/cpp/if/queryplan.thrift PRE-CREATION > odbc/src/cpp/thriftserverconstants.h fe4bac4 > odbc/src/driver/Makefile.am PRE-CREATION > odbc/src/driver/Makefile.in PRE-CREATION > odbc/src/driver/hiveodbc.h PRE-CREATION > odbc/src/driver/hiveodbc.c PRE-CREATION > odbc/src/driver/hiveodbc_logo.ico PRE-CREATION > odbc/src/driver/hiveodbc_win32_rc.h PRE-CREATION > odbc/src/driver/hiveodbc_win32_rc.rc PRE-CREATION > odbc/src/driver/libhiveodbc.def PRE-CREATION > odbc/src/driver/libtool-version PRE-CREATION > odbc/src/test/Makefile.am PRE-CREATION > odbc/src/test/Makefile.in PRE-CREATION > odbc/src/test/hiveclienttest.c fbb4e24 > odbc/src/test/hiveodbctest.c PRE-CREATION > odbc/src/test/hivetest.h PRE-CREATION > > Diff: https://reviews.apache.org/r/5685/diff/ > > > Testing > ------- > > > Thanks, > > Prasad Mujumdar > >
