Re: Re: proposal: ignore null fields in not relation type composite type based constructors

Hello

In Czech Postgres mailing list was user request for serialization to json
without null values.

He needs a similar behave like XMLFOREST has - it ignores NULLs

In some situations and conversions, when your table is +/- sparse matrix,
this request is valid

postgres=# select hstore(omega) from omega;
hstore
─────────────────────────────────
"a"=>"10", "b"=>"20", "c"=>NULL
"a"=>NULL, "b"=>"20", "c"=>"30"
(2 rows)

Proposed function

postgres=# select hstore(omega, ignore_nulls := true) from omega;
hstore
─────────────────────────────────
"a"=>"10", "b"=>"20"
"b"=>"20", "c"=>"30"
(2 rows)

What do you thinking about this proposal?

Regards

Pavel

Hello

I am sending small patch, that allows ignore nulls in row_to_json function.
It allow significant size reduction of generated JSON documents.

Regards

Pavel

2014-05-25 7:53 GMT+02:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:

> Hello
>
> In Czech Postgres mailing list was user request for serialization to json
> without null values.
>
> He needs a similar behave like XMLFOREST has - it ignores NULLs
>
> In some situations and conversions, when your table is +/- sparse matrix,
> this request is valid
>
> postgres=# select hstore(omega) from omega;
> hstore
> ─────────────────────────────────
> "a"=>"10", "b"=>"20", "c"=>NULL
> "a"=>NULL, "b"=>"20", "c"=>"30"
> (2 rows)
>
> Proposed function
>
> postgres=# select hstore(omega, ignore_nulls := true) from omega;
> hstore
> ─────────────────────────────────
> "a"=>"10", "b"=>"20"
> "b"=>"20", "c"=>"30"
> (2 rows)
>
> What do you thinking about this proposal?
>
> Regards
>
> Pavel
>
>

Hi Pavel,

You have said that XMLFOREST has something which ignores nulls, what's that?
Will you please provide an example ?

I am NOT sure, but here you are trying to omit entire field from the output
when its value is NULL. But that will add an extra efforts at other end
which is using output of this. That application need to know all fields and
then need to replace NULL values for missing fields. However we have an
optional argument for ignoring nulls and thus we are safe. Application will
use as per its choice.

Well, apart from that, tried reviewing the patch. Patch was applied but make
failed with following error.

make[3]: Entering directory `/home/jeevan/pg_master/src/backend/catalog'
cd ../../../src/include/catalog && '/usr/bin/perl' ./duplicate_oids
3255
make[3]: *** [postgres.bki] Error 1

Please run unused_oids script to find unused oid.

However, I had a quick look over code changes. At first glance it looks
good. But still need to check on SQL level and then code walk-through.

Waiting for updated patch.

Thanks

--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Hello

2014-08-22 12:21 GMT+02:00 Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>:

> Hi Pavel,
>
> You have said that XMLFOREST has something which ignores nulls, what's
> that?
> Will you please provide an example ?
>

I was partially wrong - XMLFOREST ignore null always

postgres=# select xmlforest(10 as a,20 as b,null as c);
xmlforest
--------------------
<a>10</a><b>20</b>
(1 row)

so if you would to empty elements, you should to use xmlelement and
xmlconcat

postgres=# select xmlconcat(xmlforest(10 as a,20 as b), xmlelement(name c,
null));
xmlconcat
------------------------
<a>10</a><b>20</b><c/>
(1 row)

>
> I am NOT sure, but here you are trying to omit entire field from the output
> when its value is NULL. But that will add an extra efforts at other end
> which is using output of this. That application need to know all fields and
> then need to replace NULL values for missing fields. However we have an
> optional argument for ignoring nulls and thus we are safe. Application will
> use as per its choice.
>

with my patch, you can do decision - lot of REST services doesn't
distinguishes between empty and missing tag - and some developers prefer
remove empty tags due less size of message.

>
> Well, apart from that, tried reviewing the patch. Patch was applied but
> make
> failed with following error.
>
> make[3]: Entering directory `/home/jeevan/pg_master/src/backend/catalog'
> cd ../../../src/include/catalog && '/usr/bin/perl' ./duplicate_oids
> 3255
> make[3]: *** [postgres.bki] Error 1
>
> Please run unused_oids script to find unused oid.
>

it needs remastering

update in attachemnt

