> On March 8, 2018, 7:10 p.m., Prasanth_J wrote: > > bin/ext/fixacidkeyindex.sh > > Lines 32 (patched) > > <https://reviews.apache.org/r/65988/diff/1/?file=1972682#file1972682line32> > > > > nit: whitespace
Thanks, will fix. > On March 8, 2018, 7:10 p.m., Prasanth_J wrote: > > ql/src/java/org/apache/hadoop/hive/ql/io/orc/FixAcidKeyIndex.java > > Lines 227 (patched) > > <https://reviews.apache.org/r/65988/diff/1/?file=1972683#file1972683line227> > > > > what is the contract for these 3 fields? > > is rowId always monotonically increasing? > > can transactionId change within a stripe? if so is there any ordering? > > is bucket number constant within a file? > > > > based on the ordering the ordering guarantees, we may be able to max > > stats value of individual columns as opposed to seek. if there are no > > ordering guarantees don't we have to look the entire last stripe to figure > > the max value of triple<transactionId, bucket, rowId>? Eugene, any comments about this? > On March 8, 2018, 7:10 p.m., Prasanth_J wrote: > > ql/src/java/org/apache/hadoop/hive/ql/io/orc/FixAcidKeyIndex.java > > Lines 254 (patched) > > <https://reviews.apache.org/r/65988/diff/1/?file=1972683#file1972683line254> > > > > "+= +" is that an additional + at the end? intended? Oh, that is not intended, thanks for catching. > On March 8, 2018, 7:10 p.m., Prasanth_J wrote: > > ql/src/java/org/apache/hadoop/hive/ql/io/orc/FixAcidKeyIndex.java > > Lines 257 (patched) > > <https://reviews.apache.org/r/65988/diff/1/?file=1972683#file1972683line257> > > > > put this inside try..finally (or try..with..resource) to close the > > resource? Ok, will change. > On March 8, 2018, 7:10 p.m., Prasanth_J wrote: > > ql/src/java/org/apache/hadoop/hive/ql/io/orc/FixAcidKeyIndex.java > > Lines 259 (patched) > > <https://reviews.apache.org/r/65988/diff/1/?file=1972683#file1972683line259> > > > > comment doesn't seem to match code. looks like the following block is > > finalizing the rename (commit)? The call to if (isAcidKeyIndexValid(newReader)) is the confirmation that the file is fixed - finalizing the rename only happens if that check passes. I guess I can change the comment a bit. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65988/#review198895 ----------------------------------------------------------- On March 8, 2018, 5:38 p.m., Jason Dere wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65988/ > ----------------------------------------------------------- > > (Updated March 8, 2018, 5:38 p.m.) > > > Review request for hive, Eugene Koifman and Prasanth_J. > > > Bugs: HIVE-18907 > https://issues.apache.org/jira/browse/HIVE-18907 > > > Repository: hive-git > > > Description > ------- > > Create utility similar to orcfiledump to check/fix this particular acid key > index issue. > > > Diffs > ----- > > bin/ext/fixacidkeyindex.sh PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/io/orc/FixAcidKeyIndex.java > PRE-CREATION > ql/src/test/org/apache/hadoop/hive/ql/io/orc/TestFixAcidKeyIndex.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/65988/diff/1/ > > > Testing > ------- > > > Thanks, > > Jason Dere > >
