Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/8523 )
Change subject: IMPALA-5931: Generates scan ranges in planner for s3/adls ...................................................................... Patch Set 11: (2 comments) http://gerrit.cloudera.org:8080/#/c/8523/11/common/thrift/PlanNodes.thrift File common/thrift/PlanNodes.thrift: http://gerrit.cloudera.org:8080/#/c/8523/11/common/thrift/PlanNodes.thrift@206 PS11, Line 206: Hdfs > I wonder if we should just drop Hdfs from this name given that this is used yeah, I just extended the current naming. its a good point to change it so I'll do that since I don't see a good reason now to carry along the current naming. http://gerrit.cloudera.org:8080/#/c/8523/11/common/thrift/PlanNodes.thrift@228 PS11, Line 228: 4: optional THdfsFileSplitGeneratorSpec hdfs_file_split_generator_spec > Isn't it that the frontend generates either a list of (concrete) scan range we currently put all scan ranges from all partitions into the same list. partitions may differ in their file system, so from the hdfs scan node's perspective, it needs to create a list that can have a mix of concrete and generated ranges. we'd like to handle both cases uniformly (see L205) so doing it this way in this change does the least damage to the part that is not changed (concrete hdfs scan ranges). -- To view, visit http://gerrit.cloudera.org:8080/8523 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5 Gerrit-Change-Number: 8523 Gerrit-PatchSet: 11 Gerrit-Owner: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Wed, 07 Mar 2018 23:15:26 +0000 Gerrit-HasComments: Yes
