Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10483 )

Change subject: IMPALA-7044: Prevent overflow when computing Parquet block size
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10483/2/be/src/exec/hdfs-parquet-table-writer.cc
File be/src/exec/hdfs-parquet-table-writer.cc:

http://gerrit.cloudera.org:8080/#/c/10483/2/be/src/exec/hdfs-parquet-table-writer.cc@789
PS2, Line 789:     stringstream ss;
             :     ss << "Minimum required block size is too large, try 
reducing the number of columns "
             :         "in the target table.";
why use stringstream if you're not going to construct a string?
(though maybe it'd be good to include num_cols in the error message, but 
consider using Substitute() if you do that).
also, let's add a comment explaining where the signed 32-bit limit comes from.



--
To view, visit http://gerrit.cloudera.org:8080/10483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e63420e5a093c0bbc789201771708865b16e138
Gerrit-Change-Number: 10483
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <thomasmarsh...@cmu.edu>
Gerrit-Comment-Date: Thu, 24 May 2018 03:24:54 +0000
Gerrit-HasComments: Yes

Reply via email to