impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-3905: HdfsScanner::GetNext() for Avro, RC, and Seq scans.
Date Fri, 30 Jun 2017 05:30:45 GMT
Dan Hecht has posted comments on this change.

Change subject: IMPALA-3905: HdfsScanner::GetNext() for Avro, RC, and Seq scans.

Patch Set 8: Code-Review+2

File be/src/exec/

PS8, Line 65: ProcessSplit() will issue the files' scan ranges
            :   // and those ranges will need scanner threads, so no files are marked completed
hmm, is that stale now? i guess technically not since this now happens in GetNextInternal()
which is called by ProcessSplit()?
File be/src/exec/hdfs-scanner.h:

PS8, Line 133: ProcessSplit
what's the deal with making this non-pure? oh, I guess (most) scanners now share the same
implementation? but then why keep it virtual? I guess parquet is still different? but does
parquet actually have to be different? it seems like it could conform to the same pattern
with a bit more refactoring.

Anyway, I guess it's okay as-is for now if it's not straightforward to make parquet follow
the same pattern.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie18f57b0d3fe0052a8ccd361b6a5fcdf979d0669
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Henry Robinson <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Reviewer: anujphadke <>
Gerrit-HasComments: Yes

View raw message