Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16082 )
Change subject: IMPALA-9859: Full ACID Milestone 4: Part 1 Reading modified tables (primitive types) ...................................................................... Patch Set 9: (3 comments) http://gerrit.cloudera.org:8080/#/c/16082/9/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java: http://gerrit.cloudera.org:8080/#/c/16082/9/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@45 PS9, Line 45: import org.apache.impala.analysis.Path.PathType; > nit: keep the import list sorted in groups (usually the IDE will do this fo Yeah, I use my IDE to do that for me. Interestingly, even if I delete this line, and ask VSCode to import PathType for me, it puts the import to this line again... Same with HdfsPartition.FileDescriptor. Seems like if an import path has more components it can confuse vscode. Anyway, fixed it manually :D http://gerrit.cloudera.org:8080/#/c/16082/9/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1418 PS9, Line 1418: if (addAcidSlotsIfNeeded(analyzer, hdfsTblRef, partitions)) { > nit: what about merging this if-statement with its outer scope so they are Done http://gerrit.cloudera.org:8080/#/c/16082/9/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1518 PS9, Line 1518: // Let's separate insert delta File Descriptors from delete delta FDs. > I think we should separate the file descriptors in catalogd after loading t I implemented it in PS10, but I'm afraid that the change became a bit too invasive. An alternative approach is just to add the genInsertDeltaPartition() and genDeleteDeltaPartition() to the partition classes and those would filter out the unneeded file descs and return a new Partition object. This way the change would be smaller, but we'd still have to generate these insert/delta partitions for each query. What do you think? -- To view, visit http://gerrit.cloudera.org:8080/16082 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I15c8feabf40be1658f3dd46883f5a1b2aa5d0659 Gerrit-Change-Number: 16082 Gerrit-PatchSet: 9 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Mon, 06 Jul 2020 15:46:36 +0000 Gerrit-HasComments: Yes
