> On Oct. 12, 2017, 5:34 p.m., Sahil Takiar wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java > > Line 387 (original), 411 (patched) > > <https://reviews.apache.org/r/62693/diff/1/?file=1839860#file1839860line412> > > > > What is this for?
The 'questions' list is used to create the list containing 'IN list values'. If not using PreparedStatements these would be actual values. In the PreparedStatement case they are a list of ?. This is arguably ugly but it allows us to use common code for PreparedStatements and (unprepared) Statements. See below for a more complete explanantion. > On Oct. 12, 2017, 5:34 p.m., Sahil Takiar wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java > > Line 413 (original), 444 (patched) > > <https://reviews.apache.org/r/62693/diff/1/?file=1839860#file1839860line449> > > > > Is this necessary? No, its a mistake, thanks > On Oct. 12, 2017, 5:34 p.m., Sahil Takiar wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java > > Line 432 (original), 475 (patched) > > <https://reviews.apache.org/r/62693/diff/1/?file=1839860#file1839860line480> > > > > If we are changing this, should we just use try-with-resources. I agree try-with-resources is great, I wanted to mimimize my changes > On Oct. 12, 2017, 5:34 p.m., Sahil Takiar wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnUtils.java > > Lines 158 (patched) > > <https://reviews.apache.org/r/62693/diff/1/?file=1839861#file1839861line158> > > > > why is a new return value necessary? We are looking at code that generates IN clauses: select count(*) from TXNS where (TXN_ID in (1,2,3,4,5)) and TXN_STATE = 'o' There are limits on how many values you can have in an IN clause (like maybe 1000), and the code knows something about that. If you ask it to generate code for a lot of values then it returns multiple queries: select count(*) from TXNS where (TXN_ID in (1,2,3,4,5)) and TXN_STATE = 'o' select count(*) from TXNS where (TXN_ID in (1001,1002,1003) and TXN_STATE = 'o' My change involves using the same logic to build PreparedStatements. These look like: select count(*) from TXNS where (TXN_ID in (?,?,?,?,?)) and TXN_STATE = 'o' select count(*) from TXNS where (TXN_ID in (?,?,?)) and TXN_STATE = 'o' The difference is that with PreparedStatements the code must also subsequently call pStmt.setLong(paramNum, value) The right number of times for each query. So the new method buildQueryWithINClauseStrings, in addition to building the list of queries also returns a corresponding list of the number of ? i n the the generated in clause. - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62693/#review187818 ----------------------------------------------------------- On Sept. 29, 2017, 4:51 p.m., Andrew Sherman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62693/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2017, 4:51 p.m.) > > > Review request for hive. > > > Repository: hive-git > > > Description > ------- > > Add a unit test which exercises CompactionTxnHandler.markFailed() and change > it to use PreparedStament. > Add test for checkFailedCompactions() and change it to use PreparedStatement > Add a unit test which exercises purgeCompactionHistory(). > Add buildQueryWithINClauseStrings() which is suitable for building in clauses > for PreparedStatement > Add test code to TestTxnUtils to tickle code in > TxnUtils.buildQueryWithINClauseStrings() so that it produces multiple queries. > Change markCleaned() to use PreparedStatement > > > Diffs > ----- > > beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java > 84963af10ec13979a7b3976be434efbc21cf2382 > > metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java > 60839faa352cbf959041a455e9e780dfca0afdc3 > metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnUtils.java > 30b155f3b3311fed6cd79e46a5b2abcee9927d91 > metastore/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnUtils.java > 1497c00e5dc77c02e53767b014a23e5fd8cb5b29 > > ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java > f8ae86bea3fe78374c0e0487d66c661f4f0d78ff > > > Diff: https://reviews.apache.org/r/62693/diff/1/ > > > Testing > ------- > > > Thanks, > > Andrew Sherman > >
