ZhangYao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14107 )
Change subject: KUDU-2921: Exposing the table statistics to spark relation. ...................................................................... Patch Set 7: (6 comments) http://gerrit.cloudera.org:8080/#/c/14107/6/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala: http://gerrit.cloudera.org:8080/#/c/14107/6/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala@33 PS6, Line 33: import org.slf4j.Logger > Nit: Single import per line Done http://gerrit.cloudera.org:8080/#/c/14107/6/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala@288 PS6, Line 288: override def sizeInBytes: Long = estimatedSize > Is there a jira tracking the improvement for factoring predicates and proje Ok, I will create one. And TODO added here. http://gerrit.cloudera.org:8080/#/c/14107/6/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduReadOptions.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduReadOptions.scala: http://gerrit.cloudera.org:8080/#/c/14107/6/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduReadOptions.scala@35 PS6, Line 35: * server to ensure that scanners do not time out > Please move the documentation for sizeFactor here. Done http://gerrit.cloudera.org:8080/#/c/14107/6/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduReadOptions.scala@49 PS6, Line 49: scanRequestTimeoutMs: Option[Long] = None, > I am not 100% sure how this configuration is really useful/helpful. Given t Yeah, the usage of factor needs user to know the detail about Relation and the spark plan maybe it's not suitable and hard to guide. http://gerrit.cloudera.org:8080/#/c/14107/6/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala: http://gerrit.cloudera.org:8080/#/c/14107/6/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala@32 PS6, Line 32: import org.apache.kudu.client.CreateTableOptions > Nit: One import per line is preferred. Here and below. Done http://gerrit.cloudera.org:8080/#/c/14107/6/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/14107/6/src/kudu/master/catalog_manager.cc@271 PS6, Line 271: DEFINE_bool(mock_table_metrics_for_testing, false, > Do these flags need to be marked as hidden? Yeah, Adar said the same thing. According to the comment about Flag tags, tagged as unsafe will exclude the flag automatically even if it wasn't marked as 'hidden'. I think using hidden maybe more clear. Done. -- To view, visit http://gerrit.cloudera.org:8080/14107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7742a76708f989b0ccc8ba417f3390013e260175 Gerrit-Change-Number: 14107 Gerrit-PatchSet: 7 Gerrit-Owner: ZhangYao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: ZhangYao <[email protected]> Gerrit-Comment-Date: Fri, 06 Sep 2019 05:38:48 +0000 Gerrit-HasComments: Yes
