[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

[GitBox <gi...@apache.org>]

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