Re: [HACKERS] [PATCH] Generalized JSON output functions
On July 13 I wrote: Yes, but I think the plugin is the right place to do it. What is more, this won't actually prevent you completely from producing non-ECMAScript compliant JSON, since json or jsonb values containing offending numerics won't be caught, AIUI. But a fairly simple to write function that reparsed and fixed the JSON inside the decoder would work. The OP admitted that this was a serious flaw in his approach. In fact, given that a json value can contain an offending numeric value, any approach which doesn't involve reparsing is pretty much bound to fail. I agree that was a critical omission in my thinking. Now, back to the whitespace issue: I could submit a patch to unify the whitespace w/o all the hairy callbacks. Did we have the consensus here: no spaces whatsoever unless some *_pretty function is used? -- Alex
Re: [HACKERS] [PATCH] Generalized JSON output functions
On 07/17/2015 10:11 AM, Andrew Dunstan wrote: On 07/17/2015 08:20 AM, Shulgin, Oleksandr wrote: This patch makes Postgres core more complex Yes, it does. But, that was not the purpose, obviously. :-) while not really solving the problem in Javascript. It still allows for less risk of silent data corruption on the js side. I have already pointed out how this patch is fundamentally broken. You can achieve your aims by a fairly small amount of code inside your logical decoder, and with no core code changes whatsoever. So I'm puzzled why we are even still debating this broken design. Incidentally, this doesn't look acceptable anyway: ! es-json_cxt.value(es-json_cxt, num, JSONTYPE_NUMERIC, ! NUMERICOID, 1702 /* numeric_out */); We don't hardcode function oids elsewhere. So this is also something that makes the patch unacceptable. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Jul 17, 2015 4:31 PM, Andrew Dunstan and...@dunslane.net wrote: On 07/17/2015 10:11 AM, Andrew Dunstan wrote: On 07/17/2015 08:20 AM, Shulgin, Oleksandr wrote: This patch makes Postgres core more complex Yes, it does. But, that was not the purpose, obviously. :-) while not really solving the problem in Javascript. It still allows for less risk of silent data corruption on the js side. I have already pointed out how this patch is fundamentally broken. You can achieve your aims by a fairly small amount of code inside your logical decoder, and with no core code changes whatsoever. So I'm puzzled why we are even still debating this broken design. Incidentally, this doesn't look acceptable anyway: ! es-json_cxt.value(es-json_cxt, num, JSONTYPE_NUMERIC, ! NUMERICOID, 1702 /* numeric_out */); We don't hardcode function oids elsewhere. So this is also something that makes the patch unacceptable. Well, good to know (I believe I've asked about this in the first mail specifically). Is there any way a built-in function oid would change/differ on different server versions? What would be the recommended way to do this? -- Alex
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Jul 17, 2015 12:23 AM, Ryan Pedela rped...@datalanche.com wrote: On Thu, Jul 16, 2015 at 11:51 AM, Robert Haas robertmh...@gmail.com wrote: I don't understand these issues in great technical depth, but if somebody is arguing that it's OK for PostgreSQL to be difficult to use for a certain category of user for several years until the next language rev becomes mainstream, then I disagree. The fact that somebody wrote a patch to try to solve a problem means that the thing in question is a problem for at least that one user. If he's the only one, maybe we don't need to care all that much. If his needs are representative of a significant user community, we should not turn our backs on that community, regardless of whether we like the patch he wrote, and regardless of how well we are meeting the needs of other communities (like node.js users). I completely agree. However we aren't talking about a usability problem with Postgres. We are actually talking about a usability problem with Javascript, and trying to implement a band-aid for it with Postgres. Javascript doesn't support large numbers, it just doesn't. There is nothing the Postgres community can do about that. Only the ECMAscript standards committee and implementers can fix Javascript. Here is the current user flow of reading numerics from Postgres and then doing some math with them in Javascript. 1. SELECT json 2. Use json-bignum [1] module or custom JSON parser to correctly parse numerics. 3. Perform addition, subtraction, etc of numerics using either custom numeric math library or an existing library such as bigdecimal.js [2]. Here is the user flow if this patch is accepted. 1. SELECT json with quoting flags set 2. Custom parser to find numeric strings within JSON and convert them into numerics. This is easy if JSON is simple, but may be difficult with a very complex JSON. 3. Perform addition, subtraction, etc of numerics using either custom numeric math library or an existing library such as bigdecimal.js [2]. It is almost the exact same user flow so what is the point? In my case there's no select: we're running this in the context of a logical decoding plugin. The all safeguarding idea that is enabled by this patch is that if the client *expects* big numbers *and* it needs to perform arithmetic on them, it'll have the special handling anyway. And IMO, it would actually make more sense to use big numbers module only at the point where you have the need for special handling, not to parse the whole input in a nonstandard way. But the clients that are unaware of big numbers or don't care about them shouldn't be *forced* to use external modules for parsing json. This patch makes Postgres core more complex Yes, it does. But, that was not the purpose, obviously. :-) while not really solving the problem in Javascript. It still allows for less risk of silent data corruption on the js side. -- Alex
Re: [HACKERS] [PATCH] Generalized JSON output functions
On 07/17/2015 08:20 AM, Shulgin, Oleksandr wrote: This patch makes Postgres core more complex Yes, it does. But, that was not the purpose, obviously. :-) while not really solving the problem in Javascript. It still allows for less risk of silent data corruption on the js side. I have already pointed out how this patch is fundamentally broken. You can achieve your aims by a fairly small amount of code inside your logical decoder, and with no core code changes whatsoever. So I'm puzzled why we are even still debating this broken design. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
Andrew Dunstan wrote: I have already pointed out how this patch is fundamentally broken. You can achieve your aims by a fairly small amount of code inside your logical decoder, and with no core code changes whatsoever. So I'm puzzled why we are even still debating this broken design. I went through all your responses over the entire thread and I couldn't find your argument about how this is fundamentally broken. Can you restate, or maybe give an archive link if I just missed it? (Saying but it changes so much of the existing code is not really a fundamental problem to me. I mean, it's not like the existing code is perfect and needs no changes.) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
Shulgin, Oleksandr wrote: On Jul 17, 2015 4:31 PM, Andrew Dunstan and...@dunslane.net wrote: Incidentally, this doesn't look acceptable anyway: ! es-json_cxt.value(es-json_cxt, num, JSONTYPE_NUMERIC, ! NUMERICOID, 1702 /* numeric_out */); We don't hardcode function oids elsewhere. So this is also something that makes the patch unacceptable. Well, good to know (I believe I've asked about this in the first mail specifically). Is there any way a built-in function oid would change/differ on different server versions? What would be the recommended way to do this? C'mon, that's a trivial problem. Just use getTypeOutputInfo(); numeric's OID is hardcoded as NUMERICOID. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
On 07/17/2015 10:30 AM, Alvaro Herrera wrote: Andrew Dunstan wrote: I have already pointed out how this patch is fundamentally broken. You can achieve your aims by a fairly small amount of code inside your logical decoder, and with no core code changes whatsoever. So I'm puzzled why we are even still debating this broken design. I went through all your responses over the entire thread and I couldn't find your argument about how this is fundamentally broken. Can you restate, or maybe give an archive link if I just missed it? (Saying but it changes so much of the existing code is not really a fundamental problem to me. I mean, it's not like the existing code is perfect and needs no changes.) On July 13 I wrote: Yes, but I think the plugin is the right place to do it. What is more, this won't actually prevent you completely from producing non-ECMAScript compliant JSON, since json or jsonb values containing offending numerics won't be caught, AIUI. But a fairly simple to write function that reparsed and fixed the JSON inside the decoder would work. The OP admitted that this was a serious flaw in his approach. In fact, given that a json value can contain an offending numeric value, any approach which doesn't involve reparsing is pretty much bound to fail. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Wed, Jul 15, 2015 at 12:58 PM, Andrew Dunstan and...@dunslane.net wrote: The approach take was both invasive and broken. Well, then let's not do it that way. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Wed, Jul 15, 2015 at 1:10 PM, Ryan Pedela rped...@datalanche.com wrote: Like I said previously, the situation with Javascript will hopefully be remedied in a few years with ES7 anyway. I don't understand these issues in great technical depth, but if somebody is arguing that it's OK for PostgreSQL to be difficult to use for a certain category of user for several years until the next language rev becomes mainstream, then I disagree. The fact that somebody wrote a patch to try to solve a problem means that the thing in question is a problem for at least that one user. If he's the only one, maybe we don't need to care all that much. If his needs are representative of a significant user community, we should not turn our backs on that community, regardless of whether we like the patch he wrote, and regardless of how well we are meeting the needs of other communities (like node.js users). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
2015-07-16 19:51 GMT+02:00 Robert Haas robertmh...@gmail.com: On Wed, Jul 15, 2015 at 1:10 PM, Ryan Pedela rped...@datalanche.com wrote: Like I said previously, the situation with Javascript will hopefully be remedied in a few years with ES7 anyway. I don't understand these issues in great technical depth, but if somebody is arguing that it's OK for PostgreSQL to be difficult to use for a certain category of user for several years until the next language rev becomes mainstream, then I disagree. The fact that somebody wrote a patch to try to solve a problem means that the thing in question is a problem for at least that one user. If he's the only one, maybe we don't need to care all that much. If his needs are representative of a significant user community, we should not turn our backs on that community, regardless of whether we like the patch he wrote, and regardless of how well we are meeting the needs of other communities (like node.js users). I don't think so this issue is too hot. How long we support XML? The output format is static - the date format is fixed. How much issues was there? Was there any issue, that was not solvable by casting? If somebody needs different quoting, then it can be solved by explicit cast in SQL query, and not in hacking our output routines. Regards Pavel -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [HACKERS] [PATCH] Generalized JSON output functions
On 07/15/2015 10:52 AM, Robert Haas wrote: On Mon, Jul 13, 2015 at 10:46 AM, Ryan Pedela rped...@datalanche.com wrote: As far as large numbers in JSON, I think Postgres is doing the right thing and should not be changed. It is Javascript that is stupid here, and I don't think it is wise to add something to core just because one client does stupid things with large numbers. In addition, ES7 is introducing value types which will hopefully solve the large number problem in Javascript. FWIW, I don't agree. If it's not easy to read the JSON that PostgreSQL generates using JavaScript, then a lot of people are just going to give up on doing it, and IMO that would be sad. Telling people that they have to parse the JSON using some parser other than the one built into their JavaScript engine, whack it around, and then render it as text and parse it again is not really an acceptable answer. The reason why the logical decoding stuff allows multiple output formats is because Andres, quite correctly, foresaw that different people would need different output formats. He could have designed that system to output only one output format and just said, everybody's got to read and parse this, but that would have been slow. Instead, he tried to set things up so that you could get the output in the format that was most convenient for your client, whatever that is. On this thread, we're back-pedaling from that idea: sorry, you can get JSON output, but if you want JSON output that will be properly interpreted by your JSON parser, you can't have that. Regardless of the details of this particular patch, I can't endorse that approach. If we want people to use our software, we need to meet them where they are at, especially when we are only (IIUC) talking about inserting a few extra quotation marks. The question for me is where is the best place to transform the data. The approach take was both invasive and broken. The approach I suggested, reparsing and transforming it in the logical decoder, would be both fairly simple and completely noninvasive. If someone gives me a test for what is an acceptable number for JS processors, I bet I could write a transformation function in an hour or two, and in a hundred lines or so. I admit that I probably have more experience doing this than anyone else, but the parser API was designed to be fairly simple, and I believe it is. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Wed, Jul 15, 2015 at 11:10 AM, Ryan Pedela rped...@datalanche.com
wrote:
On Wed, Jul 15, 2015 at 8:52 AM, Robert Haas robertmh...@gmail.com
wrote:
FWIW, I don't agree. If it's not easy to read the JSON that
PostgreSQL generates using JavaScript, then a lot of people are just
going to give up on doing it, and IMO that would be sad. Telling
people that they have to parse the JSON using some parser other than
the one built into their JavaScript engine, whack it around, and then
render it as text and parse it again is not really an acceptable
answer.
The vast majority of Javascript users are going to be using Node.js when
they connect to Postgres if only for security reasons. If they use Node,
they will be using node-postgres [1] or something that builds on top of it.
For int64 and numerics in a row, the default is to return a string, and
there is a flag you can set to round returned numbers if you prefer. There
is also a way to override the default parsing of each Postgres type [2]. So
in the case of JSON using my json-bignum module [3], the code looks like
this:
var pgTypes = require('pg').types;
var bignumJSON = require('json-bignum');
types.setTypeParser(JSON_TYPE_OID, function (value) {
return bignumJSON.parse(value);
});
types.setTypeParser(JSONB_TYPE_OID, function (value) {
return bignumJSON.parse(value);
});
To me that code is super simple, and no a pain in the ass. In other words,
it is not Telling people that they have to parse the JSON using some
parser other than the one built into their JavaScript engine, whack it
around, and then render it as text and parse it again. Like I said
previously, the situation with Javascript will hopefully be remedied in a
few years with ES7 anyway.
1. https://github.com/brianc/node-postgres
2. https://github.com/brianc/node-pg-types
3. https://github.com/datalanche/json-bignum
On Wed, Jul 15, 2015 at 8:52 AM, Robert Haas robertmh...@gmail.com
wrote:
The reason why the logical decoding stuff allows multiple
output formats is because Andres, quite correctly, foresaw that
different people would need different output formats. He could have
designed that system to output only one output format and just said,
everybody's got to read and parse this, but that would have been slow.
Instead, he tried to set things up so that you could get the output in
the format that was most convenient for your client, whatever that is.
On this thread, we're back-pedaling from that idea: sorry, you can get
JSON output, but if you want JSON output that will be properly
interpreted by your JSON parser, you can't have that. Regardless of
the details of this particular patch, I can't endorse that approach.
If we want people to use our software, we need to meet them where they
are at, especially when we are only (IIUC) talking about inserting a
few extra quotation marks.
I would be okay with a generic way to specify output formats if there are
many use cases beyond Javascript and JSON. I vaguely remember someone
suggesting a FORMAT clause on CREATE TABLE which would specify how a
particular column would output from a SELECT. For example, returning a date
with a non-ISO format. I liked that idea. However if the only reason for
different output formats is Javascript, that is silly. I have a very long
list of feature requests that would probably only be beneficial to me or a
handful of users. Should we implement them? No, of course not! If we did
that Postgres would cease to be the best open-source database. You can't
have the best product and say yes to everything. Feature creep is the enemy
of quality. If Javascript is the sole reason for supporting multiple output
formats, then that is the definition of feature creep in my opinion. If
there are many use cases beyond Javascript and JSON, then that is different
and a conversation worth having.
Bottom line: Large numbers are a pain to deal with in Javascript regardless
of where they come from or what format they are in. Adding code to Postgres
core will never change that.
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Wed, Jul 15, 2015 at 8:52 AM, Robert Haas robertmh...@gmail.com wrote:
FWIW, I don't agree. If it's not easy to read the JSON that
PostgreSQL generates using JavaScript, then a lot of people are just
going to give up on doing it, and IMO that would be sad. Telling
people that they have to parse the JSON using some parser other than
the one built into their JavaScript engine, whack it around, and then
render it as text and parse it again is not really an acceptable
answer.
The vast majority of Javascript users are going to be using Node.js when
they connect to Postgres if only for security reasons. If they use Node,
they will be using node-postgres [1] or something that builds on top of it.
For int64 and numerics in a row, the default is to return a string, and
there is a flag you can set to round returned numbers if you prefer. There
is also a way to override the default parsing of each Postgres type [2]. So
in the case of JSON using my json-bignum module [3], the code looks like
this:
var pgTypes = require('pg').types;
var bignumJSON = require('json-bignum');
types.setTypeParser(JSON_TYPE_OID, function (value) {
return bignumJSON.parse(value);
});
types.setTypeParser(JSONB_TYPE_OID, function (value) {
return bignumJSON.parse(value);
});
To me that code is super simple, and no a pain in the ass. In other words,
it is not Telling people that they have to parse the JSON using some
parser other than the one built into their JavaScript engine, whack it
around, and then render it as text and parse it again. Like I said
previously, the situation with Javascript will hopefully be remedied in a
few years with ES7 anyway.
1. https://github.com/brianc/node-postgres
2. https://github.com/brianc/node-pg-types
3. https://github.com/datalanche/json-bignum
On Wed, Jul 15, 2015 at 8:52 AM, Robert Haas robertmh...@gmail.com wrote:
The reason why the logical decoding stuff allows multiple
output formats is because Andres, quite correctly, foresaw that
different people would need different output formats. He could have
designed that system to output only one output format and just said,
everybody's got to read and parse this, but that would have been slow.
Instead, he tried to set things up so that you could get the output in
the format that was most convenient for your client, whatever that is.
On this thread, we're back-pedaling from that idea: sorry, you can get
JSON output, but if you want JSON output that will be properly
interpreted by your JSON parser, you can't have that. Regardless of
the details of this particular patch, I can't endorse that approach.
If we want people to use our software, we need to meet them where they
are at, especially when we are only (IIUC) talking about inserting a
few extra quotation marks.
I would be okay with a generic way to specify output formats if there are
many use cases beyond Javascript and JSON. I vaguely remember someone
suggesting a FORMAT clause on CREATE TABLE which would specify how a
particular column would output from a SELECT. For example, returning a date
with a non-ISO format. I liked that idea. However if the only reason for
different output formats is Javascript, that is silly. I have a very long
list of feature requests that would probably only be beneficial to me or a
handful of users. Should we implement them? No, of course not! If we did
that Postgres would cease to be the best open-source database. You can't
have the best product and say yes to everything. Feature creep is the enemy
of quality. If Javascript is the sole reason for supporting multiple output
formats, then that is the definition of feature creep in my opinion. If
there are many use cases beyond Javascript and JSON, then that is different
and a conversation worth having.
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Mon, Jul 13, 2015 at 10:46 AM, Ryan Pedela rped...@datalanche.com wrote: As far as large numbers in JSON, I think Postgres is doing the right thing and should not be changed. It is Javascript that is stupid here, and I don't think it is wise to add something to core just because one client does stupid things with large numbers. In addition, ES7 is introducing value types which will hopefully solve the large number problem in Javascript. FWIW, I don't agree. If it's not easy to read the JSON that PostgreSQL generates using JavaScript, then a lot of people are just going to give up on doing it, and IMO that would be sad. Telling people that they have to parse the JSON using some parser other than the one built into their JavaScript engine, whack it around, and then render it as text and parse it again is not really an acceptable answer. The reason why the logical decoding stuff allows multiple output formats is because Andres, quite correctly, foresaw that different people would need different output formats. He could have designed that system to output only one output format and just said, everybody's got to read and parse this, but that would have been slow. Instead, he tried to set things up so that you could get the output in the format that was most convenient for your client, whatever that is. On this thread, we're back-pedaling from that idea: sorry, you can get JSON output, but if you want JSON output that will be properly interpreted by your JSON parser, you can't have that. Regardless of the details of this particular patch, I can't endorse that approach. If we want people to use our software, we need to meet them where they are at, especially when we are only (IIUC) talking about inserting a few extra quotation marks. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
Yes, but I think the plugin is the right place to do it. What is more, this won't actually prevent you completely from producing non-ECMAScript compliant JSON, since json or jsonb values containing offending numerics won't be caught, AIUI. Ah, that's a good catch indeed. But a fairly simple to write function that reparsed and fixed the JSON inside the decoder would work. Need to rethink this, but reparsing was never my favorite option here. :-) -- Alex
Re: [HACKERS] [PATCH] Generalized JSON output functions
On 07/13/2015 05:41 AM, Shulgin, Oleksandr wrote: On Mon, Jul 13, 2015 at 9:44 AM, Pavel Stehule pavel.steh...@gmail.com mailto:pavel.steh...@gmail.com wrote: To reiterate: for my problem, that is escaping numerics that can potentially overflow[1] under ECMAScript standard, I want to be able to override the code that outputs the numeric converted to string. There is no way in current implementation to do that *at all*, short of copying all the code involved in producing JSON output and changing it at certain points. One could try re-parsing JSON instead, but that doesn't actually solve the issue, because type information is lost forever at that point. The whitespace unification was a mere side-effect of the original effort on this patch. The dynamic type change is some what I would not to do in database, really :) If you afraid about overflow, then convert numeric to string immediately - in this case, the client have to support both variant - so immediate cast should not be a problem. Yeah, but how would you do that in context of a logical replication decoding plugin? I've tried a number of tricks for that, including, but not limited to registering phony types to wrap numeric type and replacing the OID of numeric with this custom type OID in TupleDesc, but then again one has to register that as known record type, etc. Anyway this check on max number should be implemented in our JSON(b) out functions (as warning?). Not really, since this is a problem of ECMAScript standard, not JSON spec. For example, Python module for handling JSON doesn't suffer from this overflow problem, The thing is, we cannot know which clients are going to consume the stream of decoded events, and if it's some implementation of JavaScript, it can suffer silent data corruption if we don't guard against that in the logical decoding plugin. Hope that makes it clear. :-) Yes, but I think the plugin is the right place to do it. What is more, this won't actually prevent you completely from producing non-ECMAScript compliant JSON, since json or jsonb values containing offending numerics won't be caught, AIUI. But a fairly simple to write function that reparsed and fixed the JSON inside the decoder would work. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
On 07/13/2015 10:46 AM, Ryan Pedela wrote: On Mon, Jul 13, 2015 at 1:30 AM, Shulgin, Oleksandr oleksandr.shul...@zalando.de mailto:oleksandr.shul...@zalando.de wrote: To reiterate: for my problem, that is escaping numerics that can potentially overflow[1] under ECMAScript standard, I want to be able to override the code that outputs the numeric converted to string. There is no way in current implementation to do that *at all*, short of copying all the code involved in producing JSON output and changing it at certain points. One could try re-parsing JSON instead, but that doesn't actually solve the issue, because type information is lost forever at that point. I had the exact same problem with Node.js and client-side Javascript. That is why I wrote json-bignum [1] for Node.js. There is a bower version [2] as well. The only caveat is that it is slower than the native JSON functions, but I am happy to receive PRs to improve performance. 1. https://github.com/datalanche/json-bignum 2. https://libraries.io/bower/json-bignum As far as large numbers in JSON, I think Postgres is doing the right thing and should not be changed. It is Javascript that is stupid here, and I don't think it is wise to add something to core just because one client does stupid things with large numbers. In addition, ES7 is introducing value types which will hopefully solve the large number problem in Javascript. The random whitespace issue is valid in my opinion and should be fixed. OK, I think we're getting a consensus here. It's good to know the JS world is acquiring some sanity in this area. Let's just fix the whitespace and be done, without all the callback stuff. That should be a much smaller patch. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Mon, Jul 13, 2015 at 1:30 AM, Shulgin, Oleksandr oleksandr.shul...@zalando.de wrote: To reiterate: for my problem, that is escaping numerics that can potentially overflow[1] under ECMAScript standard, I want to be able to override the code that outputs the numeric converted to string. There is no way in current implementation to do that *at all*, short of copying all the code involved in producing JSON output and changing it at certain points. One could try re-parsing JSON instead, but that doesn't actually solve the issue, because type information is lost forever at that point. I had the exact same problem with Node.js and client-side Javascript. That is why I wrote json-bignum [1] for Node.js. There is a bower version [2] as well. The only caveat is that it is slower than the native JSON functions, but I am happy to receive PRs to improve performance. 1. https://github.com/datalanche/json-bignum 2. https://libraries.io/bower/json-bignum As far as large numbers in JSON, I think Postgres is doing the right thing and should not be changed. It is Javascript that is stupid here, and I don't think it is wise to add something to core just because one client does stupid things with large numbers. In addition, ES7 is introducing value types which will hopefully solve the large number problem in Javascript. The random whitespace issue is valid in my opinion and should be fixed. Thanks, Ryan Pedela
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Sun, Jul 12, 2015 at 8:39 PM, Pavel Stehule pavel.steh...@gmail.com wrote: The thing is - it's not only about whitespace, otherwise I would probably not bother with the generic interface. For my original problem, there is simply no way to do this correctly in an extension w/o copying over all of the logic from json.c, which I have to do right now, would rather not. I am sorry - we are talking about JSON, not about any styled document. I disagree, so it has not be implemented as extension - the backport of JSON support is a extension. Hm... I'm having a hard time making sense of that statement, sorry. To reiterate: for my problem, that is escaping numerics that can potentially overflow[1] under ECMAScript standard, I want to be able to override the code that outputs the numeric converted to string. There is no way in current implementation to do that *at all*, short of copying all the code involved in producing JSON output and changing it at certain points. One could try re-parsing JSON instead, but that doesn't actually solve the issue, because type information is lost forever at that point. The whitespace unification was a mere side-effect of the original effort on this patch. -- Best regards, Alex [1] http://stackoverflow.com/questions/307179/what-is-javascripts-highest-integer-value-that-a-number-can-go-to-without-losin
Re: [HACKERS] [PATCH] Generalized JSON output functions
2015-07-13 9:30 GMT+02:00 Shulgin, Oleksandr oleksandr.shul...@zalando.de: On Sun, Jul 12, 2015 at 8:39 PM, Pavel Stehule pavel.steh...@gmail.com wrote: The thing is - it's not only about whitespace, otherwise I would probably not bother with the generic interface. For my original problem, there is simply no way to do this correctly in an extension w/o copying over all of the logic from json.c, which I have to do right now, would rather not. I am sorry - we are talking about JSON, not about any styled document. I disagree, so it has not be implemented as extension - the backport of JSON support is a extension. Hm... I'm having a hard time making sense of that statement, sorry. To reiterate: for my problem, that is escaping numerics that can potentially overflow[1] under ECMAScript standard, I want to be able to override the code that outputs the numeric converted to string. There is no way in current implementation to do that *at all*, short of copying all the code involved in producing JSON output and changing it at certain points. One could try re-parsing JSON instead, but that doesn't actually solve the issue, because type information is lost forever at that point. The whitespace unification was a mere side-effect of the original effort on this patch. The dynamic type change is some what I would not to do in database, really :) If you afraid about overflow, then convert numeric to string immediately - in this case, the client have to support both variant - so immediate cast should not be a problem. Anyway this check on max number should be implemented in our JSON(b) out functions (as warning?). Regards Pavel -- Best regards, Alex [1] http://stackoverflow.com/questions/307179/what-is-javascripts-highest-integer-value-that-a-number-can-go-to-without-losin
Re: [HACKERS] [PATCH] Generalized JSON output functions
2015-07-12 10:29 GMT+02:00 Shulgin, Oleksandr oleksandr.shul...@zalando.de : On Jul 11, 2015 8:41 PM, Pavel Stehule pavel.steh...@gmail.com wrote: There is simple rule - be strict on output and tolerant on input. If I understand to sense of this patch - the target is one same format of JSON documents - so there are no space for any variability. So, would you prefer explain json format on a single line - no indentation or whitespace whatsoever? yes, - if you need pretty format - there is function json_pretty - any more styles is wrong on Postgres side. This far it was only about whitespace, but it can be useful for tweaking other aspects of output, as I've mentioned before. Postgres is database, not presentation server - it have to to any database operations, quickly as possible - and formatting is part of client side. I can imagine the ability for 3rd-party code to override certain aspects of the output would be really useful for extensions or background workers, decoding plugins, etc. we talking about output - I can imagine, so there is only two possibilities - plain join, and pretty formatted join (but with only one style). I am thinking so general json functions has sense, but I partially disagree with your implementation. Then what would you differently exactly? simple code. -- Alex
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Sun, Jul 12, 2015 at 4:35 AM, Pavel Stehule pavel.steh...@gmail.com wrote: 2015-07-12 10:29 GMT+02:00 Shulgin, Oleksandr oleksandr.shul...@zalando.de: On Jul 11, 2015 8:41 PM, Pavel Stehule pavel.steh...@gmail.com wrote: There is simple rule - be strict on output and tolerant on input. If I understand to sense of this patch - the target is one same format of JSON documents - so there are no space for any variability. So, would you prefer explain json format on a single line - no indentation or whitespace whatsoever? yes, - if you need pretty format - there is function json_pretty - any more styles is wrong on Postgres side. This far it was only about whitespace, but it can be useful for tweaking other aspects of output, as I've mentioned before. Postgres is database, not presentation server - it have to to any database operations, quickly as possible - and formatting is part of client side. I can imagine the ability for 3rd-party code to override certain aspects of the output would be really useful for extensions or background workers, decoding plugins, etc. we talking about output - I can imagine, so there is only two possibilities - plain join, and pretty formatted join (but with only one style). This makes sense. Postgres core really only needs to support the minimum styles necessary for core requirements. This means raw unformatted json for data productions to client and an appropriate formatting for explain. Fancier stuff like a generic formatted is fine but those features *belong in an extension*. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
2015-07-12 20:11 GMT+02:00 Shulgin, Oleksandr oleksandr.shul...@zalando.de : we talking about output - I can imagine, so there is only two possibilities - plain join, and pretty formatted join (but with only one style). This makes sense. Postgres core really only needs to support the minimum styles necessary for core requirements. This means raw unformatted json for data productions to client and an appropriate formatting for explain. Fancier stuff like a generic formatted is fine but those features *belong in an extension*. The thing is - it's not only about whitespace, otherwise I would probably not bother with the generic interface. For my original problem, there is simply no way to do this correctly in an extension w/o copying over all of the logic from json.c, which I have to do right now, would rather not. I am sorry - we are talking about JSON, not about any styled document. I disagree, so it has not be implemented as extension - the backport of JSON support is a extension. Regards Pavel Alex
Re: [HACKERS] [PATCH] Generalized JSON output functions
we talking about output - I can imagine, so there is only two possibilities - plain join, and pretty formatted join (but with only one style). This makes sense. Postgres core really only needs to support the minimum styles necessary for core requirements. This means raw unformatted json for data productions to client and an appropriate formatting for explain. Fancier stuff like a generic formatted is fine but those features *belong in an extension*. The thing is - it's not only about whitespace, otherwise I would probably not bother with the generic interface. For my original problem, there is simply no way to do this correctly in an extension w/o copying over all of the logic from json.c, which I have to do right now, would rather not. Alex
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Fri, Jul 10, 2015 at 5:16 PM, Pavel Stehule pavel.steh...@gmail.com
wrote:
Well, one could call it premature pessimization due to dynamic call
overhead.
IMO, the fact that json_out_init_context() sets the value callback to
json_out_value is an implementation detail, the other parts of code should
not rely on. And for the Explain output, there definitely going to be
*some* code between context initialization and output callbacks: these are
done in a number of different functions.
Again - it is necessary? Postgres still use modular code, not OOP code. I
can understand the using of this technique, when I need a possibility to
change behave. But these function are used for printing JSON, not printing
any others.
No, it's not strictly necessary.
For me it's not about procedural- vs. object- style, but rather about being
able to override/extend the behavior consistently. And for that I would
prefer that if I override the value callback in a JSON output context, that
it would be called for every value being printed, not only for some of them.
Thank you for pointing out the case of Explain format, I've totally
overlooked it in my first version. Trying to apply the proposed approach
in the explain printing code led me to reorganize things slightly. I've
added explicit functions for printing keys vs. values, thus no need to
expose that key_scalar param anymore. There are now separate before/after
key and before/after value functions as well, but I believe it makes for a
cleaner code.
The most of the complexity is still in the code that decides whether or not
to put spaces (between the values or for indentation) and newlines at
certain points. Should we decide to unify the style we emit ourselves,
this could be simplified, while still leaving room for great flexibility if
overridden by an extension, for example.
Have a nice weekend.
--
Alex
diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
new file mode 100644
index 7d89867..1f365f5
*** a/contrib/hstore/hstore_io.c
--- b/contrib/hstore/hstore_io.c
*** hstore_to_json_loose(PG_FUNCTION_ARGS)
*** 1241,1286
int count = HS_COUNT(in);
char *base = STRPTR(in);
HEntry *entries = ARRPTR(in);
! StringInfoData tmp,
! dst;
if (count == 0)
PG_RETURN_TEXT_P(cstring_to_text_with_len({}, 2));
initStringInfo(tmp);
! initStringInfo(dst);
!
! appendStringInfoChar(dst, '{');
for (i = 0; i count; i++)
{
resetStringInfo(tmp);
appendBinaryStringInfo(tmp, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
! escape_json(dst, tmp.data);
! appendStringInfoString(dst, : );
if (HS_VALISNULL(entries, i))
! appendStringInfoString(dst, null);
/* guess that values of 't' or 'f' are booleans */
else if (HS_VALLEN(entries, i) == 1 *(HS_VAL(entries, base, i)) == 't')
! appendStringInfoString(dst, true);
else if (HS_VALLEN(entries, i) == 1 *(HS_VAL(entries, base, i)) == 'f')
! appendStringInfoString(dst, false);
else
{
resetStringInfo(tmp);
appendBinaryStringInfo(tmp, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
if (IsValidJsonNumber(tmp.data, tmp.len))
! appendBinaryStringInfo(dst, tmp.data, tmp.len);
else
! escape_json(dst, tmp.data);
}
-
- if (i + 1 != count)
- appendStringInfoString(dst, , );
}
- appendStringInfoChar(dst, '}');
! PG_RETURN_TEXT_P(cstring_to_text(dst.data));
}
PG_FUNCTION_INFO_V1(hstore_to_json);
--- 1241,1293
int count = HS_COUNT(in);
char *base = STRPTR(in);
HEntry *entries = ARRPTR(in);
! StringInfoData tmp;
! Datum num;
! JsonOutContext dst;
if (count == 0)
PG_RETURN_TEXT_P(cstring_to_text_with_len({}, 2));
initStringInfo(tmp);
! json_out_init_context(dst, JSON_OUT_USE_SPACES);
! dst.object_start(dst);
for (i = 0; i count; i++)
{
resetStringInfo(tmp);
appendBinaryStringInfo(tmp, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
! json_out_cstring_key(dst, tmp.data);
!
if (HS_VALISNULL(entries, i))
! dst.value(dst, (Datum) 0, JSONTYPE_NULL, InvalidOid, InvalidOid);
!
/* guess that values of 't' or 'f' are booleans */
else if (HS_VALLEN(entries, i) == 1 *(HS_VAL(entries, base, i)) == 't')
! dst.value(dst, BoolGetDatum(true), JSONTYPE_BOOL,
! InvalidOid, InvalidOid);
!
else if (HS_VALLEN(entries, i) == 1 *(HS_VAL(entries, base, i)) == 'f')
! dst.value(dst, BoolGetDatum(false), JSONTYPE_BOOL,
! InvalidOid, InvalidOid);
else
{
resetStringInfo(tmp);
appendBinaryStringInfo(tmp, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
+
if (IsValidJsonNumber(tmp.data, tmp.len))
! {
! num = DirectFunctionCall3(numeric_in, CStringGetDatum(tmp.data), 0, -1);
! dst.value(dst, num, JSONTYPE_NUMERIC,
! NUMERICOID, 1702 /* numeric_out */);
! pfree(DatumGetPointer(num));
! }
else
! json_out_cstring_value(dst,
Re: [HACKERS] [PATCH] Generalized JSON output functions
2015-07-11 18:02 GMT+02:00 Shulgin, Oleksandr oleksandr.shul...@zalando.de : On Fri, Jul 10, 2015 at 5:16 PM, Pavel Stehule pavel.steh...@gmail.com wrote: Well, one could call it premature pessimization due to dynamic call overhead. IMO, the fact that json_out_init_context() sets the value callback to json_out_value is an implementation detail, the other parts of code should not rely on. And for the Explain output, there definitely going to be *some* code between context initialization and output callbacks: these are done in a number of different functions. Again - it is necessary? Postgres still use modular code, not OOP code. I can understand the using of this technique, when I need a possibility to change behave. But these function are used for printing JSON, not printing any others. No, it's not strictly necessary. For me it's not about procedural- vs. object- style, but rather about being able to override/extend the behavior consistently. And for that I would prefer that if I override the value callback in a JSON output context, that it would be called for every value being printed, not only for some of them. please, can me show any real use case? JSON is JSON, not art work. Still I don't see any value of this. Thank you for pointing out the case of Explain format, I've totally overlooked it in my first version. Trying to apply the proposed approach in the explain printing code led me to reorganize things slightly. I've added explicit functions for printing keys vs. values, thus no need to expose that key_scalar param anymore. There are now separate before/after key and before/after value functions as well, but I believe it makes for a cleaner code. The most of the complexity is still in the code that decides whether or not to put spaces (between the values or for indentation) and newlines at certain points. Should we decide to unify the style we emit ourselves, this could be simplified, while still leaving room for great flexibility if overridden by an extension, for example. Have a nice weekend. you too Regards Pavel -- Alex
Re: [HACKERS] [PATCH] Generalized JSON output functions
2015-07-11 19:57 GMT+02:00 Shulgin, Oleksandr oleksandr.shul...@zalando.de : On Jul 11, 2015 6:19 PM, Pavel Stehule pavel.steh...@gmail.com wrote: 2015-07-11 18:02 GMT+02:00 Shulgin, Oleksandr oleksandr.shul...@zalando.de: On Fri, Jul 10, 2015 at 5:16 PM, Pavel Stehule pavel.steh...@gmail.com wrote: Well, one could call it premature pessimization due to dynamic call overhead. IMO, the fact that json_out_init_context() sets the value callback to json_out_value is an implementation detail, the other parts of code should not rely on. And for the Explain output, there definitely going to be *some* code between context initialization and output callbacks: these are done in a number of different functions. Again - it is necessary? Postgres still use modular code, not OOP code. I can understand the using of this technique, when I need a possibility to change behave. But these function are used for printing JSON, not printing any others. No, it's not strictly necessary. For me it's not about procedural- vs. object- style, but rather about being able to override/extend the behavior consistently. And for that I would prefer that if I override the value callback in a JSON output context, that it would be called for every value being printed, not only for some of them. please, can me show any real use case? JSON is JSON, not art work. To quote my first mail: The motivation behind this to be able to produce specially-crafted JSON in a logical replication output plugin, such that numeric (and bigint) values are quoted. This requirement, in turn, arises from the fact that JavaScript specification, which is quite natural to expect as a consumer for this JSON data, allows to silently drop significant digits when converting from string to number object. I believe this is a well-known problem and I'm aware of a number of tricks that might be used to avoid it, but none of them seems to be optimal from my standpoint. I can also imagine this can be used to convert date/time to string differently, or adding indentation depending on the depth in object hierarchy, etc. There is simple rule - be strict on output and tolerant on input. If I understand to sense of this patch - the target is one same format of JSON documents - so there are no space for any variability. Still I don't see any value of this. Huh? Why then do you spend time on review? I am thinking so general json functions has sense, but I partially disagree with your implementation. Regards Pavel
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Jul 11, 2015 6:19 PM, Pavel Stehule pavel.steh...@gmail.com wrote: 2015-07-11 18:02 GMT+02:00 Shulgin, Oleksandr oleksandr.shul...@zalando.de: On Fri, Jul 10, 2015 at 5:16 PM, Pavel Stehule pavel.steh...@gmail.com wrote: Well, one could call it premature pessimization due to dynamic call overhead. IMO, the fact that json_out_init_context() sets the value callback to json_out_value is an implementation detail, the other parts of code should not rely on. And for the Explain output, there definitely going to be *some* code between context initialization and output callbacks: these are done in a number of different functions. Again - it is necessary? Postgres still use modular code, not OOP code. I can understand the using of this technique, when I need a possibility to change behave. But these function are used for printing JSON, not printing any others. No, it's not strictly necessary. For me it's not about procedural- vs. object- style, but rather about being able to override/extend the behavior consistently. And for that I would prefer that if I override the value callback in a JSON output context, that it would be called for every value being printed, not only for some of them. please, can me show any real use case? JSON is JSON, not art work. To quote my first mail: The motivation behind this to be able to produce specially-crafted JSON in a logical replication output plugin, such that numeric (and bigint) values are quoted. This requirement, in turn, arises from the fact that JavaScript specification, which is quite natural to expect as a consumer for this JSON data, allows to silently drop significant digits when converting from string to number object. I believe this is a well-known problem and I'm aware of a number of tricks that might be used to avoid it, but none of them seems to be optimal from my standpoint. I can also imagine this can be used to convert date/time to string differently, or adding indentation depending on the depth in object hierarchy, etc. Still I don't see any value of this. Huh? Why then do you spend time on review?
Re: [HACKERS] [PATCH] Generalized JSON output functions
forgotten attachment
Regards
Pavel
2015-07-10 14:34 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com:
Hi
I am sending review of this patch:
1. I reread a previous discussion and almost all are for this patch (me
too)
2. I have to fix a typo in hstore_io.c function (update attached), other
(patching, regress tests) without problems
My objections:
1. comments - missing comment for some basic API, basic fields like
key_scalar and similar
2. why you did indirect call via JsonOutContext?
What is benefit
dst.value(dst, (Datum) 0, JSONTYPE_NULL, InvalidOid, InvalidOid, false);
instead
json_out_value(dst, )
? Is it necessary?
3. if it should be used everywhere, then in EXPLAIN statement too.
Regards
Pavel
2015-07-10 6:31 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com:
2015-07-03 12:27 GMT+02:00 Heikki Linnakangas hlinn...@iki.fi:
On 05/27/2015 09:51 PM, Andrew Dunstan wrote:
On 05/27/2015 02:37 PM, Robert Haas wrote:
On Tue, May 26, 2015 at 2:50 AM, Shulgin, Oleksandr
oleksandr.shul...@zalando.de wrote:
Is it reasonable to add this patch to CommitFest now?
It's always reasonable to add a patch to the CommitFest if you would
like for it to be reviewed and avoid having it get forgotten about.
There seems to be some disagreement about whether we want this, but
don't let that stop you from adding it to the next CommitFest.
I'm not dead set against it either. When I have time I will take a
closer look.
Andrew, will you have the time to review this? Please add yourself as
reviewer in the commitfest app if you do.
My 2 cents is that I agree with your initial reaction: This is a lot of
infrastructure and generalizing things, for little benefit. Let's change
the current code where we generate JSON to be consistent with whitespace,
and call it a day.
I am thinking so it is not bad idea. This code can enforce uniform
format, and it can check if produced value is correct. It can be used in
our code, it can be used by extension's developers.
This patch is not small, but really new lines are not too much.
I'll do review today.
Regards
Pavel
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
new file mode 100644
index 7d89867..0ca223f
*** a/contrib/hstore/hstore_io.c
--- b/contrib/hstore/hstore_io.c
*** hstore_to_json_loose(PG_FUNCTION_ARGS)
*** 1241,1286
int count = HS_COUNT(in);
char *base = STRPTR(in);
HEntry *entries = ARRPTR(in);
! StringInfoData tmp,
! dst;
if (count == 0)
PG_RETURN_TEXT_P(cstring_to_text_with_len({}, 2));
initStringInfo(tmp);
! initStringInfo(dst);
!
! appendStringInfoChar(dst, '{');
for (i = 0; i count; i++)
{
resetStringInfo(tmp);
appendBinaryStringInfo(tmp, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
! escape_json(dst, tmp.data);
! appendStringInfoString(dst, : );
if (HS_VALISNULL(entries, i))
! appendStringInfoString(dst, null);
/* guess that values of 't' or 'f' are booleans */
else if (HS_VALLEN(entries, i) == 1 *(HS_VAL(entries, base, i)) == 't')
! appendStringInfoString(dst, true);
else if (HS_VALLEN(entries, i) == 1 *(HS_VAL(entries, base, i)) == 'f')
! appendStringInfoString(dst, false);
else
{
resetStringInfo(tmp);
appendBinaryStringInfo(tmp, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
if (IsValidJsonNumber(tmp.data, tmp.len))
! appendBinaryStringInfo(dst, tmp.data, tmp.len);
else
! escape_json(dst, tmp.data);
}
-
- if (i + 1 != count)
- appendStringInfoString(dst, , );
}
- appendStringInfoChar(dst, '}');
! PG_RETURN_TEXT_P(cstring_to_text(dst.data));
}
PG_FUNCTION_INFO_V1(hstore_to_json);
--- 1241,1289
int count = HS_COUNT(in);
char *base = STRPTR(in);
HEntry *entries = ARRPTR(in);
! StringInfoData tmp;
! JsonOutContext dst;
if (count == 0)
PG_RETURN_TEXT_P(cstring_to_text_with_len({}, 2));
initStringInfo(tmp);
! json_out_init_context(dst, JSON_OUT_USE_SPACES);
! dst.object_start(dst);
for (i = 0; i count; i++)
{
resetStringInfo(tmp);
appendBinaryStringInfo(tmp, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
! json_out_cstring(dst, tmp.data, true);
!
if (HS_VALISNULL(entries, i))
! dst.value(dst, (Datum) 0, JSONTYPE_NULL, InvalidOid, InvalidOid, false);
!
/* guess that values of 't' or 'f' are booleans */
else if (HS_VALLEN(entries, i) == 1 *(HS_VAL(entries, base, i)) == 't')
! dst.value(dst, BoolGetDatum(true), JSONTYPE_BOOL,
! InvalidOid, InvalidOid, false);
!
else if (HS_VALLEN(entries, i) == 1 *(HS_VAL(entries, base, i)) == 'f')
! dst.value(dst, BoolGetDatum(false), JSONTYPE_BOOL,
! InvalidOid, InvalidOid, false);
else
Re: [HACKERS] [PATCH] Generalized JSON output functions
2015-07-10 16:16 GMT+02:00 Shulgin, Oleksandr oleksandr.shul...@zalando.de : On Fri, Jul 10, 2015 at 4:04 PM, Pavel Stehule pavel.steh...@gmail.com wrote: 2. why you did indirect call via JsonOutContext? What is benefit dst.value(dst, (Datum) 0, JSONTYPE_NULL, InvalidOid, InvalidOid, false); instead json_out_value(dst, ) For consistency. Even though we initialize the output context ourselves, there might be some code introduced between json_out_init_context() and dst.value() calls that replaces some of the callbacks, and then there would be a difference. with this consistency? I didn't see this style everywhere in Postgres? Isn't it premature optimization? Well, one could call it premature pessimization due to dynamic call overhead. IMO, the fact that json_out_init_context() sets the value callback to json_out_value is an implementation detail, the other parts of code should not rely on. And for the Explain output, there definitely going to be *some* code between context initialization and output callbacks: these are done in a number of different functions. Again - it is necessary? Postgres still use modular code, not OOP code. I can understand the using of this technique, when I need a possibility to change behave. But these function are used for printing JSON, not printing any others. Pavel -- Alex
Re: [HACKERS] [PATCH] Generalized JSON output functions
Hi I am sending review of this patch: 1. I reread a previous discussion and almost all are for this patch (me too) 2. I have to fix a typo in hstore_io.c function (update attached), other (patching, regress tests) without problems My objections: 1. comments - missing comment for some basic API, basic fields like key_scalar and similar 2. why you did indirect call via JsonOutContext? What is benefit dst.value(dst, (Datum) 0, JSONTYPE_NULL, InvalidOid, InvalidOid, false); instead json_out_value(dst, ) ? Is it necessary? 3. if it should be used everywhere, then in EXPLAIN statement too. Regards Pavel 2015-07-10 6:31 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com: 2015-07-03 12:27 GMT+02:00 Heikki Linnakangas hlinn...@iki.fi: On 05/27/2015 09:51 PM, Andrew Dunstan wrote: On 05/27/2015 02:37 PM, Robert Haas wrote: On Tue, May 26, 2015 at 2:50 AM, Shulgin, Oleksandr oleksandr.shul...@zalando.de wrote: Is it reasonable to add this patch to CommitFest now? It's always reasonable to add a patch to the CommitFest if you would like for it to be reviewed and avoid having it get forgotten about. There seems to be some disagreement about whether we want this, but don't let that stop you from adding it to the next CommitFest. I'm not dead set against it either. When I have time I will take a closer look. Andrew, will you have the time to review this? Please add yourself as reviewer in the commitfest app if you do. My 2 cents is that I agree with your initial reaction: This is a lot of infrastructure and generalizing things, for little benefit. Let's change the current code where we generate JSON to be consistent with whitespace, and call it a day. I am thinking so it is not bad idea. This code can enforce uniform format, and it can check if produced value is correct. It can be used in our code, it can be used by extension's developers. This patch is not small, but really new lines are not too much. I'll do review today. Regards Pavel - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
2015-07-10 14:34 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com: Hi I am sending review of this patch: 1. I reread a previous discussion and almost all are for this patch (me too) 2. I have to fix a typo in hstore_io.c function (update attached), other (patching, regress tests) without problems My objections: 1. comments - missing comment for some basic API, basic fields like key_scalar and similar I thought it was pretty obvious from the code, because it's sort of the only source for docs on the subject right now. Should we add proper documentation section, this would have been documented for sure. 2. why you did indirect call via JsonOutContext? What is benefit dst.value(dst, (Datum) 0, JSONTYPE_NULL, InvalidOid, InvalidOid, false); instead json_out_value(dst, ) For consistency. Even though we initialize the output context ourselves, there might be some code introduced between json_out_init_context() and dst.value() calls that replaces some of the callbacks, and then there would be a difference. 3. if it should be used everywhere, then in EXPLAIN statement too. Ahh.. good catch. I'll have a look on this now. Thanks for the review! -- Alex
Re: [HACKERS] [PATCH] Generalized JSON output functions
2015-07-10 15:57 GMT+02:00 Shulgin, Oleksandr oleksandr.shul...@zalando.de : 2015-07-10 14:34 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com: Hi I am sending review of this patch: 1. I reread a previous discussion and almost all are for this patch (me too) 2. I have to fix a typo in hstore_io.c function (update attached), other (patching, regress tests) without problems My objections: 1. comments - missing comment for some basic API, basic fields like key_scalar and similar I thought it was pretty obvious from the code, because it's sort of the only source for docs on the subject right now. Should we add proper documentation section, this would have been documented for sure. 2. why you did indirect call via JsonOutContext? What is benefit dst.value(dst, (Datum) 0, JSONTYPE_NULL, InvalidOid, InvalidOid, false); instead json_out_value(dst, ) For consistency. Even though we initialize the output context ourselves, there might be some code introduced between json_out_init_context() and dst.value() calls that replaces some of the callbacks, and then there would be a difference. with this consistency? I didn't see this style everywhere in Postgres? Isn't it premature optimization? 3. if it should be used everywhere, then in EXPLAIN statement too. Ahh.. good catch. I'll have a look on this now. Thanks for the review! -- Alex
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Fri, Jul 10, 2015 at 4:04 PM, Pavel Stehule pavel.steh...@gmail.com wrote: 2. why you did indirect call via JsonOutContext? What is benefit dst.value(dst, (Datum) 0, JSONTYPE_NULL, InvalidOid, InvalidOid, false); instead json_out_value(dst, ) For consistency. Even though we initialize the output context ourselves, there might be some code introduced between json_out_init_context() and dst.value() calls that replaces some of the callbacks, and then there would be a difference. with this consistency? I didn't see this style everywhere in Postgres? Isn't it premature optimization? Well, one could call it premature pessimization due to dynamic call overhead. IMO, the fact that json_out_init_context() sets the value callback to json_out_value is an implementation detail, the other parts of code should not rely on. And for the Explain output, there definitely going to be *some* code between context initialization and output callbacks: these are done in a number of different functions. -- Alex
Re: [HACKERS] [PATCH] Generalized JSON output functions
2015-07-03 12:27 GMT+02:00 Heikki Linnakangas hlinn...@iki.fi: On 05/27/2015 09:51 PM, Andrew Dunstan wrote: On 05/27/2015 02:37 PM, Robert Haas wrote: On Tue, May 26, 2015 at 2:50 AM, Shulgin, Oleksandr oleksandr.shul...@zalando.de wrote: Is it reasonable to add this patch to CommitFest now? It's always reasonable to add a patch to the CommitFest if you would like for it to be reviewed and avoid having it get forgotten about. There seems to be some disagreement about whether we want this, but don't let that stop you from adding it to the next CommitFest. I'm not dead set against it either. When I have time I will take a closer look. Andrew, will you have the time to review this? Please add yourself as reviewer in the commitfest app if you do. My 2 cents is that I agree with your initial reaction: This is a lot of infrastructure and generalizing things, for little benefit. Let's change the current code where we generate JSON to be consistent with whitespace, and call it a day. I am thinking so it is not bad idea. This code can enforce uniform format, and it can check if produced value is correct. It can be used in our code, it can be used by extension's developers. This patch is not small, but really new lines are not too much. I'll do review today. Regards Pavel - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
On 07/03/2015 06:27 AM, Heikki Linnakangas wrote: On 05/27/2015 09:51 PM, Andrew Dunstan wrote: On 05/27/2015 02:37 PM, Robert Haas wrote: On Tue, May 26, 2015 at 2:50 AM, Shulgin, Oleksandr oleksandr.shul...@zalando.de wrote: Is it reasonable to add this patch to CommitFest now? It's always reasonable to add a patch to the CommitFest if you would like for it to be reviewed and avoid having it get forgotten about. There seems to be some disagreement about whether we want this, but don't let that stop you from adding it to the next CommitFest. I'm not dead set against it either. When I have time I will take a closer look. Andrew, will you have the time to review this? Please add yourself as reviewer in the commitfest app if you do. My 2 cents is that I agree with your initial reaction: This is a lot of infrastructure and generalizing things, for little benefit. Let's change the current code where we generate JSON to be consistent with whitespace, and call it a day. - Heikki I'm somewhat on vacation for the next week or so, so I won't claim it, but I'll try to make time to look at it. Other people (Merlin?) could also provide reviews. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
On 05/27/2015 09:51 PM, Andrew Dunstan wrote: On 05/27/2015 02:37 PM, Robert Haas wrote: On Tue, May 26, 2015 at 2:50 AM, Shulgin, Oleksandr oleksandr.shul...@zalando.de wrote: Is it reasonable to add this patch to CommitFest now? It's always reasonable to add a patch to the CommitFest if you would like for it to be reviewed and avoid having it get forgotten about. There seems to be some disagreement about whether we want this, but don't let that stop you from adding it to the next CommitFest. I'm not dead set against it either. When I have time I will take a closer look. Andrew, will you have the time to review this? Please add yourself as reviewer in the commitfest app if you do. My 2 cents is that I agree with your initial reaction: This is a lot of infrastructure and generalizing things, for little benefit. Let's change the current code where we generate JSON to be consistent with whitespace, and call it a day. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Tue, May 26, 2015 at 2:50 AM, Shulgin, Oleksandr oleksandr.shul...@zalando.de wrote: Is it reasonable to add this patch to CommitFest now? It's always reasonable to add a patch to the CommitFest if you would like for it to be reviewed and avoid having it get forgotten about. There seems to be some disagreement about whether we want this, but don't let that stop you from adding it to the next CommitFest. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
On 05/27/2015 02:37 PM, Robert Haas wrote: On Tue, May 26, 2015 at 2:50 AM, Shulgin, Oleksandr oleksandr.shul...@zalando.de wrote: Is it reasonable to add this patch to CommitFest now? It's always reasonable to add a patch to the CommitFest if you would like for it to be reviewed and avoid having it get forgotten about. There seems to be some disagreement about whether we want this, but don't let that stop you from adding it to the next CommitFest. I'm not dead set against it either. When I have time I will take a closer look. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Sat, May 23, 2015 at 3:03 AM, Ryan Pedela rped...@datalanche.com wrote: On Fri, May 22, 2015 at 10:51 AM, Merlin Moncure mmonc...@gmail.com wrote: On Fri, May 22, 2015 at 9:43 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Andrew Dunstan wrote: On 05/20/2015 09:16 AM, Shulgin, Oleksandr wrote: Attached is a patch against master to generalize the JSON-producing functions in utils/adt/json.c and to provide a set of callbacks which can be overridden the same way that is already provided for *parsing* JSON. I'm not necessarily opposed to this, but it sure seems like a lot of changes, and moderately invasive ones, to support something that could be done, at the cost of reparsing, with a simple loadable extension that I could create in a few hours of programming. But this seems like a pretty reasonable change to make, no? Doesn't the total amount of code decrease after this patch? JSON stuff is pretty new so some refactoring and generalization of what we have is to be expected. Yeah. Also, there have been a few previous gripes about this, for example, http://www.postgresql.org/message-id/cahbvmpzs+svr+y-ugxjrq+xw4dqtevl-cozc69zffwmxjck...@mail.gmail.com . As noted, I definitely prefer 'space free' by default for efficiency reasons, but standardizing the output has definitely got to be a reasonable goal. Every JSON implementation I have ever used defaults to the minified version of JSON (no whitespace) when printed. Hashing of arrays seems to be an important issue: we'd rather make sure to produce the same output in every code path. That would also mean: no special logic to add the line feeds in json_agg either. Is it reasonable to add this patch to CommitFest now? -- Alex
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Fri, May 22, 2015 at 10:51 AM, Merlin Moncure mmonc...@gmail.com wrote: On Fri, May 22, 2015 at 9:43 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Andrew Dunstan wrote: On 05/20/2015 09:16 AM, Shulgin, Oleksandr wrote: Attached is a patch against master to generalize the JSON-producing functions in utils/adt/json.c and to provide a set of callbacks which can be overridden the same way that is already provided for *parsing* JSON. I'm not necessarily opposed to this, but it sure seems like a lot of changes, and moderately invasive ones, to support something that could be done, at the cost of reparsing, with a simple loadable extension that I could create in a few hours of programming. But this seems like a pretty reasonable change to make, no? Doesn't the total amount of code decrease after this patch? JSON stuff is pretty new so some refactoring and generalization of what we have is to be expected. Yeah. Also, there have been a few previous gripes about this, for example, http://www.postgresql.org/message-id/cahbvmpzs+svr+y-ugxjrq+xw4dqtevl-cozc69zffwmxjck...@mail.gmail.com . As noted, I definitely prefer 'space free' by default for efficiency reasons, but standardizing the output has definitely got to be a reasonable goal. Every JSON implementation I have ever used defaults to the minified version of JSON (no whitespace) when printed.
Re: [HACKERS] [PATCH] Generalized JSON output functions
Andrew Dunstan wrote: On 05/20/2015 09:16 AM, Shulgin, Oleksandr wrote: Attached is a patch against master to generalize the JSON-producing functions in utils/adt/json.c and to provide a set of callbacks which can be overridden the same way that is already provided for *parsing* JSON. I'm not necessarily opposed to this, but it sure seems like a lot of changes, and moderately invasive ones, to support something that could be done, at the cost of reparsing, with a simple loadable extension that I could create in a few hours of programming. But this seems like a pretty reasonable change to make, no? Doesn't the total amount of code decrease after this patch? JSON stuff is pretty new so some refactoring and generalization of what we have is to be expected. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Fri, May 22, 2015 at 9:43 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Andrew Dunstan wrote: On 05/20/2015 09:16 AM, Shulgin, Oleksandr wrote: Attached is a patch against master to generalize the JSON-producing functions in utils/adt/json.c and to provide a set of callbacks which can be overridden the same way that is already provided for *parsing* JSON. I'm not necessarily opposed to this, but it sure seems like a lot of changes, and moderately invasive ones, to support something that could be done, at the cost of reparsing, with a simple loadable extension that I could create in a few hours of programming. But this seems like a pretty reasonable change to make, no? Doesn't the total amount of code decrease after this patch? JSON stuff is pretty new so some refactoring and generalization of what we have is to be expected. Yeah. Also, there have been a few previous gripes about this, for example, http://www.postgresql.org/message-id/cahbvmpzs+svr+y-ugxjrq+xw4dqtevl-cozc69zffwmxjck...@mail.gmail.com. As noted, I definitely prefer 'space free' by default for efficiency reasons, but standardizing the output has definitely got to be a reasonable goal. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] Generalized JSON output functions
Hi, Hackers!
Attached is a patch against master to generalize the JSON-producing
functions in utils/adt/json.c and to provide a set of callbacks which can
be overridden the same way that is already provided for *parsing* JSON.
The motivation behind this to be able to produce specially-crafted JSON in
a logical replication output plugin, such that numeric (and bigint) values
are quoted. This requirement, in turn, arises from the fact that
JavaScript specification, which is quite natural to expect as a consumer
for this JSON data, allows to silently drop significant digits when
converting from string to number object.
I believe this is a well-known problem and I'm aware of a number of tricks
that might be used to avoid it, but none of them seems to be optimal from
my standpoint.
I can also imagine this can be used to convert date/time to string
differently, or adding indentation depending on the depth in object
hierarchy, etc.
What this patch does apart from providing callbacks, is abstracting most of
code for producing the correct JSON structure, which was previously
scattered and repeated in a number of functions with slight differences.
In the current code there are 5 styles for producing JSON object string,
differing in whitespace only:
a) no spaces
select to_json(row(1,2));
to_json
-
{f1:1,f2:2}
b) some spaces (hstore_to_json)
select hstore(row(1,2))::json;
hstore
{f1: 1, f2: 2}
c) spaces around colon
select json_build_object('f1',1,'f2',2);
json_build_object
--
{f1 : 1, f2 : 2}
d) spaces around colon *and* curly braces
select json_object_agg(x,x) from unnest('{1,2}'::int[]) x;
json_object_agg
--
{ 1 : 1, 2 : 2 }
e) line feeds (row_to_json_pretty)
select row_to_json(row(1,2), true) as row;
row
--
{f1:1,+
f2:2}
Personally, I think we should stick to (b), however that would break a lot
of test cases that already depend on (a). I've tried hard to minimize the
amount of changes in expected/json.out, but it is quickly becomes
cumbersome trying to support all of the above formats. So I've altered (c)
and (d) to look like (b), naturally only whitespace was affected.
There's one corner case I don't see a sensible way to support:
select json_agg(x) from generate_series(1,5) x;
json_agg
-
[1, 2, 3, 4, 5]
With the patch applied it puts line feeds between the array elements
instead of spaces.
What also bothers me is that I've hard-coded output function oids for
cstring_out, and textout on assumption that they never change, but would
like to know that for sure.
Feedback is very much welcome!
--
Alex
PS: using a different email address this time, same Alex Shulgin. ;-)
PPS: sample code for mentioned use case with quoting numeric and bigint:
static void out_value_quote_numerics(JsonOutContext *out, Datum val,
JsonTypeCategory tcategory, Oid typoid, Oid
outfuncoid,
bool key_scalar) {
char *outputstr;
if (typoid == INT8OID || typoid == NUMERICOID) {
out-before_value(out);
outputstr = OidOutputFunctionCall(outfuncoid, val);
escape_json(out-result, outputstr);
pfree(outputstr);
out-after_value(out, key_scalar);
} else {
json_out_value(out, val, tcategory, typoid, outfuncoid, key_scalar);
}
}
...
json_out_init_context(out, JSON_OUT_USE_SPACES);
out-value = out_value_quote_numerics;
diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
new file mode 100644
index 7d89867..1b1e857
*** a/contrib/hstore/hstore_io.c
--- b/contrib/hstore/hstore_io.c
*** hstore_to_json_loose(PG_FUNCTION_ARGS)
*** 1241,1286
int count = HS_COUNT(in);
char *base = STRPTR(in);
HEntry *entries = ARRPTR(in);
! StringInfoData tmp,
! dst;
if (count == 0)
PG_RETURN_TEXT_P(cstring_to_text_with_len({}, 2));
initStringInfo(tmp);
! initStringInfo(dst);
!
! appendStringInfoChar(dst, '{');
for (i = 0; i count; i++)
{
resetStringInfo(tmp);
appendBinaryStringInfo(tmp, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
! escape_json(dst, tmp.data);
! appendStringInfoString(dst, : );
if (HS_VALISNULL(entries, i))
! appendStringInfoString(dst, null);
/* guess that values of 't' or 'f' are booleans */
else if (HS_VALLEN(entries, i) == 1 *(HS_VAL(entries, base, i)) == 't')
! appendStringInfoString(dst, true);
else if (HS_VALLEN(entries, i) == 1 *(HS_VAL(entries, base, i)) == 'f')
! appendStringInfoString(dst, false);
else
{
resetStringInfo(tmp);
appendBinaryStringInfo(tmp, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
if (IsValidJsonNumber(tmp.data, tmp.len))
! appendBinaryStringInfo(dst, tmp.data, tmp.len);
else
! escape_json(dst, tmp.data);
}
-
- if (i + 1 != count)
- appendStringInfoString(dst, , );
}
-
Re: [HACKERS] [PATCH] Generalized JSON output functions
On Wed, May 20, 2015 at 8:16 AM, Shulgin, Oleksandr
oleksandr.shul...@zalando.de wrote:
Hi, Hackers!
Attached is a patch against master to generalize the JSON-producing
functions in utils/adt/json.c and to provide a set of callbacks which can be
overridden the same way that is already provided for *parsing* JSON.
The motivation behind this to be able to produce specially-crafted JSON in a
logical replication output plugin, such that numeric (and bigint) values are
quoted. This requirement, in turn, arises from the fact that JavaScript
specification, which is quite natural to expect as a consumer for this JSON
data, allows to silently drop significant digits when converting from string
to number object.
I believe this is a well-known problem and I'm aware of a number of tricks
that might be used to avoid it, but none of them seems to be optimal from my
standpoint.
I can also imagine this can be used to convert date/time to string
differently, or adding indentation depending on the depth in object
hierarchy, etc.
What this patch does apart from providing callbacks, is abstracting most of
code for producing the correct JSON structure, which was previously
scattered and repeated in a number of functions with slight differences. In
the current code there are 5 styles for producing JSON object string,
differing in whitespace only:
a) no spaces
select to_json(row(1,2));
to_json
-
{f1:1,f2:2}
b) some spaces (hstore_to_json)
select hstore(row(1,2))::json;
hstore
{f1: 1, f2: 2}
c) spaces around colon
select json_build_object('f1',1,'f2',2);
json_build_object
--
{f1 : 1, f2 : 2}
d) spaces around colon *and* curly braces
select json_object_agg(x,x) from unnest('{1,2}'::int[]) x;
json_object_agg
--
{ 1 : 1, 2 : 2 }
e) line feeds (row_to_json_pretty)
select row_to_json(row(1,2), true) as row;
row
--
{f1:1,+
f2:2}
Personally, I think we should stick to (b), however that would break a lot
of test cases that already depend on (a). I've tried hard to minimize the
amount of changes in expected/json.out, but it is quickly becomes cumbersome
trying to support all of the above formats. So I've altered (c) and (d) to
look like (b), naturally only whitespace was affected.
Disagree. IMNSHO, the default should be (a), as it's the most compact
format and therefore the fastest. Whitespace injection should be
reserved for prettification functions.
merlin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Generalized JSON output functions
On 05/20/2015 09:16 AM, Shulgin, Oleksandr wrote: Hi, Hackers! Attached is a patch against master to generalize the JSON-producing functions in utils/adt/json.c and to provide a set of callbacks which can be overridden the same way that is already provided for *parsing* JSON. The motivation behind this to be able to produce specially-crafted JSON in a logical replication output plugin, such that numeric (and bigint) values are quoted. This requirement, in turn, arises from the fact that JavaScript specification, which is quite natural to expect as a consumer for this JSON data, allows to silently drop significant digits when converting from string to number object. I believe this is a well-known problem and I'm aware of a number of tricks that might be used to avoid it, but none of them seems to be optimal from my standpoint. I can also imagine this can be used to convert date/time to string differently, or adding indentation depending on the depth in object hierarchy, etc. I'm not necessarily opposed to this, but it sure seems like a lot of changes, and moderately invasive ones, to support something that could be done, at the cost of reparsing, with a simple loadable extension that I could create in a few hours of programming. The parser API was created precisely to make this sort of transformation close to trivial. Other fairly obvious transformations include translating to XML or YAML, and a less obvious one could be something very specialized, like translating certain fields. Anyway, for this purpose I could imagine a function like: json_format ( j json (or text), indent_spaces smallint default 0, space_after_colon boolean default false, space_after_comma boolean default false, quote_numerics boolean default false) returns json cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