>
> However, I had a quick look over code changes. At first glance it looks
> good. But still need to check on SQL level and then code walk-through.
>
> Waiting for updated patch.
>

thank you for review

Regards

Pavel

>
> Thanks
>
> --
> Jeevan B Chalke
> Principal Software Engineer, Product Development
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company
>
>

Hi Pavel,

Patch does look good to me. And found no issues as such.

However here are my optional suggestions:

1. Frankly, I did not like name of the function "row_to_json_pretty_choosy".
Something like "row_to_json_pretty_ignore_nulls" seems better to me.

2. To use ignore nulls feature, I have to always pass pretty flag.
Which seems weired.

Since we do support named argument, can we avoid that?
No idea how much difficult it is. If we have a default arguments to this
function then we do not need one and two argument variations for this
function as well. And we can use named argument for omitting the required
one. Just a thought.

Rest looks good to me.

Thanks

--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

2014-09-01 12:33 GMT+02:00 Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>:

> Hi Pavel,
>
> Patch does look good to me. And found no issues as such.
>
> However here are my optional suggestions:
>
> 1. Frankly, I did not like name of the function
> "row_to_json_pretty_choosy".
> Something like "row_to_json_pretty_ignore_nulls" seems better to me.
>

should be - I have no better name

>
> 2. To use ignore nulls feature, I have to always pass pretty flag.
> Which seems weired.
>
> Since we do support named argument, can we avoid that?
> No idea how much difficult it is. If we have a default arguments to this
> function then we do not need one and two argument variations for this
> function as well. And we can use named argument for omitting the required
> one. Just a thought.
>

it needs a redesign of original implementation, we should to change API to
use default values with named parameters

but it doesn't help too much (although it can be readable little bit more)

instead row_to_json(x, false, true)

be

row_ro_json(x, ignore_null := true)

it is not too much work, but I need a names for parameters

Regards

Pavel

>
> Rest looks good to me.
>
>
> Thanks
>
> --
> Jeevan B Chalke
> Principal Software Engineer, Product Development
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company
>
>

Hi Pavel,

it needs a redesign of original implementation, we should to change API to
> use default values with named parameters
>
> but it doesn't help too much (although it can be readable little bit more)
>
> instead row_to_json(x, false, true)
>
> be
>
> row_ro_json(x, ignore_null := true)
>
> it is not too much work, but I need a names for parameters
>

I have tried adding dummy names (a, b, c) in pg_proc entry you have added.
But that is not sufficient. We need to have default values provided to these
arguments to work row_ro_json(x, ignore_null := true) call.
It was not trivial. So I have not put much thought on that.

For name, I choose (row, pretty, ignore_nulls) or similar.

However it was my thought.
If it is too complex of not so useful then we can ignore it.

Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Hello

2014-09-02 13:54 GMT+02:00 Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>:

> Hi Pavel,
>
> it needs a redesign of original implementation, we should to change API
>> to use default values with named parameters
>>
>> but it doesn't help too much (although it can be readable little bit more)
>>
>> instead row_to_json(x, false, true)
>>
>> be
>>
>> row_ro_json(x, ignore_null := true)
>>
>> it is not too much work, but I need a names for parameters
>>
>
> I have tried adding dummy names (a, b, c) in pg_proc entry you have added.
> But that is not sufficient. We need to have default values provided to
> these
> arguments to work row_ro_json(x, ignore_null := true) call.
> It was not trivial. So I have not put much thought on that.
>
> For name, I choose (row, pretty, ignore_nulls) or similar.
>
> However it was my thought.
> If it is too complex of not so useful then we can ignore it.
>

here is patch

Regards

Pavel

>
> Thanks
> --
> Jeevan B Chalke
> Principal Software Engineer, Product Development
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company
>
>

2014-09-03 7:05 GMT+02:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:

> Hello
>
>
> 2014-09-02 13:54 GMT+02:00 Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>:
>
> Hi Pavel,
>>
>> it needs a redesign of original implementation, we should to change API
>>> to use default values with named parameters
>>>
>>> but it doesn't help too much (although it can be readable little bit
>>> more)
>>>
>>> instead row_to_json(x, false, true)
>>>
>>> be
>>>
>>> row_ro_json(x, ignore_null := true)
>>>
>>> it is not too much work, but I need a names for parameters
>>>
>>
>> I have tried adding dummy names (a, b, c) in pg_proc entry you have added.
>> But that is not sufficient. We need to have default values provided to
>> these
>> arguments to work row_ro_json(x, ignore_null := true) call.
>> It was not trivial. So I have not put much thought on that.
>>
>> For name, I choose (row, pretty, ignore_nulls) or similar.
>>
>
I cannot use "row" because it is keyword - I used "rowval"

