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 <borokna...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Jul 2020 15:46:36 +0000
Gerrit-HasComments: Yes

Reply via email to