> On March 1, 2018, 12:40 a.m., Eugene Koifman wrote: > > ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java > > Lines 1683 (patched) > > <https://reviews.apache.org/r/65415/diff/4/?file=1965759#file1965759line1688> > > > > For full acid table, if you have no ParseDelta.isDeleteDelta(), then > > the ((non-delete) ParseDelta from getCurrentDirectories + (base | > > getOriginalFiles())) fileset should be accurate
Can you elaborate? not sure what you mean wrt this code > On March 1, 2018, 12:40 a.m., Eugene Koifman wrote: > > ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java > > Lines 1695 (patched) > > <https://reviews.apache.org/r/65415/diff/4/?file=1965759#file1965759line1700> > > > > adding ParseDelta.isDeleteDelta() seems wrong - if anything it should > > subtract from file size/row count Well, I think at least the size is more likely to be used for scan size estimation, and delete deltas would need to be scanned together with other files. I think the proper impl of stats for ACID would need to be done in separate jira and actually account properly for ACID operations. > On March 1, 2018, 12:40 a.m., Eugene Koifman wrote: > > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java > > Lines 1619 (patched) > > <https://reviews.apache.org/r/65415/diff/4/?file=1965761#file1965761line1620> > > > > should all these todos be jiras? Will file jira(s) after the final version on the patch based on all the TODOs added where it's relevant. > On March 1, 2018, 12:40 a.m., Eugene Koifman wrote: > > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java > > Lines 1697 (patched) > > <https://reviews.apache.org/r/65415/diff/4/?file=1965761#file1965761line1698> > > > > I saw a nubmer of comments/logic to this effect - probably better to > > wait for HIVE-18824 and remove these Well, it can still happen due to some bug. I'll keep the checks for safety, we'll see 0 stats if they happen to trigger. > On March 1, 2018, 12:40 a.m., Eugene Koifman wrote: > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java > > Lines 647 (patched) > > <https://reviews.apache.org/r/65415/diff/4/?file=1965778#file1965778line656> > > > > why? No ValidTxnList? yes; also the code itself is in QL > On March 1, 2018, 12:40 a.m., Eugene Koifman wrote: > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java > > Line 675 (original), 671 (patched) > > <https://reviews.apache.org/r/65415/diff/4/?file=1965778#file1965778line682> > > > > Jira? Assert? at least a Wtf... it's impossible to assert what file list is... It would be valid to call this by getting the file list from AcidUtils. I'm going to file a follow up JIRA for this. - Sergey ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65415/#review198419 ----------------------------------------------------------- On Feb. 27, 2018, 3:14 a.m., Sergey Shelukhin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65415/ > ----------------------------------------------------------- > > (Updated Feb. 27, 2018, 3:14 a.m.) > > > Review request for hive and Eugene Koifman. > > > Repository: hive-git > > > Description > ------- > > f.,v fbghdscd > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/common/HiveStatsUtils.java > df77a4a2f2 > ql/src/java/org/apache/hadoop/hive/ql/exec/MoveTask.java b490325091 > ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java fd8423129f > > ql/src/java/org/apache/hadoop/hive/ql/exec/repl/bootstrap/load/table/LoadPartitions.java > 0a82225d4a > ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 70fcd2c142 > ql/src/java/org/apache/hadoop/hive/ql/io/merge/MergeFileWork.java > 1a63d3f971 > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 8b0af3e5c8 > ql/src/java/org/apache/hadoop/hive/ql/parse/ImportSemanticAnalyzer.java > 67d05e65dd > ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java > 7d2de75315 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > cd6f1ee692 > ql/src/java/org/apache/hadoop/hive/ql/plan/BasicStatsWork.java a4e770ce95 > > ql/src/java/org/apache/hadoop/hive/ql/plan/ConditionalResolverMergeFiles.java > 8ce0cb05b6 > ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java > 946c300750 > ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsTask.java 1d7660e8b2 > ql/src/java/org/apache/hadoop/hive/ql/stats/ColStatsProcessor.java > 7591c0681b > ql/src/java/org/apache/hadoop/hive/ql/stats/Partish.java 05b0474e90 > ql/src/java/org/apache/hadoop/hive/ql/stats/fs/FSStatsAggregator.java > d84cf136d5 > ql/src/test/results/clientpositive/autoColumnStats_4.q.out 9c0e020351 > > standalone-metastore/src/main/java/org/apache/hadoop/hive/common/StatsSetupConst.java > 59190893e6 > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java > 89354a2d34 > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > c6e34a8a22 > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/Warehouse.java > 20c10607bb > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/FileUtils.java > b44ff8ce47 > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java > 50f873a013 > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java > 2599ab103e > > > Diff: https://reviews.apache.org/r/65415/diff/4/ > > > Testing > ------- > > > Thanks, > > Sergey Shelukhin > >
