Re: thousands comma numeric formatting in psql

Thanks, fixed, and applied.  I also centralized the malloc into a
function.  We could use pg_malloc, but that doesn't export out to
/scripts, where this is shared.  We will fix that some day, but not
right now.

---------------------------------------------------------------------------

Eugen Nedelcu wrote:
> On Mon, Jul 11, 2005 at 11:37:17PM -0400, Bruce Momjian wrote:
> > 
> > Did you do a 'make clean' before testing?  I can't reproduce any failure
> > here.  Can you supply a number that is failing, like this:
> > 
> > 	SELECT 1000000;
> > 
> > ---------------------------------------------------------------------------
> > 
> 
> The bug is in the following code:
> 
> new_str = malloc(new_len) and every one of
> char *my_cell = malloc(....)
> 
> Ring a bell?
> 
> The corect form is:
> 
> new_str = malloc(new_len + 1),
> char *my_cell = malloc(.... + 1)
> 
> because of the null character that must terminate the string (\0)
> 
> The error apears of course randomly (more probably for a query which 
> returns many rows)
> 
> Regarding locale aproach, it is trivial to replace langinfo with
> localeconv:
> 
> struct lconv *l = localeconv();
> char *dec_point = l->decimal_point;
> 
> instead of:
> 
> #include langinfo.h
> char *dec_point = nl_langinfo(__DECIMAL_POINT);
> 
> I used langinfo because in linux libc it is considered
> "The Elegant and Fast Way" of using locale and conforms with
> X/Open portability guide that every modern Unix follows
> (Solaris, Linux, latest FreeBSD).
> 
> Regarding locale vs. \pset numericsep, for me it doesn't matter
> which one is used. I consider the patch very usefull, and I think 
> that other guys that use psql daily for working with financial data
> will appreciate it too.
> 
> With a quick setting like \pset numericsep ',' all my numeric fields
> will appear in easy readable form. I must underline that to_char is
> a backend function and we talk here about a little psql feature which
> makes life a little easier (for me at least).
> 

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/bin/psql/print.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/print.c,v
retrieving revision 1.63
diff -c -c -r1.63 print.c
*** src/bin/psql/print.c	10 Jul 2005 15:53:42 -0000	1.63
--- src/bin/psql/print.c	14 Jul 2005 06:44:10 -0000
***************
*** 29,34 ****
--- 29,48 ----
  
  #include "mbprint.h"
  
+ static void *
+ pg_local_malloc(size_t size)
+ {
+ 	void	   *tmp;
+ 
+ 	tmp = malloc(size);
+ 	if (!tmp)
+ 	{
+ 		psql_error("out of memory\n");
+ 		exit(EXIT_FAILURE);
+ 	}
+ 	return tmp;
+ }
+ 
  static int
  num_numericseps(const char *my_str)
  {
***************
*** 46,51 ****
--- 60,66 ----
  	else
  		return int_len / 3 - 1;	/* no leading separator */
  }