Regards

Pavel

>
>> However it was my thought.
>> If it is too complex of not so useful then we can ignore it.
>>
>
> here is patch
>
> Regards
>
> Pavel
>
>
>>
>> Thanks
>> --
>> Jeevan B Chalke
>> Principal Software Engineer, Product Development
>> EnterpriseDB Corporation
>> The Enterprise PostgreSQL Company
>>
>>
>

Hi Pavel,

Here are few more comments on new implementation.

1.
/*
- * SQL function row_to_json(row)
+ * SQL function row_to_json(row record, pretty bool, ignore_nulls bool)
*/

In above comments, parameter name "row" should changed to "rowval".

2.
-DATA(insert OID = 3155 ( row_to_json PGNSP PGUID 12 1 0 0 0 f f f f
t f s 1 0 114 "2249" _null_ _null_ _null_ _null_ row_to_json _null_ _null_
_null_ ));
+DATA(insert OID = 3155 ( row_to_json PGNSP PGUID 12 1 0 0 0 f f f f
t f s 1 0 114 "2249 16 16" _null_ _null_ "{rowval,pretty,ignore_nulls}"
_null_ row_to_json _null_ _null_ _null_ ));

Number of arguments (pronargs) should be 3 now. However, when we create it
again with default values it gets updated. But still here we should not have
inconsistency.

3.
extern Datum row_to_json(PG_FUNCTION_ARGS);
extern Datum row_to_json_pretty(PG_FUNCTION_ARGS);
+extern Datum row_to_json_pretty_choosy(PG_FUNCTION_ARGS);
extern Datum to_json(PG_FUNCTION_ARGS);

With this new implementation, we have NOT added row_to_json_pretty_choosy()
function. So need to remove that added line. Also we have only one function
with default arguments and thus removed row_to_json_pretty() function as
well. Thus need to remove extern for that too.

4.
Can we have couple of test cases with named argument along with skipped
pretty parameter test?

Thanks

--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Hi

2014-09-03 9:27 GMT+02:00 Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>:

> Hi Pavel,
>
> Here are few more comments on new implementation.
>
> 1.
> /*
> - * SQL function row_to_json(row)
> + * SQL function row_to_json(row record, pretty bool, ignore_nulls bool)
> */
>
> In above comments, parameter name "row" should changed to "rowval".
>
> 2.
> -DATA(insert OID = 3155 ( row_to_json PGNSP PGUID 12 1 0 0 0 f f f
> f t f s 1 0 114 "2249" _null_ _null_ _null_ _null_ row_to_json _null_
> _null_ _null_ ));
> +DATA(insert OID = 3155 ( row_to_json PGNSP PGUID 12 1 0 0 0 f f f
> f t f s 1 0 114 "2249 16 16" _null_ _null_ "{rowval,pretty,ignore_nulls}"
> _null_ row_to_json _null_ _null_ _null_ ));
>
> Number of arguments (pronargs) should be 3 now. However, when we create it
> again with default values it gets updated. But still here we should not
> have
> inconsistency.
>
> 3.
> extern Datum row_to_json(PG_FUNCTION_ARGS);
> extern Datum row_to_json_pretty(PG_FUNCTION_ARGS);
> +extern Datum row_to_json_pretty_choosy(PG_FUNCTION_ARGS);
> extern Datum to_json(PG_FUNCTION_ARGS);
>
> With this new implementation, we have NOT added row_to_json_pretty_choosy()
> function. So need to remove that added line. Also we have only one function
> with default arguments and thus removed row_to_json_pretty() function as
> well. Thus need to remove extern for that too.
>
> 4.
> Can we have couple of test cases with named argument along with skipped
> pretty parameter test?
>
>
>
done

Regards

Pavel

>
> Thanks
>
> --
> Jeevan B Chalke
> Principal Software Engineer, Product Development
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company
>
>

Hi Pavel,

You have attached wrong patch.

Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

I am sory

too much patches

Regards

Pavel

2014-09-04 7:35 GMT+02:00 Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>:

