Mailing-List: contact commits-help@hawq.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hawq.incubator.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: kavinder@apache.org
To: commits@hawq.incubator.apache.org
Message-Id: <7e954238afd043288e84ee44789f8309@git.apache.org>
Subject: incubator-hawq git commit: HAWQ-944. Implement new pg_ltoa function
 as per postgres
Date: Fri, 24 Feb 2017 17:28:06 +0000 (UTC)
archived-at: Fri, 24 Feb 2017 17:28:12 -0000

Repository: incubator-hawq
Updated Branches:
  refs/heads/master 6bc9fd7aa -> 6508ebb84


HAWQ-944. Implement new pg_ltoa function as per postgres


Project: http://git-wip-us.apache.org/repos/asf/incubator-hawq/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-hawq/commit/6508ebb8
Tree: http://git-wip-us.apache.org/repos/asf/incubator-hawq/tree/6508ebb8
Diff: http://git-wip-us.apache.org/repos/asf/incubator-hawq/diff/6508ebb8

Branch: refs/heads/master
Commit: 6508ebb846ee00c94bcd7ebe6c21e52c65d49b61
Parents: 6bc9fd7
Author: Kavinder Dhaliwal <kavinderd@gmail.com>
Authored: Wed Jul 20 16:14:10 2016 -0700
Committer: Kavinder Dhaliwal <kavinderd@gmail.com>
Committed: Fri Feb 24 09:27:06 2017 -0800

----------------------------------------------------------------------
 configure                                       |   5 +
 configure.in                                    |   4 +
 src/backend/access/external/pxfheaders.c        |   5 +-
 src/backend/access/external/pxfutils.c          |   2 +-
 .../access/external/test/ha_config_test.c       |   4 +-
 src/backend/commands/copy.c                     |   2 +-
 src/backend/commands/tablecmds.c                |   4 +-
 src/backend/utils/adt/numutils.c                | 112 +++++++++----------
 src/include/pg_config.h.in                      |   3 +
 9 files changed, 71 insertions(+), 70 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/6508ebb8/configure
----------------------------------------------------------------------
diff --git a/configure b/configure
index b5dca4f..14f88e4 100755
--- a/configure
+++ b/configure
@@ -15105,6 +15105,11 @@ cat >>confdefs.h <<_ACEOF
 _ACEOF
 
 