+ 
  static int
  len_with_numericsep(const char *my_str)
  {
***************
*** 77,88 ****
  	if (digits_before_sep == 0)
  		new_len--;	/* no leading separator */
  
! 	new_str = malloc(new_len);
! 	if (!new_str)
! 	{
! 		fprintf(stderr, _("out of memory\n"));
! 		exit(EXIT_FAILURE);
! 	}
  
  	for (i=0, j=0; ; i++, j++)
  	{
--- 92,98 ----
  	if (digits_before_sep == 0)
  		new_len--;	/* no leading separator */
  
! 	new_str = pg_local_malloc(new_len + 1);
  
  	for (i=0, j=0; ; i++, j++)
  	{
***************
*** 167,179 ****
  		if ((opt_align[i % col_count] == 'r') && strlen(*ptr) > 0 &&
  			opt_numericsep != NULL && strlen(opt_numericsep) > 0)
  		{
! 			char *my_cell = malloc(len_with_numericsep(*ptr));
  		
- 			if (!my_cell)
- 			{
- 				fprintf(stderr, _("out of memory\n"));
- 				exit(EXIT_FAILURE);
- 			}
  			strcpy(my_cell, *ptr);
  			format_numericsep(my_cell, opt_numericsep);
  			fputs(my_cell, fout);
--- 177,184 ----
  		if ((opt_align[i % col_count] == 'r') && strlen(*ptr) > 0 &&
  			opt_numericsep != NULL && strlen(opt_numericsep) > 0)
  		{
! 			char *my_cell = pg_local_malloc(len_with_numericsep(*ptr) + 1);
  		
  			strcpy(my_cell, *ptr);
  			format_numericsep(my_cell, opt_numericsep);
  			fputs(my_cell, fout);
***************
*** 249,261 ****
  		if ((opt_align[i % col_count] == 'r') && strlen(*ptr) != 0 &&
  			opt_numericsep != NULL && strlen(opt_numericsep) > 0)
  		{
! 			char *my_cell = malloc(len_with_numericsep(*ptr));
  		
- 			if (!my_cell)
- 			{
- 				fprintf(stderr, _("out of memory\n"));
- 				exit(EXIT_FAILURE);
- 			}
  			strcpy(my_cell, *ptr);
  			format_numericsep(my_cell, opt_numericsep);
  			fputs(my_cell, fout);
--- 254,261 ----
  		if ((opt_align[i % col_count] == 'r') && strlen(*ptr) != 0 &&
  			opt_numericsep != NULL && strlen(opt_numericsep) > 0)
  		{
! 			char *my_cell = pg_local_malloc(len_with_numericsep(*ptr) + 1);
  		
  			strcpy(my_cell, *ptr);
  			format_numericsep(my_cell, opt_numericsep);
  			fputs(my_cell, fout);
***************
*** 482,494 ****
  		{
  		    if (strlen(*ptr) > 0 && opt_numericsep != NULL && strlen(opt_numericsep) > 0)
  		    {
! 				char *my_cell = malloc(cell_w[i]);
  
- 				if (!my_cell)
- 				{
- 					fprintf(stderr, _("out of memory\n"));
- 					exit(EXIT_FAILURE);
- 				}
  				strcpy(my_cell, *ptr);
  				format_numericsep(my_cell, opt_numericsep);
  				fprintf(fout, "%*s%s", widths[i % col_count] - cell_w[i], "", my_cell);
--- 482,489 ----
  		{
  		    if (strlen(*ptr) > 0 && opt_numericsep != NULL && strlen(opt_numericsep) > 0)
  		    {
! 				char *my_cell = pg_local_malloc(cell_w[i] + 1);
  
  				strcpy(my_cell, *ptr);
  				format_numericsep(my_cell, opt_numericsep);
  				fprintf(fout, "%*s%s", widths[i % col_count] - cell_w[i], "", my_cell);
***************
*** 634,645 ****
  		fprintf(fout, "%s\n", title);
  
  	/* make horizontal border */
! 	divider = malloc(hwidth + dwidth + 10);
! 	if (!divider)
! 	{
! 		fprintf(stderr, _("out of memory\n"));
! 		exit(EXIT_FAILURE);
! 	}
  	divider[0] = '\0';
  	if (opt_border == 2)
  		strcat(divider, "+-");
--- 629,635 ----
  		fprintf(fout, "%s\n", title);
  
  	/* make horizontal border */
! 	divider = pg_local_malloc(hwidth + dwidth + 10);
  	divider[0] = '\0';
  	if (opt_border == 2)
  		strcat(divider, "+-");
***************
*** 661,675 ****
  		{
  			if (!opt_barebones)
  			{
! 				char	   *record_str = malloc(32);
  				size_t		record_str_len;
  
- 				if (!record_str)
- 				{
- 					fprintf(stderr, _("out of memory\n"));
- 					exit(EXIT_FAILURE);
- 				}
- 
  				if (opt_border == 0)
  					snprintf(record_str, 32, "* Record %d", record++);
  				else
--- 651,659 ----
  		{
  			if (!opt_barebones)
  			{
! 				char	   *record_str = pg_local_malloc(32);
  				size_t		record_str_len;
  
  				if (opt_border == 0)
  					snprintf(record_str, 32, "* Record %d", record++);
  				else
***************
*** 709,721 ****
  			fputs(" ", fout);
  
  		{
! 			char *my_cell = malloc(cell_w[i]);
  
- 			if (!my_cell)
- 			{
- 				fprintf(stderr, _("out of memory\n"));
- 				exit(EXIT_FAILURE);
- 			}
  			strcpy(my_cell, *ptr);
  			if ((opt_align[i % col_count] == 'r') && strlen(*ptr) != 0 &&
  				opt_numericsep != NULL && strlen(opt_numericsep) > 0)
--- 693,700 ----
  			fputs(" ", fout);
  
  		{
! 			char *my_cell = pg_local_malloc(cell_w[i] + 1);
  
  			strcpy(my_cell, *ptr);
  			if ((opt_align[i % col_count] == 'r') && strlen(*ptr) != 0 &&
  				opt_numericsep != NULL && strlen(opt_numericsep) > 0)
***************
*** 855,867 ****
  		else if ((opt_align[i % col_count] == 'r') && strlen(*ptr) != 0 &&
  				 opt_numericsep != NULL && strlen(opt_numericsep) > 0)
  		{
! 			char *my_cell = malloc(len_with_numericsep(*ptr));
  
- 			if (!my_cell)
- 			{
- 				fprintf(stderr, _("out of memory\n"));
- 				exit(EXIT_FAILURE);
- 			}
  		    strcpy(my_cell, *ptr);
  		    format_numericsep(my_cell, opt_numericsep);
  		    html_escaped_print(my_cell, fout);
--- 834,841 ----
  		else if ((opt_align[i % col_count] == 'r') && strlen(*ptr) != 0 &&
  				 opt_numericsep != NULL && strlen(opt_numericsep) > 0)
  		{
! 			char *my_cell = pg_local_malloc(len_with_numericsep(*ptr) + 1);
  
  		    strcpy(my_cell, *ptr);
  		    format_numericsep(my_cell, opt_numericsep);
  		    html_escaped_print(my_cell, fout);
***************
*** 946,958 ****
  		else if ((opt_align[i % col_count] == 'r') && strlen(*ptr) != 0 &&
  			opt_numericsep != NULL && strlen(opt_numericsep) > 0)
  		{
! 			char *my_cell = malloc(len_with_numericsep(*ptr));
  		    
- 			if (!my_cell)
- 			{
- 				fprintf(stderr, _("out of memory\n"));
- 				exit(EXIT_FAILURE);
- 			}
  		    strcpy(my_cell, *ptr);
  		    format_numericsep(my_cell, opt_numericsep);
  		    html_escaped_print(my_cell, fout);
--- 920,927 ----
  		else if ((opt_align[i % col_count] == 'r') && strlen(*ptr) != 0 &&
  			opt_numericsep != NULL && strlen(opt_numericsep) > 0)
  		{
! 			char *my_cell = pg_local_malloc(len_with_numericsep(*ptr) + 1);
  		    
  		    strcpy(my_cell, *ptr);
  		    format_numericsep(my_cell, opt_numericsep);
  		    html_escaped_print(my_cell, fout);
***************
*** 1646,1657 ****
  			exit(EXIT_FAILURE);
  		}
  
! 		footers[0] = malloc(100);
! 		if (!footers[0])
! 		{
! 			fprintf(stderr, _("out of memory\n"));
! 			exit(EXIT_FAILURE);
! 		}
  		if (PQntuples(result) == 1)
  			snprintf(footers[0], 100, _("(1 row)"));
  		else
--- 1615,1621 ----
  			exit(EXIT_FAILURE);
  		}
  
! 		footers[0] = pg_local_malloc(100);
  		if (PQntuples(result) == 1)
  			snprintf(footers[0], 100, _("(1 row)"));
  		else