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 <[email protected]> Authored: Wed Jul 20 16:14:10 2016 -0700 Committer: Kavinder Dhaliwal <[email protected]> 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