> Hi Pavel,
>
> You have attached wrong patch.
>
>
> Thanks
> --
> Jeevan B Chalke
> Principal Software Engineer, Product Development
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company
>
>

On Thu, Sep 4, 2014 at 11:41 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
wrote:

> I am sory
>
> too much patches
>

:)

Patch looks good to me.
Marking "Ready for Committer".

Thanks

>
> Regards
>
> Pavel
>

--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Thank you

Regards

Pavel

2014-09-05 8:29 GMT+02:00 Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>:

>
>
>
> On Thu, Sep 4, 2014 at 11:41 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
>
>> I am sory
>>
>> too much patches
>>
>
> :)
>
> Patch looks good to me.
> Marking "Ready for Committer".
>
> Thanks
>
>
>>
>> Regards
>>
>> Pavel
>>
>
>
> --
> Jeevan B Chalke
> Principal Software Engineer, Product Development
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company
>
>

Pavel, All,

* Pavel Stehule (pavel(dot)stehule(at)gmail(dot)com) wrote:
> Thank you

I made a few minor adjustments, but the bigger issue (in my view) is
what to do about array_to_json_pretty- seems like we should do the same
there, no? Perhaps you could propose a new patch which incorporates my
minor changes and also removes array_to_json_pretty and makes
array_to_json take an optional argument?

Thoughts?

Thanks,

Stephen

Hi

2014-09-08 5:48 GMT+02:00 Stephen Frost <sfrost(at)snowman(dot)net>:

> Pavel, All,
>
> * Pavel Stehule (pavel(dot)stehule(at)gmail(dot)com) wrote:
> > Thank you
>
> I made a few minor adjustments, but the bigger issue (in my view) is
> what to do about array_to_json_pretty- seems like we should do the same
> there, no? Perhaps you could propose a new patch which incorporates my
> minor changes and also removes array_to_json_pretty and makes
> array_to_json take an optional argument?
>

I though about it, and I am not sure. If somebody uses arrays as set, then
it can be good idea, when it is used as array, then you cannot to throw a
nulls from array.

I am thinking, so it is not necessary, because we can remove NULLs from
array simply via iteration over array (what is impossible for row fields)

CREATE OR REPLACE FUNCTION remove_null(anyarray)
RETURNS anyarray AS $$
SELECT ARRAY(SELECT a FROM unnest($1) g(a) WHERE a IS NOT NULL)
$$ LANGUAGE plpgsql;

or this function can be in core for general usage.

ignore_nulls in array_to_json_pretty probably is not necessary. On second
hand, the cost is zero, and we can have it for API consistency.

Regards

Pavel

>
> Thoughts?
>
> Thanks,
>
> Stephen
>

* Pavel Stehule (pavel(dot)stehule(at)gmail(dot)com) wrote:
> ignore_nulls in array_to_json_pretty probably is not necessary. On second
> hand, the cost is zero, and we can have it for API consistency.

