Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9134 )
Change subject: IMPALA-5717: Support for reading ORC data files ...................................................................... Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/9134/4/be/src/exec/hdfs-scanner.h File be/src/exec/hdfs-scanner.h: http://gerrit.cloudera.org:8080/#/c/9134/4/be/src/exec/hdfs-scanner.h@379 PS4, Line 379: virtual why make this virtual? This subroutine interface is pretty ill-defined, so I think we should avoid making it overridable. I don't think it's needed to be virtual anyway, since only the subclasses call it, so you could always just not use it from the orc scanner. But, it looks like the orc scanner version is the same implementation anyway... -- To view, visit http://gerrit.cloudera.org:8080/9134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7b6ae4ce3b9ee8125b21993702faa87537790a4 Gerrit-Change-Number: 9134 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Mon, 12 Mar 2018 19:58:41 +0000 Gerrit-HasComments: Yes
