> On March 11, 2015, 12:29 a.m., Parth Chandra wrote: > > contrib/native/client/src/clientlib/drillClient.cpp, line 354 > > <https://reviews.apache.org/r/31876/diff/1/?file=889993#file889993line354> > > > > If your intention is to set the applications' pointer to null, then you > > must pass the address of the pointer. > > > > void DrillClient::freeRecordBatch(RecordBatch** ppRecordBatch){ > > delete *ppRecordBatch; > > *ppRecordBatch=NULL; > > }
Oops! You are right. Thanks for catching it. I think I don't needed the delete statement. I needed this API because in the driver we delete the record batch as soon as it is processed. We do not really need to do this and can wait for the destructor to do it. > On March 11, 2015, 12:29 a.m., Parth Chandra wrote: > > contrib/native/client/src/include/drill/drillClient.hpp, line 311 > > <https://reviews.apache.org/r/31876/diff/1/?file=889994#file889994line311> > > > > This is required on Windows but not on Linux. Can you add a link to the > > explanation? Record batch objects are created in DrillClient and we can’t delete them in Driver side because the driver heap manger does not have access to the Dll heap manager. I guess it would be the same for Linux and Windows with regard to executables vs .so and .dylib heap separations. So basically I think this function could be an optimization or not necessary. If you think deletion of the recordbatches should be left to the destructor or somewhere else in C++ client we will go in that direction. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31876/#review76017 ----------------------------------------------------------- On March 10, 2015, 8:44 p.m., Alexander zarei wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31876/ > ----------------------------------------------------------- > > (Updated March 10, 2015, 8:44 p.m.) > > > Review request for drill, Norris Lee, Parth Chandra, and Xiao Meng. > > > Repository: drill-git > > > Description > ------- > > DRILL-2415: Export Drill C++ Client symbols so as to provide dynamic linking > > > Diffs > ----- > > contrib/native/client/src/clientlib/drillClient.cpp 878dad4 > contrib/native/client/src/include/drill/drillClient.hpp 19fec69 > contrib/native/client/src/include/drill/recordBatch.hpp 4abc2de > > Diff: https://reviews.apache.org/r/31876/diff/ > > > Testing > ------- > > Built and tested on Windows Win32/x64, Linux x86/x8664 and Mac OS X Darwin > Universal platforms; > > Tested by query submitter with > > ./querySubmitter query='SELECT * FROM `hive43`.`default`.`orders` limit > 5000;SELEct * from INFORMATION_SCHEMA.SCHEMATA' type=sql > connectStr=local=192.168.39.44:31010 api=async logLevel=fatal > test.txt > > (please note that for Windows double quotations needed e.g. "select * ...") > > and comparing the result files with the previous version of C++ client; > > Also thoroughly tested by automated and manual testing of Drill ODBC driver. > > Thanks, > Alex > > > Thanks, > > Alexander zarei > >