+# Define the length of char bytes needed to represent int32
+
+$as_echo "#define INT32_CHAR_SIZE 12" >>confdefs.h
+
+
 # Now we have checked all the reasons to replace snprintf
 if test $pgac_need_repl_snprintf = yes; then
 

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/6508ebb8/configure.in
----------------------------------------------------------------------
diff --git a/configure.in b/configure.in
index 2f35e16..14e2f6a 100644
--- a/configure.in
+++ b/configure.in
@@ -1898,6 +1898,10 @@ AC_DEFINE_UNQUOTED(INT64_FORMAT, $INT64_FORMAT,
 AC_DEFINE_UNQUOTED(UINT64_FORMAT, $UINT64_FORMAT,
                    [Define to the appropriate snprintf format for unsigned 64-bit ints, if any.])
 
+# Define the length of char bytes needed to represent int32
+AC_DEFINE(INT32_CHAR_SIZE, 12,
+	  [Define number of bytes required to represent an Int32 in Char format])
+
 # Now we have checked all the reasons to replace snprintf
 if test $pgac_need_repl_snprintf = yes; then
   AC_DEFINE(USE_REPL_SNPRINTF, 1, [Use replacement snprintf() functions.])

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/6508ebb8/src/backend/access/external/pxfheaders.c
----------------------------------------------------------------------
diff --git a/src/backend/access/external/pxfheaders.c b/src/backend/access/external/pxfheaders.c
index a60dc48..8e91644 100644
--- a/src/backend/access/external/pxfheaders.c
+++ b/src/backend/access/external/pxfheaders.c
@@ -140,9 +140,8 @@ static void add_alignment_size_httpheader(CHURL_HEADERS headers)
  */
 static void add_tuple_desc_httpheader(CHURL_HEADERS headers, Relation rel)
 {	
-    char long_number[sizeof(int32) * 8];
-
-    StringInfoData formatter;
+    char long_number[INT32_CHAR_SIZE];
+    StringInfoData formatter;	
     TupleDesc tuple;		
     initStringInfo(&formatter);
 	

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/6508ebb8/src/backend/access/external/pxfutils.c
----------------------------------------------------------------------
diff --git a/src/backend/access/external/pxfutils.c b/src/backend/access/external/pxfutils.c
index fec3223..fb6b799 100644
--- a/src/backend/access/external/pxfutils.c
+++ b/src/backend/access/external/pxfutils.c
@@ -43,7 +43,7 @@ bool are_ips_equal(char *ip1, char *ip2)
 /* override port str with given new port int */
 void port_to_str(char **port, int new_port)
 {
-	char tmp[10];
+	char tmp[INT32_CHAR_SIZE];
 
 	if (!port)
 		elog(ERROR, "unexpected internal error in pxfutils.c");

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/6508ebb8/src/backend/access/external/test/ha_config_test.c
----------------------------------------------------------------------
diff --git a/src/backend/access/external/test/ha_config_test.c b/src/backend/access/external/test/ha_config_test.c
index 424c7ed..ddb8cbe 100644
--- a/src/backend/access/external/test/ha_config_test.c
+++ b/src/backend/access/external/test/ha_config_test.c
@@ -154,7 +154,7 @@ test__GPHD_HA_load_nodes__PxfServicePortIsAssigned(void **state)
 {
 	unsigned int numn = 2;
 	Namenode nns[] = { {"mdw:2080", "mdw:50070"}, {"smdw:2080", "smdw:50070"}};
-	char strPort[30] = {0};
+	char strPort[INT32_CHAR_SIZE] = {0};
 	pg_ltoa(pxf_service_port, strPort);
 	
 	will_return(hdfsGetHANamenodes, nns);
@@ -188,7 +188,7 @@ test__GPHD_HA_load_nodes__HostMissing(void **state)
 {
 	unsigned int numn = 2;
 	Namenode nns[] = { {":2080", "mdw:50070"}, {"smdw:2080", "smdw:50070"}};
-	char strPort[30] = {0};
+	char strPort[INT32_CHAR_SIZE] = {0};
 	pg_ltoa(pxf_service_port, strPort);
 	
 	will_return(hdfsGetHANamenodes, nns);

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/6508ebb8/src/backend/commands/copy.c
----------------------------------------------------------------------
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index ccdd169..d500d11 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2520,7 +2520,7 @@ CopyOneRowTo(CopyState cstate, Oid tupleOid, Datum *values, bool *isnulls)
 			if (out_functions[attnum -1].fn_oid == 39 ||  /* int2out or int4out */
 				out_functions[attnum -1].fn_oid == 43 )
 			{
-				char tmp[33];
+				char tmp[INT32_CHAR_SIZE];
 				/*
 				 * The standard postgres way is to call the output function, but that involves one or more pallocs,
 				 * and a call to sprintf, followed by a conversion to client charset.

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/6508ebb8/src/backend/commands/tablecmds.c
----------------------------------------------------------------------
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f4ce8e6..eb6944e 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -869,7 +869,7 @@ static Datum AddDefaultPageRowGroupSize(Datum relOptions, List *defList){
 			text *t;
 			const char *name = "pagesize";
 			Size len;
-			char value[20];
+			char value[INT32_CHAR_SIZE];
 			pg_ltoa(DEFAULT_PARQUET_PAGE_SIZE_PARTITION, value);
 
 			len = VARHDRSZ + strlen(name) + 1 + strlen(value);
@@ -885,7 +885,7 @@ static Datum AddDefaultPageRowGroupSize(Datum relOptions, List *defList){
 		if(rowgroupSizeSet == false){
 			text *t;
 			const char *name = "rowgroupsize";
-			char value[20];
+			char value[INT32_CHAR_SIZE];
 			Size len;
 
 			pg_ltoa(DEFAULT_PARQUET_ROWGROUP_SIZE_PARTITION, value);

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/6508ebb8/src/backend/utils/adt/numutils.c
----------------------------------------------------------------------
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index aed3e3e..0d3863b 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -112,81 +112,71 @@ pg_atoi(char *s, int size, int c)
 }
 
 /*
- *		pg_itoa			- converts a short int to its string representation
+ *		pg_itoa: converts a signed 16-bit integer to its string representation
  *
- *		Note:
- *				previously based on ~ingres/source/gutil/atoi.c
- *				now uses vendor's sprintf conversion
+ *		Caller must ensure that 'a' points to enough memory to hold the result
+ *		(at least 7 bytes, counting a leading sign and trailing NUL).
+ *
+ *		It doesn't seem worth implementing this separately.
  */
 void
 pg_itoa(int16 i, char *a)
 {
-	/* 
-	 * The standard postgres way is to sprintf, but that uses a lot of cpu.
-	 * Do a fast conversion to string instead.
-	 */
-	char tmp[33];
-	char *tp = tmp;
-	char *sp;
-	int ii = 0;
-	unsigned long v;
-	long value = i;
-	bool sign = (value < 0);;
-	if (sign)
-		v = -value;
-	else
-		v = (unsigned long)value;
-	while (v || tp == tmp)
-	{
-		ii = v % 10;
-		v = v / 10;
-		*tp++ = ii+'0';
-	}
-	sp = a;
-	if (sign)
-		*sp++ = '-';
-	while (tp > tmp)
-		*sp++ = *--tp;
-	*sp = 0;
-	
+		pg_ltoa((int32)i, a);
 }
 
 /*
- *		pg_ltoa			- converts a long int to its string representation
+ *		pg_ltoa: converts a signed 32-bit integer to its string representation
  *
- *		Note:
- *				previously based on ~ingres/source/gutil/atoi.c
- *				now uses vendor's sprintf conversion
+ *		Caller must ensure that 'a' points to enough memory to hold the result
+ *		(at least 12 bytes, counting a leading sign and trailing NUL).
+ *
+ *		This is ported from Postgres commit #4fc115b. The previous implementation
+ *		in HAWQ allocated a 33 byte char[] when converting but an int32's string
+ *		representation requires only a maximum 12.
  */
 void
 pg_ltoa(int32 l, char *a)
 {
-	/* 
-	 * The standard postgres way is to sprintf, but that uses a lot of cpu.
-	 * Do a fast conversion to string instead.
+	char *start = a;
+	bool neg = false;
+
+	/*
+	 * Avoid problems with the most negative integer not being representable
+	 * as a positive integer.
 	 */
-	char tmp[33];
-	char *tp = tmp;
-	char *sp;
-	int ii = 0;
-	unsigned long v;
-	long value = l;
-	bool sign = (value < 0);;
-	if (sign)
-		v = -value;
-	else
-		v = (unsigned long)value;
-	while (v || tp == tmp)
+	if (l == INT32_MIN)
+	{
+		memcpy(a, "-2147483648", 12);
+		return;
+	}
+	else if (l < 0)
+	{
+		l = -l;
+		neg = true;
+	}
+
+	/* Compute the result backwards. */
+	do
 	{
-		ii = v % 10;
-		v = v / 10;
-		*tp++ = ii+'0';
+		int32 remainder;
+		int32 oldval = l;
+		l /= 10;
+		remainder = oldval - l * 10;
+		*a++  = '0' + remainder;
+	} while (l != 0);
+	if (neg)
+		*a++ = '-';
+
+	/* Add trailing NUL byte. */
+	*a-- = '\0';
+
+	/* reverse string */
+	while (start < a)
+	{
+		char swap = *start;
+		*start++ = *a;
+		*a-- = swap;
 	}
-	sp = a;
-	if (sign)
-		*sp++ = '-';
-	while (tp > tmp)
-		*sp++ = *--tp;
-	*sp = 0;
-	
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/6508ebb8/src/include/pg_config.h.in
----------------------------------------------------------------------
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 10cb221..0b1ccd3 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -704,6 +704,9 @@
 /* HAWQ version as a number */
 #undef HQ_VERSION_NUM
 
+/* Define number of bytes required to represent an Int32 in Char format */
+#undef INT32_CHAR_SIZE
+
 /* Define to the appropriate snprintf format for 64-bit ints, if any. */
 #undef INT64_FORMAT