Dan Hecht 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: (1 comment) 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@228 PS11, Line 228: 4: optional THdfsFileSplitGeneratorSpec hdfs_file_split_generator_spec > we currently put all scan ranges from all partitions into the same list. pa Right, but what I'm thinking is replace list<Planner.TScanRangeLocationList> with list<TSplitConcreteOrGenSpec> where that thing is basically a union: // Needs better name struct TSplitConcreteOrGenSpec { // Only set for concrete range. optional Planner.TScanRangeLocationList; // Only set for generated spec optional THdfsFileSplitGeneratorSpec } After all, once we've traversed in TScanRangeLocationList we're now talking about physical locations and such, so that stuff seems like it belongs on the "concrete" side of the divide. Maybe I'm still not seeing what you mean by "least damage" though. "Least damage" in what sense? -- 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:30:42 +0000 Gerrit-HasComments: Yes
