trafodion-codereview mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [trafodion] DaveBirdsall commented on a change in pull request #1821: [TRAFODION-3291] Fix core when multi-column stats are done on lots of columns
Date Wed, 03 Apr 2019 19:04:25 GMT
DaveBirdsall commented on a change in pull request #1821: [TRAFODION-3291] Fix core when multi-column
stats are done on lots of columns
URL: https://github.com/apache/trafodion/pull/1821#discussion_r271888543
 
 

 ##########
 File path: core/sql/ustat/hs_globals.cpp
 ##########
 @@ -994,7 +994,8 @@ void HSGlobalsClass::formGroupSets()
       {
          if (LM->LogNeeded())
          {
-            sprintf(LM->msg, "\tMC: GROUP (%s) has state DONT_TRY, is skipped", mgroup_set->colNames->data());
+            sprintf(LM->msg, "\tMC: GROUP (%s) has state DONT_TRY, is skipped", 
+              LM->truncate(mgroup_set->colNames->data(),sizeof(LM->msg-200)));
 
 Review comment:
   This is complicated in general. For example, some of the log messages have multiple parameters
substituted in the sprintf. I would need to do several sprintfs, each one calculating the
L from the previous substitution. There are many examples of this. Too, where two strings
are involved that may need truncation, I would need additional logic to evenly apportion the
available space to them. 
   
   My feeling is the effort does not justify the benefit.
   
   I did consider using snprintf. The disadvantage there is that it would truncate the end
of the message. That might leave out interesting and vital information that today follows
the column name list. Instead, I truncated the column name list and visually checked in all
cases that I truncated it enough that the end of the message fits within the sprintf buffer.
   
   I don't claim this is ideal. A better approach would be to completely rewrite the HSLogMan
methods to use a C++ streaming style into, say, an NAString, eliminating the need for sprintf.
I could avoid truncation completely, having HSLogMan fold the message into multiple lines
if the underlying logger has any line length limitations. However, the code changes would
be truly massive; logging is ubiquitous throughout the UPDATE STATISTICS code. Again, I did
not feel that the effort was justified. 
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message