On 11/30/2014 11:45 AM, Tom Lane wrote:
Andrew Dunstan <and...@dunslane.net> writes:
what do you want to do about this? In the back branches, exposing a
function like this would be an API change, wouldn't it? Perhaps there we
just need to pick up the 100 lines or so involved from json.c and copy
them into hstore_io.c, suitably modified. In the development branch I
thing adding the function to the API is the best way.
If we're going to do it by calling some newly-exposed function, I'd be
inclined to fix it the same way in the back branches. Otherwise the
discrepancy between the branches is a big back-patching hazard.
(For instance, if we realize we need to fix a bug in the numeric-parsing
code, what are the odds that we remember to fix hstore's additional copy
in the back branches?)
The "API break" isn't a big issue imo. The net effect would be that eg
hstore 9.3.6 wouldn't work against a 9.3.5 server. We do that sort of
thing *all the time* --- at least twice in the past year, according to
a quick scan of the commit logs. If you were changing or removing a
function that third-party code might depend on, it'd be problematic,
but an addition has no such risk.
OK, here's the patch.
cheers
andrew
diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
index 6ce3047..ee3d696 100644
--- a/contrib/hstore/hstore_io.c
+++ b/contrib/hstore/hstore_io.c
@@ -1240,7 +1240,6 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
int count = HS_COUNT(in);
char *base = STRPTR(in);
HEntry *entries = ARRPTR(in);
- bool is_number;
StringInfoData tmp,
dst;
@@ -1267,48 +1266,9 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
appendStringInfoString(&dst, "false");
else
{
- is_number = false;
resetStringInfo(&tmp);
appendBinaryStringInfo(&tmp, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
-
- /*
- * don't treat something with a leading zero followed by another
- * digit as numeric - could be a zip code or similar
- */
- if (tmp.len > 0 &&
- !(tmp.data[0] == '0' &&
- isdigit((unsigned char) tmp.data[1])) &&
- strspn(tmp.data, "+-0123456789Ee.") == tmp.len)
- {
- /*
- * might be a number. See if we can input it as a numeric
- * value. Ignore any actual parsed value.
- */
- char *endptr = "junk";
- long lval;
-
- lval = strtol(tmp.data, &endptr, 10);
- (void) lval;
- if (*endptr == '\0')
- {
- /*
- * strol man page says this means the whole string is
- * valid
- */
- is_number = true;
- }
- else
- {
- /* not an int - try a double */
- double dval;
-
- dval = strtod(tmp.data, &endptr);
- (void) dval;
- if (*endptr == '\0')
- is_number = true;
- }
- }
- if (is_number)
+ if (IsValidJsonNumber(tmp.data, tmp.len))
appendBinaryStringInfo(&dst, tmp.data, tmp.len);
else
escape_json(&dst, tmp.data);
diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c
index d2bf640..271a2a4 100644
--- a/src/backend/utils/adt/json.c
+++ b/src/backend/utils/adt/json.c
@@ -173,6 +173,31 @@ lex_expect(JsonParseContext ctx, JsonLexContext *lex, JsonTokenType token)
(c) == '_' || \
IS_HIGHBIT_SET(c))
+/* utility function to check if a string is a valid JSON number */
+extern bool
+IsValidJsonNumber(char * str, int len)
+{
+ bool numeric_error;
+ JsonLexContext dummy_lex;
+
+
+ /* json_lex_number expects a leading - to have been eaten already */
+ if (*str == '-')
+ {
+ dummy_lex.input = str + 1;
+ dummy_lex.input_length = len - 1;
+ }
+ else
+ {
+ dummy_lex.input = str;
+ dummy_lex.input_length = len;
+ }
+
+ json_lex_number(&dummy_lex, dummy_lex.input, &numeric_error);
+
+ return ! numeric_error;
+}
+
/*
* Input.
*/
@@ -1338,8 +1363,6 @@ datum_to_json(Datum val, bool is_null, StringInfo result,
{
char *outputstr;
text *jsontext;
- bool numeric_error;
- JsonLexContext dummy_lex;
/* callers are expected to ensure that null keys are not passed in */
Assert( ! (key_scalar && is_null));
@@ -1376,25 +1399,14 @@ datum_to_json(Datum val, bool is_null, StringInfo result,
break;
case JSONTYPE_NUMERIC:
outputstr = OidOutputFunctionCall(outfuncoid, val);
- if (key_scalar)
- {
- /* always quote keys */
- escape_json(result, outputstr);
- }
+ /*
+ * Don't call escape_json for a non-key if it's a valid JSON
+ * number.
+ */
+ if (!key_scalar && IsValidJsonNumber(outputstr, strlen(outputstr)))
+ appendStringInfoString(result, outputstr);
else
- {
- /*
- * Don't call escape_json for a non-key if it's a valid JSON
- * number.
- */
- dummy_lex.input = *outputstr == '-' ? outputstr + 1 : outputstr;
- dummy_lex.input_length = strlen(dummy_lex.input);
- json_lex_number(&dummy_lex, dummy_lex.input, &numeric_error);
- if (!numeric_error)
- appendStringInfoString(result, outputstr);
- else
- escape_json(result, outputstr);
- }
+ escape_json(result, outputstr);
pfree(outputstr);
break;
case JSONTYPE_DATE:
diff --git a/src/include/utils/jsonapi.h b/src/include/utils/jsonapi.h
index df05712..b3a579b 100644
--- a/src/include/utils/jsonapi.h
+++ b/src/include/utils/jsonapi.h
@@ -117,4 +117,7 @@ extern JsonLexContext *makeJsonLexContextCstringLen(char *json,
int len,
bool need_escapes);
+/* utility function to check if a string is a valid JSON number */
+extern bool IsValidJsonNumber(char * str, int len);
+
#endif /* JSONAPI_H */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