I'm willing to be persuaded either way on this, really. I do think it
would be nice for both array_to_json and row_to_json to be single
functions which take default values, but as for if array_to_json has a
ignore_nulls option, I'm on the fence and would defer to people who use
that function regularly (I don't today).

Beyond that, I'm pretty happy moving forward with this patch.

Thanks,

Stephen

2014-09-08 6:27 GMT+02:00 Stephen Frost <sfrost(at)snowman(dot)net>:

> * Pavel Stehule (pavel(dot)stehule(at)gmail(dot)com) wrote:
> > ignore_nulls in array_to_json_pretty probably is not necessary. On second
> > hand, the cost is zero, and we can have it for API consistency.
>
> I'm willing to be persuaded either way on this, really. I do think it
> would be nice for both array_to_json and row_to_json to be single
> functions which take default values, but as for if array_to_json has a
> ignore_nulls option, I'm on the fence and would defer to people who use
> that function regularly (I don't today).
>
> Beyond that, I'm pretty happy moving forward with this patch.
>

ok

Regards

Pavel

>
> Thanks,
>
> Stephen
>

Hi Stephen

Can I help with something, it is there some open question?

Regards

Pavel

2014-09-08 6:27 GMT+02:00 Stephen Frost <sfrost(at)snowman(dot)net>:

> * Pavel Stehule (pavel(dot)stehule(at)gmail(dot)com) wrote:
> > ignore_nulls in array_to_json_pretty probably is not necessary. On second
> > hand, the cost is zero, and we can have it for API consistency.
>
> I'm willing to be persuaded either way on this, really. I do think it
> would be nice for both array_to_json and row_to_json to be single
> functions which take default values, but as for if array_to_json has a
> ignore_nulls option, I'm on the fence and would defer to people who use
> that function regularly (I don't today).
>
> Beyond that, I'm pretty happy moving forward with this patch.
>
> Thanks,
>
> Stephen
>

* Pavel Stehule (pavel(dot)stehule(at)gmail(dot)com) wrote:
> Can I help with something, it is there some open question?

I had been hoping for a more definitive answer regarding this option for
array_to_json, or even a comment about the change to row_to_json.
Andrew- any thoughts on this? (that's what the ping on IRC is for :).

Thanks,

Stephen

> 2014-09-08 6:27 GMT+02:00 Stephen Frost <sfrost(at)snowman(dot)net>:
> > * Pavel Stehule (pavel(dot)stehule(at)gmail(dot)com) wrote:
> > > ignore_nulls in array_to_json_pretty probably is not necessary. On second
> > > hand, the cost is zero, and we can have it for API consistency.
> >
> > I'm willing to be persuaded either way on this, really. I do think it
> > would be nice for both array_to_json and row_to_json to be single
> > functions which take default values, but as for if array_to_json has a
> > ignore_nulls option, I'm on the fence and would defer to people who use
> > that function regularly (I don't today).
> >
> > Beyond that, I'm pretty happy moving forward with this patch.

On 09/11/2014 08:29 AM, Stephen Frost wrote:
> * Pavel Stehule (pavel(dot)stehule(at)gmail(dot)com) wrote:
>> Can I help with something, it is there some open question?
> I had been hoping for a more definitive answer regarding this option for
> array_to_json, or even a comment about the change to row_to_json.
> Andrew- any thoughts on this? (that's what the ping on IRC is for :).

The change in row_to_json looks OK, I think. we're replacing an
overloading with use of default params, yes? That seems quite
reasonable, and users shouldn't notice the difference.

There might be a case for optionally suppressing nulls in array_to_json,
and it might work reasonably since unlike SQL arrays JSON arrays don't
have to be regular (if nested they are arrays of arrays, not
multi-dimensional single arrays). OTOH I'm not sure if it's worth doing
just for the sake of orthogonality. If someone wants it, then go for it.

cheers

andrew

Andrew,

* Andrew Dunstan (andrew(at)dunslane(dot)net) wrote:
> On 09/11/2014 08:29 AM, Stephen Frost wrote:
> >* Pavel Stehule (pavel(dot)stehule(at)gmail(dot)com) wrote:
> >>Can I help with something, it is there some open question?
> >I had been hoping for a more definitive answer regarding this option for
> >array_to_json, or even a comment about the change to row_to_json.
> >Andrew- any thoughts on this? (that's what the ping on IRC is for :).
>
> The change in row_to_json looks OK, I think. we're replacing an
> overloading with use of default params, yes? That seems quite
> reasonable, and users shouldn't notice the difference.

Right. Great, thanks.

> There might be a case for optionally suppressing nulls in
> array_to_json, and it might work reasonably since unlike SQL arrays
> JSON arrays don't have to be regular (if nested they are arrays of
> arrays, not multi-dimensional single arrays). OTOH I'm not sure if
> it's worth doing just for the sake of orthogonality. If someone
> wants it, then go for it.

Ok. I'll handle updating both of these to remove the overloading
and use default params instead, but I'll only add the 'ignore_null'
option to row_to_json.

Thanks again!

Stephen

All,

* Stephen Frost (sfrost(at)snowman(dot)net) wrote:
> Ok. I'll handle updating both of these to remove the overloading
> and use default params instead, but I'll only add the 'ignore_null'
> option to row_to_json.

Barring objections or any issues I find as I go back through it, this is
what I'm planning to push a bit later on tonight.

Thanks!

Stephen

* Stephen Frost (sfrost(at)snowman(dot)net) wrote:
> * Stephen Frost (sfrost(at)snowman(dot)net) wrote:
> > Ok. I'll handle updating both of these to remove the overloading
> > and use default params instead, but I'll only add the 'ignore_null'
> > option to row_to_json.
>
> Barring objections or any issues I find as I go back through it, this is
> what I'm planning to push a bit later on tonight.

Pushed.

Thanks!

Stephen