Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

> On Fri, Sep 22, 2017 at 3:51 PM, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:
> On 9/21/17 11:24, Dmitry Dolgov wrote:
>> One last thing that I need to clarify. Initially there was an idea to
>> minimize changes in `pg_type`
>
> I see, but there is no value in that if it makes everything else more
> complicated.

I'm still working on that, but obviously I'll not manage to finish it
within this CF,
so I'm going to move it to the next one.

Re: [HACKERS] Causal reads take II

[Dmitry Dolgov]

> On 31 July 2017 at 07:49, Thomas Munro <thomas.mu...@enterprisedb.com>
wrote:
>> On Sun, Jul 30, 2017 at 7:07 AM, Dmitry Dolgov <9erthali...@gmail.com>
wrote:
>>
>> I looked through the code of `synchronous-replay-v1.patch` a bit and ran
a few
>> tests. I didn't manage to break anything, except one mysterious error
that I've
>> got only once on one of my replicas, but I couldn't reproduce it yet.
>> Interesting thing is that this error did not affect another replica or
primary.
>> Just in case here is the log for this error (maybe you can see something
>> obvious, that I've not noticed):
>>
>> LOG:  could not remove directory "pg_tblspc/47733/PG_10_201707211/47732":
>> Directory not empty
>> ...
>
> Hmm.  The first error ("could not remove directory") could perhaps be
> explained by temporary files from concurrent backends.
> ...
> Perhaps in your testing you accidentally copied a pgdata directory over
the
> top of it while it was running?  In any case I'm struggling to see how
> anything in this patch would affect anything at the REDO level.

Hmm...no, I don't think so. Basically what I was doing is just running
`installcheck` against a primary instance (I assume there is nothing wrong
with
this approach, am I right?). This particular error was caused by
`tablespace`
test which was failed in this case:

```
INSERT INTO testschema.foo VALUES(1);
ERROR:  could not open file "pg_tblspc/16388/PG_11_201709191/16386/16390":
No such file or directory
```

I tried few more times, and I've got it two times from four attempts on a
fresh
installation (when all instances were on the same machine). But anyway I'll
try
to investigate, maybe it has something to do with my environment.

> > * Also I noticed that some time-related values are hardcoded (e.g.
50%/25%
> >   time shift when we're dealing with leases). Does it make sense to move
> >   them out and make them configurable?
>
> These numbers are interrelated, and I think they're best fixed in that
> ratio.  You could make it more adjustable, but I think it's better to
> keep it simple with just a single knob.

Ok, but what do you think about converting them to constants to make them
more
self explanatory? Like:

```
/*
+ * Since this timestamp is being sent to the standby where it will be
+ * compared against a time generated by the standby's system clock, we
+ * must consider clock skew.  We use 25% of the lease time as max
+ * clock skew, and we subtract that from the time we send with the
+ * following reasoning:
+ */
+int max_clock_skew = synchronous_replay_lease_time *
MAX_CLOCK_SKEW_PORTION;
```

Also I have another question. I tried to test this patch little bit more,
and
I've got some strange behaviour after pgbench (here is the full output [1]):

```
# primary

$ ./bin/pgbench -s 100 -i test

NOTICE:  table "pgbench_history" does not exist, skipping
NOTICE:  table "pgbench_tellers" does not exist, skipping
NOTICE:  table "pgbench_accounts" does not exist, skipping
NOTICE:  table "pgbench_branches" does not exist, skipping
creating tables...
10 of 1000 tuples (1%) done (elapsed 0.11 s, remaining 10.50 s)
20 of 1000 tuples (2%) done (elapsed 1.06 s, remaining 52.00 s)
30 of 1000 tuples (3%) done (elapsed 1.88 s, remaining 60.87 s)
2017-09-30 15:47:26.884 CEST [6035] LOG:  revoking synchronous replay lease
for standby "walreceiver"...
2017-09-30 15:47:26.900 CEST [6035] LOG:  standby "walreceiver" is no
longer available for synchronous replay
2017-09-30 15:47:26.903 CEST [6197] LOG:  revoking synchronous replay lease
for standby "walreceiver"...
40 of 1000 tuples (4%) done (elapsed 2.44 s, remaining 58.62 s)
2017-09-30 15:47:27.979 CEST [6197] LOG:  standby "walreceiver" is no
longer available for synchronous replay
```

```
# replica

2017-09-30 15:47:51.802 CEST [6034] FATAL:  could not receive data from WAL
stream: server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
2017-09-30 15:47:55.154 CEST [6030] LOG:  invalid magic number  in log
segment 00010020, offset 10092544
2017-09-30 15:47:55.257 CEST [10508] LOG:  started streaming WAL from
primary at 0/2000 on timeline 1
2017-09-30 15:48:09.622 CEST [10508] FATAL:  could not receive data from
WAL stream: server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
```

Is it something well known or unrelated to the patch itself?

[1]: https://gist.github.com/erthalion/cdc9357f7437171192348239eb4db764

Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

> On 20 September 2017 at 17:19, Arthur Zakirov 
wrote:
> As a conclusion:
> * additional field are needed to pg_type for *_fetch and *_assign
functions to solve dependency problem

One last thing that I need to clarify. Initially there was an idea to
minimize
changes in `pg_type` - that's why I added only one column there that
contains an
OID of main subscripting function (and everything else you should find out
inside it). But I have no objections about adding more columns if everyone
is
ok with that. Basically pros and cons (marked as + and -):

one new column in `pg_type`:

* less intrusive (+)
* it's neccessary to make a dependency record between subscripting functions
  explicitly (-)

three new columns in `pg_type`:

* more intrusive (-)
* we can create a dependency record between subscripting functions
  simultaneously with a custom type creation (+)
* custom subscripting code does not need to resolve `fetch` and `assign`
  functions (+)

Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

> On 19 September 2017 at 10:21, Arthur Zakirov <a.zaki...@postgrespro.ru>
wrote:
> On Mon, Sep 18, 2017 at 12:25:04PM +0200, Dmitry Dolgov wrote:
>> > I think it would be good to add new catalog table. It may be named as
>> pg_type_sbs or pg_subscripting (second is better I think).
>> > This table may have the fields:
>> > - oid
>> > - sbstype
>> > - sbsinit
>> > - sbsfetch
>> > - sbsassign
>>
>> What is `sbstype`?
>
>'sbstype' is oid of type from pg_type for which subscripting is created.
I.e. pg_type may not have the 'typsubsparse' field.

I'm confused, why do we need it? I mean, isn't it enough to have a
subscripting
oid in a pg_type record?

> On 18 September 2017 at 12:25, Dmitry Dolgov <9erthali...@gmail.com>
wrote:
>
> Overall I have only one concern about this suggestion - basically it
changes
> nothing from the perspective of functionality or implementation quality.

Few more thoughts about this point. Basically if we're going this way (i.e.
having `pg_subscripting`) there will be one possible change of
functionality -
in this case since we store oids of all the required functions, we can pass
them to a `parse` function (so that a custom extension does not need to
resolve
them every time).

At the same time there are consequences of storing `pg_subscripting`, e.g.:

* I assume the performance would be worse because we have to do more
actions to
  actually call a proper function.

* The implementation itself will be little bit more complex I think.

* Should we think about other functionality besides `CREATE` and `DROP`, for
  example `ALTER` (as far as I see aggregations support that).

and maybe something else that I don't see now.

Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

> On 18 September 2017 at 11:39, Arthur Zakirov 
wrote:
> I think it would be good to add new catalog table. It may be named as
pg_type_sbs or pg_subscripting (second is better I think).
> This table may have the fields:
> - oid
> - sbstype
> - sbsinit
> - sbsfetch
> - sbsassign

What is `sbstype`?

> And command 'CREATE SUBSCRIPTING' should add an entry to the
pg_subscripting table. It also adds entries to the pg_depend table:
dependency between pg_type and pg_subscripting, dependency between pg_type
and pg_proc.
> 'DROP SUBSCRIPTING' should drop this entries, it should not drop init
function.

Why we should keep those subscripting functions? From my understanding
they're
totally internal and useless without subscripting context.

Overall I have only one concern about this suggestion - basically it changes
nothing from the perspective of functionality or implementation quality. The
only purpose of it is to make a `subscripting` entity more explicit at the
expense of overhead of having one more catalog table and little bit more
complexity. I'm not really sure if it's necessary or not, and I would
appreciate any commentaries about that topic from the community (to make me
more convinced that this is a good decision or not).

Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

> On 17 September 2017 at 23:34, Arthur Zakirov 
wrote:
>
> I have put some thought into it. What about the following syntax?
>
> CREATE SUBSCRIPTING FOR type_name
>   INITFUNC = subscripting_init_func
>   FETCHFUNC = subscripting_fetch_func
>   ASSIGNFUNC = subscripting_assign_func
> DROP SUBSCRIPTING FOR type_name

Just to clarify, do you mean that `CREATE SUBSCRIPTING FOR` would only make
a
dependency record? In this case `DROP SUBSCRIPTING FOR` actually means just
drop an init function.

Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

> On 17 September 2017 at 00:04, Arthur Zakirov 
wrote:
>
> In my opinion, 'DEPENDS ON' syntax is not actually appropriate here. It
> also looks like a not very good hack to me.

Hm...why do you think about it as a hack?

> Moreover user can implement subscripting to its own type without using
> 'DEPENDS ON' syntax. And he will face the bug mentioned above too.

Yes, but since it will require from a user to create few independent custom
functions for subscripting (as we discussed before, there were few reasons
of
having them as a proper separate function), I don't see how to avoid this
step
of explicitly marking all of them as related to a subscripting logic for
particular data type. And therefore it's possible to forget to do that step
in
spite of what form this step will be. Maybe it's possible to make something
like `CREATE FUNCTION ... FOR SUBSCRIPTING`, then verify that assign/extract
functions are presented and notify user if he missed them (but I would
rather
not do this unless it's really necessary, since it looks like an overkill).

But I'm open to any suggestions, do you have something in mind?

Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

> On 11 September 2017 at 23:45, Tom Lane <t...@sss.pgh.pa.us> wrote:
>
> Dmitry Dolgov <9erthali...@gmail.com> writes:
> >> On 11 September 2017 at 23:19, Tom Lane <t...@sss.pgh.pa.us> wrote:
> >> Uh, what?  Sure you can.  Just because the existing code never has a
> >> reason to create such a dependency doesn't mean it wouldn't work.
>
> > Well, I thought that `pg_depend` was not intended to be used from
> > user-defined code and it's something "internal".
>
> Well, no, we're not expecting that SQL code will manually insert rows
> there.  This feature should have some sort of SQL command that will
> set up the relevant catalog entries, including the dependencies.
> If you don't want to do that, you're going to need the runtime tests.

Sure, an SQL command for that purpose is much better than a runtime check.
I'm going to add such command to the patch, thank you for the information!

Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

> On 11 September 2017 at 23:19, Tom Lane  wrote:
>
> Uh, what?  Sure you can.  Just because the existing code never has a
> reason to create such a dependency doesn't mean it wouldn't work.

Well, I thought that `pg_depend` was not intended to be used from
user-defined
code and it's something "internal". But if I'm wrong then maybe the problem
Arhur raised is a valid reason for that.

Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

> On 9 September 2017 at 23:33, Arthur Zakirov 
wrote:
> PostgreSQL and documentation with the patch compiles without any errors.
All
> regression tests passed.

Thank you!

> But honestly I still cannot say that I agree with *_extract() and
*_assign()
> functions creation way. For example, there is no entry in pg_depend for
them
> ...
> =# drop function custom_subscripting_extract(internal);
> =# select data[0] from test_subscripting;
> ERROR:  function 0x55deb7911bfd returned NULL

Hm...I never thought about the feature in this way. When I was
experimenting I
also tried another approach for this - save to `refevalfunc` a function
pointer to an appropriate function. For simple situations it was ok, but
there
were questions about how it would work with node-related functions from
`outfuncs`/`copyfuncs` etc. Another my idea was to find out an actual
`refevalfunc` not at the time of a node creation but later on - this was
also
questionable since in this case we need to carry a lot of information with
a node
just for this sole purpose. Maybe you can suggest something else?

About dependencies between functions - as far as I understand one cannot
create
a `pg_depend` entry or any other kind of dependencies between custom
user-defined functions. So yes, looks like with the current approach the
only
solution would be to check in the `_parse` function that `_extract` and
`_assign` functions are existed (which is inconvenient).

> For example, there is no entry in pg_depend

Are there any other disadvantages of this approach?

Re: [HACKERS] log_destination=file

[Dmitry Dolgov]

> On 7 September 2017 at 15:46, Dmitry Dolgov <9erthali...@gmail.com> wrote:
>
> It seems that for this particular workload it was about 20-25% slower.​

Hmm...looks like I provided misleading data, sorry. The numbers from
previous
email are correct and I'm able to reproduce them, but surprisingly for me
only
for one particular situation when `log_statement=all;
log_destination=stderr`
and stderr is sent to a console with a tmux session running in it (and
apparently
it has some impact on the performance, but I'm not sure how exactly). In all
other cases (when stderr is sent to a plain console, /dev/null, or we send
logs
to a file) numbers are different, and the difference between patch and
master
is quite less significant.

average latency:

clients patch   master

10  0.321   0.286

20  0.669   0.602

30  1.016   0.942

40  1.358   1.280

50  1.727   1.637

Re: [HACKERS] log_destination=file

[Dmitry Dolgov]

> On 8 September 2017 at 01:32, Magnus Hagander  wrote:
>
> 1. where was stderr actually sent? To the console, to /dev/null or to a
file?

To the console (but I can also try other options, although I'm not sure if
it would have any impact).

> 2. Could you run the same thing (on the same machine) with the logging
collector on and logging to file, without the patch? I'd assume that gives
performance similar to running with the patch, but it would be good to
confirm that.

Sure, I'll do it this weekend.

Re: [HACKERS] log_destination=file

[Dmitry Dolgov]

Hi

> On 31 August 2017 at 14:49, Tom Lane  wrote:
>> Are you actually asking for a benchmark of if logging gets slower?
>
> Yes.
>
>> If so,
>> could you suggest a workload to make an actual benchmark of it (where
>> logging would be high enough that it could be come a bottleneck -- and
not
>> writing the log data to disk, but the actual logging). I'm not sure what
a
>> good one would be.
>
> pgbench with log_statement = all would be a pretty easy test case.

This part of the discussion caught my attention, and I tried to perform such
easy test. I was doing:

pgbench -S -j2 -c${clients} -T500 test

with `log_statement=all` and `log_destination=stderr`, what I assume should
be
enough to get some approximation for numbers.

scaling factor: 100
average latency:

clients patch   master

10  1.827   1.456

20  4.027   3.300

30  6.284   4.921

40  8.409   6.767

50  10.985 8.646

It seems that for this particular workload it was about 20-25% slower.

Re: [HACKERS] Causal reads take II

[Dmitry Dolgov]

> On 12 July 2017 at 23:45, Thomas Munro 
wrote:
> I renamed it to "synchronous replay", because "causal reads" seemed a bit
too
> arcane.

Hi

I looked through the code of `synchronous-replay-v1.patch` a bit and ran a
few
tests. I didn't manage to break anything, except one mysterious error that
I've
got only once on one of my replicas, but I couldn't reproduce it yet.
Interesting thing is that this error did not affect another replica or
primary.
Just in case here is the log for this error (maybe you can see something
obvious, that I've not noticed):

```
LOG:  could not remove directory "pg_tblspc/47733/PG_10_201707211/47732":
Directory not empty
CONTEXT:  WAL redo at 0/125F4D90 for Tablespace/DROP: 47733
LOG:  could not remove directory "pg_tblspc/47733/PG_10_201707211":
Directory not empty
CONTEXT:  WAL redo at 0/125F4D90 for Tablespace/DROP: 47733
LOG:  could not remove directory "pg_tblspc/47733/PG_10_201707211/47732":
Directory not empty
CONTEXT:  WAL redo at 0/125F4D90 for Tablespace/DROP: 47733
LOG:  could not remove directory "pg_tblspc/47733/PG_10_201707211":
Directory not empty
CONTEXT:  WAL redo at 0/125F4D90 for Tablespace/DROP: 47733
LOG:  directories for tablespace 47733 could not be removed
HINT:  You can remove the directories manually if necessary.
CONTEXT:  WAL redo at 0/125F4D90 for Tablespace/DROP: 47733
FATAL:  could not create directory "pg_tblspc/47734/PG_10_201707211/47732":
File exists
CONTEXT:  WAL redo at 0/125F5768 for Storage/CREATE:
pg_tblspc/47734/PG_10_201707211/47732/47736
LOG:  startup process (PID 8034) exited with exit code 1
LOG:  terminating any other active server processes
LOG:  database system is shut down
```

And speaking about the code, so far I have just a few notes (some of them
merely questions):

* In general the idea behind this patch sounds interesting for me, but it
  relies heavily on time synchronization. As mentioned in the documentation:
  "Current hardware clocks, NTP implementations and public time servers are
  unlikely to allow the system clocks to differ more than tens or hundreds
of
  milliseconds, and systems synchronized with dedicated local time servers
may
  be considerably more accurate." But as far as I remember from my own
  experience sometimes it maybe not that trivial on something like AWS
because
  of virtualization. Maybe it's an unreasonable fear, but is it possible to
  address this problem somehow?

* Also I noticed that some time-related values are hardcoded (e.g. 50%/25%
  time shift when we're dealing with leases). Does it make sense to move
them
  out and make them configurable?

* Judging from the `SyncReplayPotentialStandby` function, it's possible to
have
  `synchronous_replay_standby_names = "*, server_name"`, which is basically
an
  equivalent for just `*`, but it looks confusing. Is it worth it to prevent
  this behaviour?

* In the same function `SyncReplayPotentialStandby` there is this code:

```
if (!SplitIdentifierString(rawstring, ',', ))
{
/* syntax error in list */
pfree(rawstring);
list_free(elemlist);
/* GUC machinery will have already complained - no need to do again */
return false;
}
```

  Am I right that ideally this (a situation when at this point in the code
  `synchronous_replay_standby_names` has incorrect value) should not happen,
  because GUC will prevent us from that? If yes, then it looks for me that
it
  still makes sense to put here a log message, just to give more
information in
  a potentially weird situation.

* In the function `SyncRepReleaseWaiters` there is a commentary:

```
  /*
  * If the number of sync standbys is less than requested or we aren't
* managing a sync standby or a standby in synchronous replay state that
* blocks then just leave.
  * /
if ((!got_recptr || !am_sync) && !walsender_sr_blocker)
```

  Is this commentary correct? If I understand everything right
`!got_recptr` -
  the number of sync standbys is less than requested (a), `!am_sync` - we
aren't
  managing a sync standby (b), `walsender_sr_blocker` - a standby in
synchronous
  replay state that blocks (c). Looks like condition is `(a or b) and not
c`.

* In the function `ProcessStandbyReplyMessage` there is a code that
implements
  this:

```
 * If the standby reports that it has fully replayed the WAL for at
least
* 10 seconds, then let's clear the lag times that were measured when it
* last wrote/flushed/applied a WAL record.  This way we avoid displaying
* stale lag data until more WAL traffic arrives.
```
  but I never found any mention of this 10 seconds in the documentation. Is
it
  not that important? Also, question 2 is related to this one.

* In the function `WalSndGetSyncReplayStateString` all the states are in
lower
  case except `UNKNOWN`, is there any particular reason for that?

There are also few more not that important notes (mostly about some typos
and
few confusing names), but I'm going to do another round of review and
testing
anyway so I'll just send them all 

Re: [HACKERS] JSONB - JSONB operator feature request

[Dmitry Dolgov]

> On 20 July 2017 at 16:24, David Fetter  wrote:
>
> If we can agree to a definition, we can make this go.  My vague
> memories from graph theory indicate that that "agree to a definition"
> part is the real problem to be solved.

I tried to embody some relevant thoughts in this thread [1], I think it
would be great if
you can take a look and suggest something more.

[1]:
https://www.postgresql.org/message-id/ca+q6zcu+gy1+dxqd09msz8zwqq+sppfs-6gykmynqgvqdfe...@mail.gmail.com

[HACKERS] Proposal about a "deep" versions for some jsonb functions

[Dmitry Dolgov]

Hi

As far as I know, since 9.5 we're missing some convenient features, namely a
deepversion of `jsonb_concat` and `jsonb_minus`. There are already few
feature
requests about `jsonb_minus` (see [1], [2]) and a lot of confusion and
requests
about a deep version of `jsonb_concat`. From my point of view they're pretty
much related, so I want to propose the following description for this
functionality and eventually implement it.

# jsonb_minus

```
jsonb_minus(jsonb, jsonb, deep=False)
```

Looks like we have to abandon "-" operator for that purpose (see a concern
about that in this thread [2]).

In general this functionality is something like the relative complement for
two
jsonb objects. Basically we're taking all the paths inside all jsonb objects
and remove duplicated paths from the left one. Of course an actual
implementation may be different, but I think it's a nice way of thinking
about
this logic.

Here are few examples, where "->" is an operation to get an actual value,
".->" - an operation to get a next key, "#->" an operation to get a value
from
an array ("-" operator is just for the sake of readability):

--

{"a": 1} - {"a": 1}
=> null

paths:
 a -> 1

 a -> 1

--

{"a": 1} - {"a": 2}
=> {"a": 1}

paths:
 a -> 1

 a -> 2

--

{"a": 1} - {"a": {"b": 1}}
=> {"a": 1}

paths:
 a ->

 a -> .b -> 1

--

{"a": 1, "b": {"c": 2}} - {"b": 1, "b": {"c": 3}}
=> {"b": {"c": 2}}

paths:
 a -> 1
 b .-> c -> 2

 b -> 1
 b .-> c -> 3

--

{"a": {"b": 1}} - {"a": {"b": 1}}
=> null

paths:
 a .-> b -> 1

 a .-> b -> 1

--

{"a": {"b": 1, "c": 2}} - {"a": {"b": 1}}
=> {"a": {"b": 1}}

paths:
 a .-> b -> 1
 a .-> c -> 2

 a .-> b -> 1

--

{"a": {
"b": {"b1": 1},
"c": {"c2": 2}
}}

-

{"a": {
"b": {"b1": 1},
"c": {"c2": 3}
}}
=> {"a": {"c": {"c2": 2}}

paths:
 a .-> b .-> b1 -> 1
 a .-> c .-> c2 -> 2

 a .-> b .-> b1 -> 1
 a .-> c .-> c2 -> 3

--

{"a": [1, 2, 3]} - {"a": [1, 2]}
=> {"a": [3]}

paths:
 a #-> 1
 a #-> 2
 a #-> 3

 a #-> 1
 a #-> 2

--

{"a": [{"b": 1}, {"c": 2}]} - {"a": [{"b": 1}, {"c": 3}]}
=> {"a": [{"c": 3}]}

paths:
 a #-> b -> 1
 a #-> c -> 2

 a #-> b -> 1
 a #-> c -> 3


But judging from the previous discussions, there is a demand for a bit
different behavior, when `jsonb_minus` is operating only on the top level of
jsonb objects. For that purpose I suggest introducing a flag `deep`, that
should be false by default (as for `jsonb_concat`), that will allow to
enable a
"deep logic" (a.k.a relative complement) I described above. With
`deep=False`
this function will behave similar to `hstore`:

{"a": 1, "b": {"c": 2}} - {"a": 1, "b": {"c": 3}}
=> {"a": 1}

# jsonb_concat

We already have this function implemented, but a "deep" mode is missing.

```
jsonb_concat(jsonb, jsonb, deep=False)
```

Basically we're taking all the paths inside all jsonb objects and override
duplicated paths in the left one, then add all unique paths from right one
to
the result.

Here are few examples for deep mode ("||" operator is just for the sake of
readability):

--

{"a": 1, "b": {"c": 2}} || {"a": 1, "b": {"d": 3}}
=> {"a": 1, "b": {"c": 2, "d": 3}}

paths:
 a -> 1
 b .-> c -> 2

 a -> 1
 b .-> d -> 3

--

{"a": 1, "b": {"c": 2}} || {"a": 1, "b": {"c": 3}}
=> {"a": 1, "b": {"c": 3}}

paths:
 a -> 1
 b .-> c -> 2

 a -> 1
 b .-> c -> 3

--

{"a": [1, 2, 3]} || {"a": [3, 4]}
=> {"a": [1, 2, 3, 4]}

paths:
 a #-> 1
 a #-> 2
 a #-> 3

 a #-> 3
 a #-> 4


What do you think about that?

[1]:
https://www.postgresql.org/message-id/flat/CAHyXU0wtJ%2Bi-4MC5FPVc_oFu%2B3-tQVC8u04GmMNwYdPEAX1XSA%40mail.gmail.com#cahyxu0wtj+i-4mc5fpvc_ofu+3-tqvc8u04gmmnwydpeax1...@mail.gmail.com
[2]:
https://www.postgresql.org/message-id/flat/CAHyXU0wm0pkX0Gvzb5BH2jUAA_%3DswMJmyYuhBWzgOjfKxdrKfw%40mail.gmail.com#CAHyXU0wm0pkX0Gvzb5BH2jUAA_=swmjmyyuhbwzgojfkxdr...@mail.gmail.com

Re: [HACKERS] Causal reads take II

[Dmitry Dolgov]

> On 23 June 2017 at 13:48, Thomas Munro 
wrote:
>
> Apologies for the extended delay.  Here is the rebased patch, now with a
> couple of improvements (see below).

Thank you. I started to play with it a little bit, since I think it's an
interesting idea. And there are already few notes:

* I don't see a CF item for that, where is it?

* Looks like there is a sort of sensitive typo in `postgresql.conf.sample`:

```
+#causal_reads_standy_names = '*' # standby servers that can potentially
become
+ # available for causal reads; '*' = all
+
```

it should be `causal_reads_standby_names`. Also I hope in the nearest
future I
can provide a full review.

Re: [HACKERS] [PATCH] pg_reload_backend to signal SIGHUP to a specific backend

[Dmitry Dolgov]

> On 29 Jun 2017 08:18, "Yugo Nagata"  wrote:
>
> This is because this function is defined as strict

Yes, I realized that few moments after I sent my message.

> Arg type is needed.
>
> =# grant execute on function pg_reload_backend(int) to test_user;

Oh, I see, thank you.

Re: [HACKERS] [PATCH] pg_reload_backend to signal SIGHUP to a specific backend

[Dmitry Dolgov]

> On 28 June 2017 at 12:17, Yugo Nagata  wrote:
>
> Hi,
>
> Attached is a patch of pg_reload_backend that is a function signaling
> SIGHUP to a specific backend. The original idea is from Michael
Paquier[1].
> The documatation isn't included in this patch yet.

I have few questions. I'm curious, why this function returns something
different from bool when I'm passing null as an argument?

=# select pg_reload_backend(27961);
WARNING:  PID 27961 is not a PostgreSQL server process
WARNING:  failed to send signal to backend: 27961
 pg_reload_backend
---
 f
(1 row)

=# select pg_reload_backend(27962);
 pg_reload_backend
---
 t
(1 row)

=# select pg_reload_backend(null);
 pg_reload_backend
---

(1 row)

Also for some reason I can't grant an execute permission on this function,
am I doing something wrong?

=# grant execute on function pg_reload_backend() to test_user;
ERROR:  function pg_reload_backend() does not exist
=# grant execute on function pg_reload_conf() to test_user;
GRANT

Re: [HACKERS] type cache for concat functions

[Dmitry Dolgov]

> On 20 May 2017 at 10:03, Pavel Stehule  wrote:
>
> Now concat is 2x times slower than || operator. With cached FmgrInfo for
> output function it will be only 5%.

Looks nice and does what's expected (what else one may need from a Saturday
evening). I just can mention that from what I see the previous version of
`concat` is slower the more arguments are getting involved, so looks like it
can be more than 2x.

Also, it was a little bit confusing to see that the function `concat`
behaves
differently from operator `||` in terms of performance. When you're looking
at
the code it's becoming obvious, but I couldn't find any mention about that
in
the documentation.

Re: [HACKERS] Create subscription with `create_slot=false` and incorrect slot name

[Dmitry Dolgov]

> On 26 May 2017 at 23:05, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:
>
> > On 5/25/17 19:16, Petr Jelinek wrote:
> >> The reported error is just one of many errors that can happen when DROP
> >> SUBSCRIPTION tries to drop the slot (doens't exist, still active, no
> >> permission, etc.).  We don't want to give the hint that is effectively
> >> "just forget about the slot then" for all of them.  So we would need
> >> some way to distinguish "you can't do this right now" from "this would
> >> never work" (400 vs 500 errors).
> >>
> > This specific error returns ERRCODE_UNDEFINED_OBJECT error code so we
> > could check for it and offer hint only for this case.
>
> We would have to extend the walreceiver interface a little to pass that
> through, but it seems doable.

Just to make it clear for me. If I understand correctly, it should be more
or
less easy to extend it in that way, something like in the attached patch.
Or am I missing something?
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 86eb31d..9f7d73c 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -941,10 +941,20 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
 		res = walrcv_exec(wrconn, cmd.data, 0, NULL);
 
 		if (res->status != WALRCV_OK_COMMAND)
-			ereport(ERROR,
-			(errmsg("could not drop the replication slot \"%s\" on publisher",
-	slotname),
-			 errdetail("The error was: %s", res->err)));
+		{
+			if (res->errcode == ERRCODE_UNDEFINED_OBJECT)
+ereport(ERROR,
+(errmsg("could not drop the replication slot \"%s\" on publisher",
+		slotname),
+ errdetail("The error was: %s", res->err),
+ errhint("Use ALTER SUBSCRIPTION ... SET (slot_name = NONE) "
+		 "to disassociate the subscription from the slot.")));
+			else
+ereport(ERROR,
+(errmsg("could not drop the replication slot \"%s\" on publisher",
+		slotname),
+ errdetail("The error was: %s", res->err)));
+		}
 		else
 			ereport(NOTICE,
 	(errmsg("dropped replication slot \"%s\" on publisher",
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index ebe9c91..1caa051 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -873,6 +873,7 @@ libpqrcv_exec(WalReceiverConn *conn, const char *query,
 {
 	PGresult   *pgres = NULL;
 	WalRcvExecResult *walres = palloc0(sizeof(WalRcvExecResult));
+	const char *errorcode = NULL;
 
 	if (MyDatabaseId == InvalidOid)
 		ereport(ERROR,
@@ -915,6 +916,9 @@ libpqrcv_exec(WalReceiverConn *conn, const char *query,
 		case PGRES_FATAL_ERROR:
 		case PGRES_BAD_RESPONSE:
 			walres->status = WALRCV_ERROR;
+			errorcode = PQresultErrorField(pgres, PG_DIAG_SQLSTATE);
+			walres->errcode = MAKE_SQLSTATE(errorcode[0], errorcode[1], errorcode[2],
+			errorcode[3], errorcode[4]);
 			walres->err = pchomp(PQerrorMessage(conn->streamConn));
 			break;
 	}
diff --git a/src/include/replication/walreceiver.h b/src/include/replication/walreceiver.h
index 31d090c..b0582af 100644
--- a/src/include/replication/walreceiver.h
+++ b/src/include/replication/walreceiver.h
@@ -186,6 +186,7 @@ typedef struct WalRcvExecResult
 {
 	WalRcvExecStatus status;
 	char	   *err;
+	int			errcode;
 	Tuplestorestate *tuplestore;
 	TupleDesc	tupledesc;
 } WalRcvExecResult;

-- 
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] Create subscription with `create_slot=false` and incorrect slot name

[Dmitry Dolgov]

On 23 May 2017 at 07:26, Euler Taveira  wrote:
>
> ReplicationSlotValidateName() should be called in
parse_subscription_options() to avoid a pilot error.
> IMHO we should prevent a future error (use of invalid slot name).

Yes, I see now. I assume this little patch should be enough for that.
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 86eb31d..625b5e9 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -39,6 +39,7 @@
 
 #include "replication/logicallauncher.h"
 #include "replication/origin.h"
+#include "replication/slot.h"
 #include "replication/walreceiver.h"
 #include "replication/walsender.h"
 #include "replication/worker_internal.h"
@@ -138,6 +139,9 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
 			*slot_name_given = true;
 			*slot_name = defGetString(defel);
 
+			/* Validate slot_name even if create_slot = false */
+			ReplicationSlotValidateName(*slot_name, ERROR);
+
 			/* Setting slot_name = NONE is treated as no slot name. */
 			if (strcmp(*slot_name, "none") == 0)
 *slot_name = NULL;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 91ba8ab..6334a18 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -62,6 +62,9 @@ CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpu
 ERROR:  subscription with slot_name = NONE must also set create_slot = false
 CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = false);
 ERROR:  subscription with slot_name = NONE must also set enabled = false
+CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = "invalid name", create_slot = false);
+ERROR:  replication slot name "invalid name" contains invalid character
+HINT:  Replication slot names may only contain lower case letters, numbers, and the underscore character.
 -- ok - with slot_name = NONE
 CREATE SUBSCRIPTION testsub3 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, connect = false);
 WARNING:  tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 4b694a3..5b635bc 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -47,6 +47,7 @@ CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpu
 CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE);
 CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = false);
 CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = false);
+CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = "invalid name", create_slot = false);
 
 -- ok - with slot_name = NONE
 CREATE SUBSCRIPTION testsub3 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, connect = false);

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Create subscription with `create_slot=false` and incorrect slot name

[Dmitry Dolgov]

Hi

Maybe this question was already raised before, but I couldn't find a
discussion. When I'm creating a subscription with `create_slot=false` looks
like it's possible to pass a slot name with invalid characters. In this
particular case both publisher and subscriber were on the same machine:

=# CREATE SUBSCRIPTION test_sub CONNECTION 'dbname=db host=host
port=port user=user' PUBLICATION test_pub WITH (create_slot=false,
slot_name='test slot');
NOTICE:  0: synchronized table states
LOCATION:  CreateSubscription, subscriptioncmds.c:443
CREATE SUBSCRIPTION
Time: 5.887 ms

Of course `test slot` doesn't exist, because I can't create a slot with
incorrect name. And consequently I can't drop this subscription:

=# DROP SUBSCRIPTION test_sub;
ERROR:  XX000: could not drop the replication slot "test slot" on
publisher
DETAIL:  The error was: ERROR:  replication slot name "test slot"
contains invalid character
HINT:  Replication slot names may only contain lower case letters,
numbers, and the underscore character.
LOCATION:  DropSubscription, subscriptioncmds.c:947
Time: 2.615 ms

=# CREATE SUBSCRIPTION test_sub CONNECTION 'dbname=db host=host
port=port user=user' PUBLICATION test_pub WITH (create_slot=false,
slot_name='test slot');
ERROR:  42710: subscription "test_sub" already exists
LOCATION:  CreateSubscription, subscriptioncmds.c:344
Time: 0.492 ms

Is it an issue or I'm doing something wrong?

Re: [HACKERS] Causal reads take II

[Dmitry Dolgov]

Hi

I'm wondering about status of this patch and how can I try it out?

> On 3 January 2017 at 02:43, Thomas Munro 
wrote:
> The replay lag tracking patch this depends on is in the current commitfest

I assume you're talking about this patch [1] (at least it's the only thread
where I could find a `replay-lag-v16.patch`)? But `replay lag tracking` was
returned with feedback, so what's the status of this one (`causal reads`)?

> First apply replay-lag-v16.patch, then refactor-syncrep-exit-v16.patch,
then
> causal-reads-v16.patch.

It would be nice to have all three of them attached (for some reason I see
only
last two of them in this thread). But anyway there are a lot of failed hunks
when I'm trying to apply `replay-lag-v16.patch` and
`refactor-syncrep-exit-v16.patch`,
`causal-reads-v16.patch` (or last two of them separately).

[1] https://commitfest.postgresql.org/12/920/

Re: [HACKERS] logical decoding of two-phase transactions

[Dmitry Dolgov]

On 13 May 2017 at 22:22, Tom Lane  wrote:
>
> Apparently you are not testing against current HEAD.  That's been there
> since d10c626de (a whole two days now ;-))

Indeed, I was working on a more than two-day old antiquity. Unfortunately,
it's even more complicated
to apply this patch against the current HEAD, so I'll wait for a rebased
version.

Re: [HACKERS] logical decoding of two-phase transactions

[Dmitry Dolgov]

Hi

> On 4 April 2017 at 19:13, Masahiko Sawada  wrote:
>
> Other than that issue current patch still could not pass 'make check'
> test of contrib/test_decoding.

Just a note about this patch. Of course time flies by and it needs rebase,
but also there are few failing tests right now:

* one that was already mentioned by Masahiko
* one from `ddl`, where expected is:

```
SELECT slot_name, plugin, slot_type, active,
NOT catalog_xmin IS NULL AS catalog_xmin_set,
xmin IS NULl  AS data_xmin_not_set,
pg_wal_lsn_diff(restart_lsn, '0/0100') > 0 AS some_wal
FROM pg_replication_slots;
slot_name|plugin | slot_type | active | catalog_xmin_set |
data_xmin_not_set | some_wal
-+---+---++--+---+--
 regression_slot | test_decoding | logical   | f  | t|
t | t
(1 row)
```

but the result is:

```
SELECT slot_name, plugin, slot_type, active,
NOT catalog_xmin IS NULL AS catalog_xmin_set,
xmin IS NULl  AS data_xmin_not_set,
pg_wal_lsn_diff(restart_lsn, '0/0100') > 0 AS some_wal
FROM pg_replication_slots;
ERROR:  function pg_wal_lsn_diff(pg_lsn, unknown) does not exist
LINE 5: pg_wal_lsn_diff(restart_lsn, '0/0100') > 0 AS some_w...
^
HINT:  No function matches the given name and argument types. You might
need to add explicit type casts.
```

[HACKERS] Valgrind & tests for `numeric`

[Dmitry Dolgov]

Hi

Recently I noticed, that when I'm running the regression tests under
Valgrind 3.9.0,
one test for `numeric` is constantly failing:

Here is what expected:
```
SELECT i as pow,
round((-2.5 * 10 ^ i)::numeric, -i),
round((-1.5 * 10 ^ i)::numeric, -i),
round((-0.5 * 10 ^ i)::numeric, -i),
round((0.5 * 10 ^ i)::numeric, -i),
round((1.5 * 10 ^ i)::numeric, -i),
round((2.5 * 10 ^ i)::numeric, -i)
FROM generate_series(-5,5) AS t(i);
 pow |  round   |  round   |  round   |  round  |  round  |  round
-+--+--+--+-+-+-
  -5 | -0.3 | -0.2 | -0.1 | 0.1 | 0.2 | 0.3
  -4 |  -0.0003 |  -0.0002 |  -0.0001 |  0.0001 |  0.0002 |  0.0003
  -3 |   -0.003 |   -0.002 |   -0.001 |   0.001 |   0.002 |   0.003
  -2 |-0.03 |-0.02 |-0.01 |0.01 |0.02 |0.03
  -1 | -0.3 | -0.2 | -0.1 | 0.1 | 0.2 | 0.3
   0 |   -3 |   -2 |   -1 |   1 |   2 |   3
   1 |  -30 |  -20 |  -10 |  10 |  20 |  30
   2 | -300 | -200 | -100 | 100 | 200 | 300
   3 |-3000 |-2000 |-1000 |1000 |2000 |3000
   4 |   -3 |   -2 |   -1 |   1 |   2 |   3
   5 |  -30 |  -20 |  -10 |  10 |  20 |  30
(11 rows)
```

and here is what I've got (see the last row):
```
SELECT i as pow,
round((-2.5 * 10 ^ i)::numeric, -i),
round((-1.5 * 10 ^ i)::numeric, -i),
round((-0.5 * 10 ^ i)::numeric, -i),
round((0.5 * 10 ^ i)::numeric, -i),
round((1.5 * 10 ^ i)::numeric, -i),
round((2.5 * 10 ^ i)::numeric, -i)
FROM generate_series(-5,5) AS t(i);
 pow |  round   |  round   |  round   |  round  |  round  |  round
-+--+--+--+-+-+-
  -5 | -0.3 | -0.2 | -0.1 | 0.1 | 0.2 | 0.3
  -4 |  -0.0003 |  -0.0002 |  -0.0001 |  0.0001 |  0.0002 |  0.0003
  -3 |   -0.003 |   -0.002 |   -0.001 |   0.001 |   0.002 |   0.003
  -2 |-0.03 |-0.02 |-0.01 |0.01 |0.02 |0.03
  -1 | -0.3 | -0.2 | -0.1 | 0.1 | 0.2 | 0.3
   0 |   -3 |   -2 |   -1 |   1 |   2 |   3
   1 |  -30 |  -20 |  -10 |  10 |  20 |  30
   2 | -300 | -200 | -100 | 100 | 200 | 300
   3 |-3000 |-2000 |-1000 |1000 |2000 |3000
   4 |   -3 |   -2 |   -1 |   1 |   2 |   3
   5 |  -30 |  -20 |0 |   0 |  20 |  30
(11 rows)
```

I can see this behavior on the latest master with `USE_VALGRIND` enabled. Is
it something well known?

Re: [HACKERS] Function to move the position of a replication slot

[Dmitry Dolgov]

> On 4 May 2017 at 20:05, Magnus Hagander  wrote:
>
> PFA a patch that adds a new function, pg_move_replication_slot, that
makes it
> possible to move the location of a replication slot without actually
> consuming all the WAL on it.

Just a few questions just a few questions out of curiosity:

* does it make sense to create a few tests for this function in
  `contrib/test_decoding` (as shown in attachment)?

* what should happen if the second argument is `NULL`? There is a
verification
  `XLogRecPtrIsInvalid(moveto)`, but it's possible to pass `NULL`, and looks
  like it leads to result different from boolean:

```
SELECT pg_move_replication_slot('regression_slot_5', NULL);
 pg_move_replication_slot
--

(1 row)
```
diff --git a/contrib/test_decoding/expected/slot.out b/contrib/test_decoding/expected/slot.out
index 9f5f8a9..7c69110 100644
--- a/contrib/test_decoding/expected/slot.out
+++ b/contrib/test_decoding/expected/slot.out
@@ -101,3 +101,39 @@ SELECT pg_drop_replication_slot('regression_slot1');
 ERROR:  replication slot "regression_slot1" does not exist
 SELECT pg_drop_replication_slot('regression_slot2');
 ERROR:  replication slot "regression_slot2" does not exist
+SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot_3', 'test_decoding');
+ ?column? 
+--
+ init
+(1 row)
+
+SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot_4', 'test_decoding', true);
+ ?column? 
+--
+ init
+(1 row)
+
+SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot_5', 'test_decoding');
+ ?column? 
+--
+ init
+(1 row)
+
+SELECT pg_move_replication_slot('regression_slot_3', '0/aabbccdd');
+ pg_move_replication_slot 
+--
+ t
+(1 row)
+
+SELECT pg_move_replication_slot('regression_slot_4', '0/aabbccdd');
+ pg_move_replication_slot 
+--
+ t
+(1 row)
+
+SELECT pg_move_replication_slot('regression_slot_5', NULL);
+ pg_move_replication_slot 
+--
+ 
+(1 row)
+
diff --git a/contrib/test_decoding/sql/slot.sql b/contrib/test_decoding/sql/slot.sql
index fa9561f..207f845 100644
--- a/contrib/test_decoding/sql/slot.sql
+++ b/contrib/test_decoding/sql/slot.sql
@@ -53,3 +53,12 @@ SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot1', 'test_
 -- both should error as they should be dropped on error
 SELECT pg_drop_replication_slot('regression_slot1');
 SELECT pg_drop_replication_slot('regression_slot2');
+
+SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot_3', 'test_decoding');
+SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot_4', 'test_decoding', true);
+
+SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot_5', 'test_decoding');
+
+SELECT pg_move_replication_slot('regression_slot_3', '0/aabbccdd');
+SELECT pg_move_replication_slot('regression_slot_4', '0/aabbccdd');
+SELECT pg_move_replication_slot('regression_slot_5', NULL);
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 6298657..d05974e 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18986,6 +18986,24 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
 be called when connected to the same database the slot was created on.

   
+  
+   
+
+ pg_move_replication_slot
+
+pg_move_replication_slot(slot_name name, position pg_lsn)
+   
+   
+bool
+   
+   
+Moves the current restart position of a physical or logical
+replication slot named slot_name.
+The slot will not be moved backwards, and it will not be
+moved beyond the current insert location. Returns true if
+the position was changed.
+   
+  
 
   

diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 6ee1e68..a9faa10 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -176,6 +176,66 @@ pg_drop_replication_slot(PG_FUNCTION_ARGS)
 }
 
 /*
+ * SQL function for moving the position in a replication slot.
+ */
+Datum
+pg_move_replication_slot(PG_FUNCTION_ARGS)
+{
+	Name		slotname = PG_GETARG_NAME(0);
+	XLogRecPtr	moveto = PG_GETARG_LSN(1);
+	char	   *slotnamestr;
+	bool		changed = false;
+	bool 		backwards = false;
+
+	Assert(!MyReplicationSlot);
+
+	check_permissions();
+
+	if (XLogRecPtrIsInvalid(moveto))
+		ereport(ERROR,
+(errmsg("Invalid target xlog position ")));
+
+	/* Temporarily acquire the slot so we "own" it */
+	ReplicationSlotAcquire(NameStr(*slotname));
+
+	if (moveto > GetXLogWriteRecPtr())
+		/* Can't move past current position, so truncate there */
+		moveto = GetXLogWriteRecPtr();
+
+	/* Now adjust it */
+	SpinLockAcquire(>mutex);
+	if (MyReplicationSlot->data.restart_lsn != moveto)
+	{
+		/* Never move backwards, because bad things can happen */
+		if 

Re: [HACKERS] [PATCH] few fts functions for jsonb

[Dmitry Dolgov]

On 31 March 2017 at 00:01, Andrew Dunstan 
wrote:
>
> I have just noticed as I was writing/testing the non-existent docs for
> this patch that it doesn't supply variants of to_tsvector that take a
> regconfig as the first argument. Is there a reason for that? Why
> should the json(b) versions be different from the text versions?

No, there is no reason, I just missed that. Here is a new version of the
patch (only the functions part)
to add those variants.
diff --git a/src/backend/tsearch/to_tsany.c b/src/backend/tsearch/to_tsany.c
index 6e5de8f..f19383e 100644
--- a/src/backend/tsearch/to_tsany.c
+++ b/src/backend/tsearch/to_tsany.c
@@ -16,6 +16,7 @@
 #include "tsearch/ts_cache.h"
 #include "tsearch/ts_utils.h"
 #include "utils/builtins.h"
+#include "utils/jsonapi.h"
 
 
 typedef struct MorphOpaque
@@ -24,6 +25,14 @@ typedef struct MorphOpaque
 	int			qoperator;		/* query operator */
 } MorphOpaque;
 
+typedef struct TSVectorBuildState
+{
+	ParsedText	*prs;
+	TSVector	result;
+	Oid			cfgId;
+} TSVectorBuildState;
+
+static void add_to_tsvector(void *state, char *elem_value, int elem_len);
 
 Datum
 get_current_ts_config(PG_FUNCTION_ARGS)
@@ -256,6 +265,135 @@ to_tsvector(PG_FUNCTION_ARGS)
 		PointerGetDatum(in)));
 }
 
+Datum
+jsonb_to_tsvector_byid(PG_FUNCTION_ARGS)
+{
+	Oid	cfgId = PG_GETARG_OID(0);
+	Jsonb*jb = PG_GETARG_JSONB(1);
+	TSVectorBuildState	state;
+	ParsedText			*prs = (ParsedText *) palloc(sizeof(ParsedText));
+
+	prs->words = NULL;
+	state.result = NULL;
+	state.cfgId = cfgId;
+	state.prs = prs;
+
+	iterate_jsonb_string_values(jb, , (JsonIterateStringValuesAction) add_to_tsvector);
+
+	PG_FREE_IF_COPY(jb, 1);
+
+	if (state.result == NULL)
+	{
+		/* There weren't any string elements in jsonb,
+		 * so wee need to return an empty vector */
+
+		if (prs->words != NULL)
+			pfree(prs->words);
+
+		state.result = palloc(CALCDATASIZE(0, 0));
+		SET_VARSIZE(state.result, CALCDATASIZE(0, 0));
+		state.result->size = 0;
+	}
+
+	PG_RETURN_TSVECTOR(state.result);
+}
+
+Datum
+jsonb_to_tsvector(PG_FUNCTION_ARGS)
+{
+	Jsonb	*jb = PG_GETARG_JSONB(0);
+	Oid		cfgId;
+
+	cfgId = getTSCurrentConfig(true);
+	PG_RETURN_DATUM(DirectFunctionCall2(jsonb_to_tsvector_byid,
+		ObjectIdGetDatum(cfgId),
+		JsonbGetDatum(jb)));
+}
+
+Datum
+json_to_tsvector_byid(PG_FUNCTION_ARGS)
+{
+	Oid	cfgId = PG_GETARG_OID(0);
+	text*json = PG_GETARG_TEXT_P(1);
+	TSVectorBuildState	state;
+	ParsedText			*prs = (ParsedText *) palloc(sizeof(ParsedText));
+
+	prs->words = NULL;
+	state.result = NULL;
+	state.cfgId = cfgId;
+	state.prs = prs;
+
+	iterate_json_string_values(json, , (JsonIterateStringValuesAction) add_to_tsvector);
+
+	PG_FREE_IF_COPY(json, 1);
+	if (state.result == NULL)
+	{
+		/* There weren't any string elements in json,
+		 * so wee need to return an empty vector */
+
+		if (prs->words != NULL)
+			pfree(prs->words);
+
+		state.result = palloc(CALCDATASIZE(0, 0));
+		SET_VARSIZE(state.result, CALCDATASIZE(0, 0));
+		state.result->size = 0;
+	}
+
+	PG_RETURN_TSVECTOR(state.result);
+}
+
+Datum
+json_to_tsvector(PG_FUNCTION_ARGS)
+{
+	text	*json = PG_GETARG_TEXT_P(0);
+	Oid		cfgId;
+
+	cfgId = getTSCurrentConfig(true);
+	PG_RETURN_DATUM(DirectFunctionCall2(json_to_tsvector_byid,
+		ObjectIdGetDatum(cfgId),
+		PointerGetDatum(json)));
+}
+
+/*
+ * Extend current TSVector from _state with a new one,
+ * build over a json(b) element.
+ */
+static void
+add_to_tsvector(void *_state, char *elem_value, int elem_len)
+{
+	TSVectorBuildState *state = (TSVectorBuildState *) _state;
+	ParsedText	*prs = state->prs;
+	TSVector	item_vector;
+	int			i;
+
+	prs->lenwords = elem_len / 6;
+	if (prs->lenwords == 0)
+		prs->lenwords = 2;
+
+	prs->words = (ParsedWord *) palloc(sizeof(ParsedWord) * prs->lenwords);
+	prs->curwords = 0;
+	prs->pos = 0;
+
+	parsetext(state->cfgId, prs, elem_value, elem_len);
+
+	if (prs->curwords)
+	{
+		if (state->result != NULL)
+		{
+			for (i = 0; i < prs->curwords; i++)
+prs->words[i].pos.pos = prs->words[i].pos.pos + TS_JUMP;
+
+			item_vector = make_tsvector(prs);
+
+			state->result = (TSVector) DirectFunctionCall2(tsvector_concat,
+	TSVectorGetDatum(state->result),
+	PointerGetDatum(item_vector));
+		}
+		else
+			state->result = make_tsvector(prs);
+	}
+}
+
 /*
  * to_tsquery
  */
diff --git a/src/backend/tsearch/wparser.c b/src/backend/tsearch/wparser.c
index 8ca1c62..6e4e445 100644
--- a/src/backend/tsearch/wparser.c
+++ b/src/backend/tsearch/wparser.c
@@ -20,6 +20,7 @@
 #include "tsearch/ts_cache.h"
 #include "tsearch/ts_utils.h"
 #include "utils/builtins.h"
+#include "utils/jsonapi.h"
 #include "utils/varlena.h"
 
 
@@ -31,6 +32,19 @@ typedef struct
 	LexDescr   *list;
 } TSTokenTypeStorage;
 
+/* state for ts_headline_json_* */
+typedef struct HeadlineJsonState
+{
+	HeadlineParsedText *prs;
+	TSConfigCacheEntry *cfg;
+	TSParserCacheEntry *prsobj;
+	TSQueryquery;
+	List

Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

On 30 March 2017 at 19:36, Arthur Zakirov  wrote:
>
> The last point is about the tutorial. As Tom pointed it is not useful
when the tutorial doesn't execute. It happens because there is not "custom"
type in subscripting.sql.

I'm confused. Maybe I'm missing something, but there is "custom" type in
this file:

```
-- subscripting.sql

CREATE TYPE custom (
   internallength = 8,
   input = custom_in,
   output = custom_out,
   subscripting = custom_subscript_parse
);
```

```
> \i subscripting.sql
psql:subscripting.sql:39: NOTICE:  42704: type "custom" is not yet defined
DETAIL:  Creating a shell type definition.
LOCATION:  compute_return_type, functioncmds.c:141
CREATE FUNCTION
Time: 4.257 ms
psql:subscripting.sql:47: NOTICE:  42809: argument type custom is only a
shell
LOCATION:  interpret_function_parameter_list, functioncmds.c:245
CREATE FUNCTION
Time: 37.038 ms
CREATE FUNCTION
Time: 13.891 ms
CREATE FUNCTION
Time: 0.946 ms
CREATE FUNCTION
Time: 1.161 ms
CREATE TYPE
Time: 1.336 ms
CREATE TABLE
Time: 2.129 ms
INSERT 0 1
Time: 2.501 ms
 data
--
2
(1 row)

Time: 0.960 ms
UPDATE 1
Time: 0.887 ms
```

So the only problem I see is notification about "type 'custom' is not yet
defined", but it's the same for "complex" tutorial

```
> \i complex.sql
psql:complex.sql:39: NOTICE:  42704: type "complex" is not yet defined
DETAIL:  Creating a shell type definition.
LOCATION:  compute_return_type, functioncmds.c:141
CREATE FUNCTION
Time: 1.741 ms
psql:complex.sql:47: NOTICE:  42809: argument type complex is only a shell
LOCATION:  interpret_function_parameter_list, functioncmds.c:245
CREATE FUNCTION
Time: 0.977 ms
psql:complex.sql:55: NOTICE:  42809: return type complex is only a shell
LOCATION:  compute_return_type, functioncmds.c:105
CREATE FUNCTION
Time: 0.975 ms
psql:complex.sql:63: NOTICE:  42809: argument type complex is only a shell
LOCATION:  interpret_function_parameter_list, functioncmds.c:245
CREATE FUNCTION
Time: 0.893 ms
CREATE TYPE
Time: 0.992 ms
...
```

Can you clarify this point?

Re: [HACKERS] [PATCH] few fts functions for jsonb

[Dmitry Dolgov]

> On 29 March 2017 at 18:28, Andrew Dunstan 
wrote:
>
> These patches seem fundamentally OK. But I'm still not happy with the
> naming etc.

I've changed names for all functions and action definitions, moved out the
changes in header file to `jsonapi.h` and removed `is_jsonb_data` macro. So
it
should be better now.
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 6a7aab2..c9f86b0 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -52,6 +52,25 @@ typedef struct OkeysState
 	int			sent_count;
 } OkeysState;
 
+/* state for iterate_json function */
+typedef struct IterateJsonState
+{
+	JsonLexContext	*lex;
+	JsonIterateStringValuesAction	action;			/* an action that will be applied
+	   to each json value */
+	void			*action_state;	/* any necessary context for iteration */
+} IterateJsonState;
+
+/* state for transform_json function */
+typedef struct TransformJsonState
+{
+	JsonLexContext	*lex;
+	StringInfo		strval;			/* resulting json */
+	JsonTransformStringValuesAction	action;			/* an action that will be applied
+	   to each json value */
+	void			*action_state;	/* any necessary context for transformation */
+} TransformJsonState;
+
 /* state for json_get* functions */
 typedef struct GetState
 {
@@ -271,6 +290,18 @@ static void setPathArray(JsonbIterator **it, Datum *path_elems,
 			 int level, Jsonb *newval, uint32 nelems, int op_type);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
+/* function supporting iterate_json(b) */
+static void iterate_string_values_scalar(void *state, char *token, JsonTokenType tokentype);
+
+/* function supporting transform_json(b) */
+static void transform_string_values_object_start(void *state);
+static void transform_string_values_object_end(void *state);
+static void transform_string_values_array_start(void *state);
+static void transform_string_values_array_end(void *state);
+static void transform_string_values_object_field_start(void *state, char *fname, bool isnull);
+static void transform_string_values_array_element_start(void *state, bool isnull);
+static void transform_string_values_scalar(void *state, char *token, JsonTokenType tokentype);
+
 
 /*
  * SQL function json_object_keys
@@ -4130,3 +4161,208 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 		}
 	}
 }
+
+/*
+ * Iterate over jsonb string values or elements, and pass them together with an
+ * iteration state to a specified JsonIterateStringValuesAction.
+ */
+void
+iterate_jsonb_string_values(Jsonb *jb, void *state, JsonIterateStringValuesAction action)
+{
+	JsonbIterator		*it;
+	JsonbValue			v;
+	JsonbIteratorToken	type;
+
+	it = JsonbIteratorInit(>root);
+
+	while ((type = JsonbIteratorNext(, , false)) != WJB_DONE)
+	{
+		if ((type == WJB_VALUE || type == WJB_ELEM) && v.type == jbvString)
+		{
+			action(state, v.val.string.val, v.val.string.len);
+		}
+	}
+}
+
+/*
+ * Iterate over json string values or elements, and pass them together with an
+ * iteration state to a specified JsonIterateStringValuesAction.
+ */
+void
+iterate_json_string_values(text *json, void *action_state, JsonIterateStringValuesAction action)
+{
+	JsonLexContext *lex = makeJsonLexContext(json, true);
+	JsonSemAction *sem = palloc0(sizeof(JsonSemAction));
+	IterateJsonState   *state = palloc0(sizeof(IterateJsonState));
+
+	state->lex = lex;
+	state->action = action;
+	state->action_state = action_state;
+
+	sem->semstate = (void *) state;
+	sem->scalar = iterate_string_values_scalar;
+
+	pg_parse_json(lex, sem);
+}
+
+/*
+ * An auxiliary function for iterate_json_string_values to invoke a specified
+ * JsonIterateStringValuesAction.
+ */
+static void
+iterate_string_values_scalar(void *state, char *token, JsonTokenType tokentype)
+{
+	IterateJsonState   *_state = (IterateJsonState *) state;
+	if (tokentype == JSON_TOKEN_STRING)
+		(*_state->action) (_state->action_state, token, strlen(token));
+}
+
+/*
+ * Iterate over a jsonb, and apply a specified JsonTransformStringValuesAction
+ * to every string value or element. Any necessary context for a
+ * JsonTransformStringValuesAction can be passed in the action_state variable.
+ * Function returns a copy of an original jsonb object with transformed values.
+ */
+Jsonb *
+transform_jsonb_string_values(Jsonb *jsonb, void *action_state,
+			  JsonTransformStringValuesAction transform_action)
+{
+	JsonbIterator		*it;
+	JsonbValue			v, *res = NULL;
+	JsonbIteratorToken	type;
+	JsonbParseState		*st = NULL;
+	text*out;
+	boolis_scalar = false;
+
+	it = JsonbIteratorInit(>root);
+	is_scalar = it->isScalar;
+
+	while ((type = JsonbIteratorNext(, , false)) != WJB_DONE)
+	{
+		if ((type == WJB_VALUE || type == WJB_ELEM) && v.type == jbvString)
+		{
+			out = transform_action(action_state, v.val.string.val, v.val.string.len);
+			v.val.string.val = VARDATA_ANY(out);
+			

Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

On 28 March 2017 at 11:58, Arthur Zakirov  wrote:
>
> Your patch reverts commits from 25-26 march. And therefore contains 15000
lines.

Wow, I didn't notice that, sorry - will fix it shortly.

Re: [HACKERS] [PATCH] few fts functions for jsonb

[Dmitry Dolgov]

> I'm not through looking at this. However, here are a few preliminary
comments

I've attached new versions of the patches with improvements related to
these commentaries.
diff --git a/src/backend/tsearch/to_tsany.c b/src/backend/tsearch/to_tsany.c
index 6e5de8f..8f7bcfe 100644
--- a/src/backend/tsearch/to_tsany.c
+++ b/src/backend/tsearch/to_tsany.c
@@ -16,6 +16,7 @@
 #include "tsearch/ts_cache.h"
 #include "tsearch/ts_utils.h"
 #include "utils/builtins.h"
+#include "utils/jsonb.h"
 
 
 typedef struct MorphOpaque
@@ -24,6 +25,14 @@ typedef struct MorphOpaque
 	int			qoperator;		/* query operator */
 } MorphOpaque;
 
+typedef struct TSVectorBuildState
+{
+	ParsedText	*prs;
+	TSVector	result;
+	Oid			cfgId;
+} TSVectorBuildState;
+
+static void add_to_tsvector(void *state, char *elem_value, int elem_len);
 
 Datum
 get_current_ts_config(PG_FUNCTION_ARGS)
@@ -256,6 +265,109 @@ to_tsvector(PG_FUNCTION_ARGS)
 		PointerGetDatum(in)));
 }
 
+Datum
+jsonb_to_tsvector(PG_FUNCTION_ARGS)
+{
+	Jsonb*jb = PG_GETARG_JSONB(0);
+	TSVectorBuildState	state;
+	ParsedText			*prs = (ParsedText *) palloc(sizeof(ParsedText));
+
+	prs->words = NULL;
+	state.result = NULL;
+	state.cfgId = getTSCurrentConfig(true);
+	state.prs = prs;
+
+	iterate_jsonb_values(jb, , (JsonIterateAction) add_to_tsvector);
+
+	PG_FREE_IF_COPY(jb, 1);
+
+	if (state.result == NULL)
+	{
+		/* There weren't any string elements in jsonb,
+		 * so wee need to return an empty vector */
+
+		if (prs->words != NULL)
+			pfree(prs->words);
+
+		state.result = palloc(CALCDATASIZE(0, 0));
+		SET_VARSIZE(state.result, CALCDATASIZE(0, 0));
+		state.result->size = 0;
+	}
+
+	PG_RETURN_TSVECTOR(state.result);
+}
+
+Datum
+json_to_tsvector(PG_FUNCTION_ARGS)
+{
+	text*json = PG_GETARG_TEXT_P(0);
+	TSVectorBuildState	state;
+	ParsedText			*prs = (ParsedText *) palloc(sizeof(ParsedText));
+
+	prs->words = NULL;
+	state.result = NULL;
+	state.cfgId = getTSCurrentConfig(true);
+	state.prs = prs;
+
+	iterate_json_values(json, , (JsonIterateAction) add_to_tsvector);
+
+	PG_FREE_IF_COPY(json, 1);
+	if (state.result == NULL)
+	{
+		/* There weren't any string elements in json,
+		 * so wee need to return an empty vector */
+
+		if (prs->words != NULL)
+			pfree(prs->words);
+
+		state.result = palloc(CALCDATASIZE(0, 0));
+		SET_VARSIZE(state.result, CALCDATASIZE(0, 0));
+		state.result->size = 0;
+	}
+
+	PG_RETURN_TSVECTOR(state.result);
+}
+
+/*
+ * Extend current TSVector from _state with a new one,
+ * build over a json(b) element.
+ */
+static void
+add_to_tsvector(void *_state, char *elem_value, int elem_len)
+{
+	TSVectorBuildState *state = (TSVectorBuildState *) _state;
+	ParsedText	*prs = state->prs;
+	TSVector	item_vector;
+	int			i;
+
+	prs->lenwords = elem_len / 6;
+	if (prs->lenwords == 0)
+		prs->lenwords = 2;
+
+	prs->words = (ParsedWord *) palloc(sizeof(ParsedWord) * prs->lenwords);
+	prs->curwords = 0;
+	prs->pos = 0;
+
+	parsetext(state->cfgId, prs, elem_value, elem_len);
+
+	if (prs->curwords)
+	{
+		if (state->result != NULL)
+		{
+			for (i = 0; i < prs->curwords; i++)
+prs->words[i].pos.pos = prs->words[i].pos.pos + TS_JUMP;
+
+			item_vector = make_tsvector(prs);
+
+			state->result = (TSVector) DirectFunctionCall2(tsvector_concat,
+	TSVectorGetDatum(state->result),
+	PointerGetDatum(item_vector));
+		}
+		else
+			state->result = make_tsvector(prs);
+	}
+}
+
 /*
  * to_tsquery
  */
diff --git a/src/backend/tsearch/wparser.c b/src/backend/tsearch/wparser.c
index 8ca1c62..ab1716a 100644
--- a/src/backend/tsearch/wparser.c
+++ b/src/backend/tsearch/wparser.c
@@ -20,6 +20,7 @@
 #include "tsearch/ts_cache.h"
 #include "tsearch/ts_utils.h"
 #include "utils/builtins.h"
+#include "utils/jsonb.h"
 #include "utils/varlena.h"
 
 
@@ -31,6 +32,19 @@ typedef struct
 	LexDescr   *list;
 } TSTokenTypeStorage;
 
+/* state for ts_headline_json_* */
+typedef struct HeadlineJsonState
+{
+	HeadlineParsedText *prs;
+	TSConfigCacheEntry *cfg;
+	TSParserCacheEntry *prsobj;
+	TSQueryquery;
+	List*prsoptions;
+	booltransformed;
+} HeadlineJsonState;
+
+static text * headline_json_value(void *_state, char *elem_value, int elem_len);
+
 static void
 tt_setup_firstcall(FuncCallContext *funcctx, Oid prsid)
 {
@@ -362,3 +376,177 @@ ts_headline_opt(PG_FUNCTION_ARGS)
 		PG_GETARG_DATUM(1),
 		PG_GETARG_DATUM(2)));
 }
+
+Datum
+ts_headline_jsonb_byid_opt(PG_FUNCTION_ARGS)
+{
+	Jsonb			*out, *jb = PG_GETARG_JSONB(1);
+	TSQuery			query = PG_GETARG_TSQUERY(2);
+	text			*opt = (PG_NARGS() > 3 && PG_GETARG_POINTER(3)) ? PG_GETARG_TEXT_P(3) : NULL;
+
+	HeadlineParsedText prs;
+	HeadlineJsonState *state = palloc0(sizeof(HeadlineJsonState));
+
+	memset(, 0, sizeof(HeadlineParsedText));
+	prs.lenwords = 32;
+	prs.words = (HeadlineWordEntry *) palloc(sizeof(HeadlineWordEntry) * prs.lenwords);
+
+	state->prs = 
+	state->cfg = lookup_ts_config_cache(PG_GETARG_OID(0));
+	state->prsobj = lookup_ts_parser_cache(state->cfg->prsId);
+	

Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

On 24 March 2017 at 15:39, David Steele  wrote:
>
> Do you have an idea when you will have a patch ready?

Yes, I'll prepare a new version with most important changes in two days.

Re: [HACKERS] [PATCH] few fts functions for jsonb

[Dmitry Dolgov]

> On 21 March 2017 at 03:03, Andrew Dunstan 
wrote:
>
> However, I think it should probably be broken up into a couple of pieces -
> one for the generic json/jsonb transforms infrastructure (which probably
> needs some more comments) and one for the FTS functions that will use it.

Sure, here are two patches with separated functionality and a bit more
commentaries for the transform functions.
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 6a7aab2..bac08c0 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -42,6 +42,8 @@
 #define JB_PATH_CREATE_OR_INSERT \
 	(JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER | JB_PATH_CREATE)
 
+#define is_jsonb_data(type) (type == WJB_KEY || type == WJB_VALUE || type == WJB_ELEM)
+
 /* state for json_object_keys */
 typedef struct OkeysState
 {
@@ -52,6 +54,23 @@ typedef struct OkeysState
 	int			sent_count;
 } OkeysState;
 
+/* state for iterate_json function */
+typedef struct IterateJsonState
+{
+	JsonLexContext		*lex;
+	JsonIterateAction	action;			/* an action that will be applied to each json value */
+	void*action_state;	/* any necessary context for iteration */
+} IterateJsonState;
+
+/* state for transform_json function */
+typedef struct TransformJsonState
+{
+	JsonLexContext		*lex;
+	StringInfo			strval;			/* resulting json */
+	JsonTransformAction	action;			/* an action that will be applied to each json value */
+	void*action_state;	/* any necessary context for transformation */
+} TransformJsonState;
+
 /* state for json_get* functions */
 typedef struct GetState
 {
@@ -271,6 +290,18 @@ static void setPathArray(JsonbIterator **it, Datum *path_elems,
 			 int level, Jsonb *newval, uint32 nelems, int op_type);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
+/* function supporting iterate_json(b) */
+static void apply_action(void *state, char *token, JsonTokenType tokentype);
+
+/* function supporting transform_json(b) */
+static void transform_object_start(void *state);
+static void transform_object_end(void *state);
+static void transform_array_start(void *state);
+static void transform_array_end(void *state);
+static void transform_object_field_start(void *state, char *fname, bool isnull);
+static void transform_array_element_start(void *state, bool isnull);
+static void transform_scalar(void *state, char *token, JsonTokenType tokentype);
+
 
 /*
  * SQL function json_object_keys
@@ -4130,3 +4161,206 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 		}
 	}
 }
+
+/*
+ * Iterate over jsonb string values or elements, and pass them together with
+ * an iteration state to a specified JsonIterateAction.
+ */
+void *
+iterate_jsonb_values(Jsonb *jb, void *state, JsonIterateAction action)
+{
+	JsonbIterator		*it;
+	JsonbValue			v;
+	JsonbIteratorToken	type;
+
+	it = JsonbIteratorInit(>root);
+
+	while ((type = JsonbIteratorNext(, , false)) != WJB_DONE)
+	{
+		if ((type == WJB_VALUE || type == WJB_ELEM) && v.type == jbvString)
+		{
+			action(state, v.val.string.val, v.val.string.len);
+		}
+	}
+
+	return state;
+}
+
+/*
+ * Iterate over json string values or elements, and pass them together with an
+ * iteration state to a specified JsonIterateAction.
+ */
+void *
+iterate_json_values(text *json, void *action_state, JsonIterateAction action)
+{
+	JsonLexContext *lex = makeJsonLexContext(json, true);
+	JsonSemAction *sem = palloc0(sizeof(JsonSemAction));
+	IterateJsonState   *state = palloc0(sizeof(IterateJsonState));
+
+	state->lex = lex;
+	state->action = action;
+	state->action_state = action_state;
+
+	sem->semstate = (void *) state;
+	sem->scalar = apply_action;
+
+	pg_parse_json(lex, sem);
+
+	return state;
+}
+
+/*
+ * An auxiliary function for iterate_json_values to invoke a specified
+ * JsonIterateAction.
+ */
+static void
+apply_action(void *state, char *token, JsonTokenType tokentype)
+{
+	IterateJsonState   *_state = (IterateJsonState *) state;
+	if (tokentype == JSON_TOKEN_STRING)
+		(*_state->action) (_state->action_state, token, strlen(token));
+}
+
+/*
+ * Iterate over a jsonb, and apply a specified JsonTransformAction to every
+ * string value or element. Any necessary context for a JsonTransformAction can
+ * be passed in the action_state variable. Function returns a copy of an original jsonb
+ * object with transformed values.
+ */
+Jsonb *
+transform_jsonb(Jsonb *jsonb, void *action_state, JsonTransformAction transform_action)
+{
+	JsonbIterator		*it;
+	JsonbValue			v, *res = NULL;
+	JsonbIteratorToken	type;
+	JsonbParseState		*st = NULL;
+	text*out;
+	boolis_scalar = false;
+
+	it = JsonbIteratorInit(>root);
+	is_scalar = it->isScalar;
+
+	while ((type = JsonbIteratorNext(, , false)) != WJB_DONE)
+	{
+		if ((type == WJB_VALUE || type == WJB_ELEM) && v.type == jbvString)
+		{
+			out = transform_action(action_state, v.val.string.val, v.val.string.len);
+			v.val.string.val = 

Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

> On 21 March 2017 at 18:16, David Steele <da...@pgmasters.net> wrote:
>
> This thread has been idle for over a week.

Yes, sorry for the late reply. I'm still trying to find a better solution
for
some of the problems, that arose in this patch.

> On 15 March 2017 at 00:10, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Dmitry Dolgov <9erthali...@gmail.com> writes:
>
> I looked through this a bit.
>

Thank you for the feedback.

> > On 10 March 2017 at 06:20, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:
> > I see a possible problem here:  This design only allows one subscripting
> > function.  But what you'd really want in this case is at least two: one
> > taking an integer type for selecting by array index, and one taking text
> > for selecting by field name.
>
> I would guess that what we really want for jsonb is the ability to
> intermix integer and text subscripts, so that
>  jsonbcol['foo'][42]['bar']
> would extract the "bar" field of an object in position 42 of an
> array in field "foo" of the given jsonb value.
>

Maybe I misunderstood you, but isn't it already possible?

```
=# select ('{"a": [{"b": 1}, {"c": 2}]}'::jsonb)['a'][0]['b'];
 jsonb
---
 1
(1 row)

=# select * from test;
data
-
 {"a": [{"b": 1}, {"c": 2}]}
(1 row)

=# update test set data['a'][0]['b'] = 42;
UPDATE 1

=# select * from test;
data
-
 {"a": [{"b": 42}, {"c": 2}]}
(1 row)
```

> I'm not totally excited about the naming you've chosen though,
> particularly the function names "array_subscripting()" and
> "jsonb_subscripting()" --- those are too generic, and a person coming to
> them for the first time would probably expect that they actually execute
> subscripting, when they do no such thing.  Names like
> "array_subscript_parse()" might be better.  Likewise the name of the
> new pg_type column doesn't really convey what it does, though I suppose
> "typsubscriptparse" is too much of a mouthful.  "typsubparse" seems short
> enough but might be confusing too.

It looks quite tempting for me to replace the word "subscript" by an
abbreviation all over the patch. Then it will become something like
"typsbsparse" which is not that mouthful, but I'm not sure if it will be
easily
recognizable.

> I wonder also if we should try to provide some helper functions rather
> than expecting every data type to know all there is to know about parsing
> and execution of subscripting.  Not sure what would be helpful, however.

I don't really see what details we can hide behind this helper, because
almost
all code there is type specific (e.g. to check if subscript indexes are
correct), can you elaborate on that?

> The documentation needs a lot of work of course, and I do not think
> you're doing either yourself or anybody else any favors with the proposed
> additions to src/tutorial/.

Yes, unfortunately, I forget to update documentation from the previous
version
of the patch. I'll fix it soon in the next version.

> Aren't SBS_VALIDATION and SBS_EXEC just hangovers from the previous
> design?  They're still in parse_node.h, and they're still mentioned in
> the docs, but I don't see them used in actual code anywhere.

Yes, these are from the previous version too, I'll remove them.

> Another question here is whether every datatype will be on board
> with the current rules about null subscripts, or whether we need to
> delegate null-handling to the datatype-specific execution function.
> I'm not sure; it would complicate the API significantly for what might be
> useless flexibility.

It looks for me that it's a great idea to perform null-handling inside
datatype
specific code and I'm not sure, what would be complicated? All necessary
information for that is already in `SubscriptingExecData` (I just have to
use
`eisnull` in a different way).

> I do not think that the extra SubscriptingBase data structure is paying
> for itself either; you're taking a hit in readability from the extra level
> of struct, and as far as I can find it's not buying you one single thing,
> because there's no code that operates on a SubscriptingBase argument.
> I'd just drop that idea and make two independent struct types, or else
> stick with the original ArrayRef design that made one struct serve both
> purposes.  (IOW, my feeling that a separate assign struct would be a
> readability improvement isn't exactly getting borne out by what you've
> done here.  But maybe there's a better way to do that.)

I'm thinking to replace these structures by more meaningful ones, something
like:

```
typedef struct S

Re: [HACKERS] [PATCH] few fts functions for jsonb

[Dmitry Dolgov]

> On 28 February 2017 at 19:21, Oleg Bartunov  wrote:
> 1. add json support

I've added json support for all functions.

>  Its_headline  should returns the original json with highlighting

Yes, I see now. I don't think it's worth it to add a special option for that
purpose, so I've just changed the implementation to return the original
json(b).
diff --git a/src/backend/tsearch/to_tsany.c b/src/backend/tsearch/to_tsany.c
index 6e5de8f..8f7bcfe 100644
--- a/src/backend/tsearch/to_tsany.c
+++ b/src/backend/tsearch/to_tsany.c
@@ -16,6 +16,7 @@
 #include "tsearch/ts_cache.h"
 #include "tsearch/ts_utils.h"
 #include "utils/builtins.h"
+#include "utils/jsonb.h"
 
 
 typedef struct MorphOpaque
@@ -24,6 +25,14 @@ typedef struct MorphOpaque
 	int			qoperator;		/* query operator */
 } MorphOpaque;
 
+typedef struct TSVectorBuildState
+{
+	ParsedText	*prs;
+	TSVector	result;
+	Oid			cfgId;
+} TSVectorBuildState;
+
+static void add_to_tsvector(void *state, char *elem_value, int elem_len);
 
 Datum
 get_current_ts_config(PG_FUNCTION_ARGS)
@@ -256,6 +265,109 @@ to_tsvector(PG_FUNCTION_ARGS)
 		PointerGetDatum(in)));
 }
 
+Datum
+jsonb_to_tsvector(PG_FUNCTION_ARGS)
+{
+	Jsonb*jb = PG_GETARG_JSONB(0);
+	TSVectorBuildState	state;
+	ParsedText			*prs = (ParsedText *) palloc(sizeof(ParsedText));
+
+	prs->words = NULL;
+	state.result = NULL;
+	state.cfgId = getTSCurrentConfig(true);
+	state.prs = prs;
+
+	iterate_jsonb_values(jb, , (JsonIterateAction) add_to_tsvector);
+
+	PG_FREE_IF_COPY(jb, 1);
+
+	if (state.result == NULL)
+	{
+		/* There weren't any string elements in jsonb,
+		 * so wee need to return an empty vector */
+
+		if (prs->words != NULL)
+			pfree(prs->words);
+
+		state.result = palloc(CALCDATASIZE(0, 0));
+		SET_VARSIZE(state.result, CALCDATASIZE(0, 0));
+		state.result->size = 0;
+	}
+
+	PG_RETURN_TSVECTOR(state.result);
+}
+
+Datum
+json_to_tsvector(PG_FUNCTION_ARGS)
+{
+	text*json = PG_GETARG_TEXT_P(0);
+	TSVectorBuildState	state;
+	ParsedText			*prs = (ParsedText *) palloc(sizeof(ParsedText));
+
+	prs->words = NULL;
+	state.result = NULL;
+	state.cfgId = getTSCurrentConfig(true);
+	state.prs = prs;
+
+	iterate_json_values(json, , (JsonIterateAction) add_to_tsvector);
+
+	PG_FREE_IF_COPY(json, 1);
+	if (state.result == NULL)
+	{
+		/* There weren't any string elements in json,
+		 * so wee need to return an empty vector */
+
+		if (prs->words != NULL)
+			pfree(prs->words);
+
+		state.result = palloc(CALCDATASIZE(0, 0));
+		SET_VARSIZE(state.result, CALCDATASIZE(0, 0));
+		state.result->size = 0;
+	}
+
+	PG_RETURN_TSVECTOR(state.result);
+}
+
+/*
+ * Extend current TSVector from _state with a new one,
+ * build over a json(b) element.
+ */
+static void
+add_to_tsvector(void *_state, char *elem_value, int elem_len)
+{
+	TSVectorBuildState *state = (TSVectorBuildState *) _state;
+	ParsedText	*prs = state->prs;
+	TSVector	item_vector;
+	int			i;
+
+	prs->lenwords = elem_len / 6;
+	if (prs->lenwords == 0)
+		prs->lenwords = 2;
+
+	prs->words = (ParsedWord *) palloc(sizeof(ParsedWord) * prs->lenwords);
+	prs->curwords = 0;
+	prs->pos = 0;
+
+	parsetext(state->cfgId, prs, elem_value, elem_len);
+
+	if (prs->curwords)
+	{
+		if (state->result != NULL)
+		{
+			for (i = 0; i < prs->curwords; i++)
+prs->words[i].pos.pos = prs->words[i].pos.pos + TS_JUMP;
+
+			item_vector = make_tsvector(prs);
+
+			state->result = (TSVector) DirectFunctionCall2(tsvector_concat,
+	TSVectorGetDatum(state->result),
+	PointerGetDatum(item_vector));
+		}
+		else
+			state->result = make_tsvector(prs);
+	}
+}
+
 /*
  * to_tsquery
  */
diff --git a/src/backend/tsearch/wparser.c b/src/backend/tsearch/wparser.c
index 8ca1c62..b648996 100644
--- a/src/backend/tsearch/wparser.c
+++ b/src/backend/tsearch/wparser.c
@@ -21,6 +21,7 @@
 #include "tsearch/ts_utils.h"
 #include "utils/builtins.h"
 #include "utils/varlena.h"
+#include "utils/jsonb.h"
 
 
 /**sql-level interface**/
@@ -31,6 +32,19 @@ typedef struct
 	LexDescr   *list;
 } TSTokenTypeStorage;
 
+/* state for ts_headline_json_* */
+typedef struct HeadlineJsonState
+{
+	HeadlineParsedText *prs;
+	TSConfigCacheEntry *cfg;
+	TSParserCacheEntry *prsobj;
+	TSQueryquery;
+	List*prsoptions;
+	booltransformed;
+} HeadlineJsonState;
+
+static text * headline_json_value(void *_state, char *elem_value, int elem_len);
+
 static void
 tt_setup_firstcall(FuncCallContext *funcctx, Oid prsid)
 {
@@ -362,3 +376,177 @@ ts_headline_opt(PG_FUNCTION_ARGS)
 		PG_GETARG_DATUM(1),
 		PG_GETARG_DATUM(2)));
 }
+
+Datum
+ts_headline_jsonb_byid_opt(PG_FUNCTION_ARGS)
+{
+	Jsonb			*out, *jb = PG_GETARG_JSONB(1);
+	TSQuery			query = PG_GETARG_TSQUERY(2);
+	text			*opt = (PG_NARGS() > 3 && PG_GETARG_POINTER(3)) ? PG_GETARG_TEXT_P(3) : NULL;
+
+	HeadlineParsedText prs;
+	HeadlineJsonState *state = palloc0(sizeof(HeadlineJsonState));
+
+	memset(, 0, sizeof(HeadlineParsedText));
+	prs.lenwords = 32;
+	prs.words = 

[HACKERS] [PATCH] few fts functions for jsonb

[Dmitry Dolgov]

Hi all

I would like to propose patch with a set of new small functions for fts in
case of
jsonb data type:

* to_tsvector(config, jsonb) - make a tsvector from all string values and
  elements of jsonb object. To prevent the situation, when tsquery can find
a
  phrase consisting of lexemes from two different values/elements, this
  function will add an increment to position of each lexeme from every new
  value/element.

* ts_headline(config, jsonb, tsquery, options) - generate a headline
directly
  from jsonb object

Here are the examples how they work:

```
=# select to_tsvector('{"a": "aaa bbb", "b": ["ccc ddd"], "c": {"d": "eee
fff"}}'::jsonb);
   to_tsvector
-
 'aaa':1 'bbb':2 'ccc':4 'ddd':5 'eee':7 'fff':8
(1 row)


=# select ts_headline('english', '{"a": "aaa bbb", "b": {"c": "ccc
ddd"}}'::jsonb, tsquery('bbb & ddd & hhh'), 'StartSel = <, StopSel = >');
 ts_headline
--
 aaa  ccc 
(1 row)
```

Any comments or suggestions?
diff --git a/src/backend/tsearch/to_tsany.c b/src/backend/tsearch/to_tsany.c
index 6e5de8f..08e08e5 100644
--- a/src/backend/tsearch/to_tsany.c
+++ b/src/backend/tsearch/to_tsany.c
@@ -16,6 +16,8 @@
 #include "tsearch/ts_cache.h"
 #include "tsearch/ts_utils.h"
 #include "utils/builtins.h"
+#include "utils/jsonb.h"
+#include "utils/fmgrprotos.h"
 
 
 typedef struct MorphOpaque
@@ -256,6 +258,58 @@ to_tsvector(PG_FUNCTION_ARGS)
 		PointerGetDatum(in)));
 }
 
+Datum
+jsonb_to_tsvector(PG_FUNCTION_ARGS)
+{
+	Jsonb*jb = PG_GETARG_JSONB(0);
+	JsonbIterator		*it;
+	JsonbValue			v;
+	Oid	cfgId;
+	ParsedText			prs;
+	TSVector			result, item_vector;
+	JsonbIteratorToken	type;
+	int	i;
+
+	cfgId = getTSCurrentConfig(true);
+	it = JsonbIteratorInit(>root);
+
+	while ((type = JsonbIteratorNext(, , false)) != WJB_DONE)
+	{
+		if ((type == WJB_VALUE || type == WJB_ELEM) && v.type == jbvString)
+		{
+			prs.lenwords = v.val.string.len / 6;
+
+			if (prs.lenwords == 0)
+prs.lenwords = 2;
+
+			prs.curwords = 0;
+			prs.pos = 0;
+			prs.words = (ParsedWord *) palloc(sizeof(ParsedWord) * prs.lenwords);
+
+			parsetext(cfgId, , v.val.string.val, v.val.string.len);
+
+			if (prs.curwords)
+			{
+if (result->size != 0)
+{
+	for (i = 0; i < prs.curwords; i++)
+		prs.words[i].pos.pos = prs.words[i].pos.pos + TS_JUMP;
+
+	item_vector = make_tsvector();
+
+	result = DirectFunctionCall2(tsvector_concat,
+			TSVectorGetDatum(result),
+			PointerGetDatum(item_vector));
+}
+else
+	result = make_tsvector();
+			}
+		}
+	}
+
+	PG_RETURN_DATUM(result);
+}
+
 /*
  * to_tsquery
  */
diff --git a/src/backend/tsearch/wparser.c b/src/backend/tsearch/wparser.c
index 8ca1c62..035632e 100644
--- a/src/backend/tsearch/wparser.c
+++ b/src/backend/tsearch/wparser.c
@@ -21,6 +21,7 @@
 #include "tsearch/ts_utils.h"
 #include "utils/builtins.h"
 #include "utils/varlena.h"
+#include "utils/jsonb.h"
 
 
 /**sql-level interface**/
@@ -362,3 +363,41 @@ ts_headline_opt(PG_FUNCTION_ARGS)
 		PG_GETARG_DATUM(1),
 		PG_GETARG_DATUM(2)));
 }
+
+Datum
+ts_headline_jsonb(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_DATUM(DirectFunctionCall3(ts_headline_byid_opt,
+  ObjectIdGetDatum(getTSCurrentConfig(true)),
+		CStringGetTextDatum(jsonb_values_as_string(PG_GETARG_DATUM(0))),
+		PG_GETARG_DATUM(1)));
+}
+
+Datum
+ts_headline_jsonb_byid(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_DATUM(DirectFunctionCall3(ts_headline_byid_opt,
+		PG_GETARG_DATUM(0),
+		CStringGetTextDatum(jsonb_values_as_string(PG_GETARG_DATUM(1))),
+		PG_GETARG_DATUM(2)));
+}
+
+Datum
+ts_headline_jsonb_opt(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_DATUM(DirectFunctionCall4(ts_headline_byid_opt,
+  ObjectIdGetDatum(getTSCurrentConfig(true)),
+		CStringGetTextDatum(jsonb_values_as_string(PG_GETARG_DATUM(0))),
+		PG_GETARG_DATUM(1),
+		PG_GETARG_DATUM(2)));
+}
+
+Datum
+ts_headline_jsonb_byid_opt(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_DATUM(DirectFunctionCall4(ts_headline_byid_opt,
+		PG_GETARG_DATUM(0),
+		CStringGetTextDatum(jsonb_values_as_string(PG_GETARG_DATUM(1))),
+		PG_GETARG_DATUM(2),
+		PG_GETARG_DATUM(3)));
+}
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 6a7aab2..d504b87 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -4130,3 +4130,29 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 		}
 	}
 }
+
+/*
+ * Gather all string values and elements from jsonb into one string buffer.
+ * It's convenient for using inside ts_headline_* functions.
+ */
+char*
+jsonb_values_as_string(Jsonb *jb)
+{
+	JsonbIterator		*it;
+	JsonbValue			v;
+	JsonbIteratorToken	type;
+	StringInfo			buffer = makeStringInfo();
+
+	it = JsonbIteratorInit(>root);
+
+	while ((type = JsonbIteratorNext(, , false)) != WJB_DONE)
+	{
+		if 

Re: [HACKERS] [PATCH] Generic type subscription

[Dmitry Dolgov]

On 24 January 2017 at 02:36, Tom Lane  wrote:

> It might be possible to get away with having only one pg_type column,
> pointing at the parse-analysis function.  That function would generate
> a SubscriptingRef tree node containing the OID of the appropriate
> execution function, which execQual.c could call without ever knowing
> its name explicitly.

Btw, is it acceptable if such generated SubscriptingRef will contain just a
function pointer to the appropriate execution function instead of an OID
(e.g.
like `ExprStateEvalFunc`)? It will help to avoid problems in case of
extensions.

Re: [HACKERS] [PATCH] Generic type subscription

[Dmitry Dolgov]

> On 24 January 2017 at 02:07, Tom Lane  wrote:
> I took an extremely quick look over the patch

Thank you for the feedback. It took some time for me to think about all
suggestions and notes.

> 1. As I mentioned previously, it's a seriously bad idea that ArrayRef
> is used for both array subscripting and array assignment.  Let's fix
> that while we're at it, rather than setting that mistake in even more
> stone by embedding it in a datatype extension API.

Sure. But I assume that `SubscriptingRef` and `SubscriptingAssignmentRef`
will
be almost identical since they carry the same information to get a value
and to assign a new value (so, probably it will be just an alias with a
different related function).

> 2. I'm not very pleased that the subscripting functions have signature
> "subscripting(internal) returns internal";

Basically, current implementation of subscripting operation contains node
related logic (e.g. like to verify that we're not using slice syntax for
jsonb)
and data type related logic (e.g. to get/to assign a value in an array).
And if
it's easy enough to use:
`array_subscript(anyarray, internal) returns anyelement`
`array_assign(anyarray, internal, anyelement) returns anyarray`
form for the second one, the first one should accept node as an argument and
return node - I'm not sure if it's possible to use something else than
`internal` here. Speaking about other signature issues, sure, I'll fix them.

> 3. The contents of ArrayRef are designed on the assumption that the same
> typmod and collation values apply to both an array and its elements.

Yes, I missed that. It looks straightforward for me, we can just split
`refcollid` and `reftypmod` to `refcontainercollid`, `refelementcollid` and
`refcontainertypmod`, `refelementtypmod`.

> 4. It looks like your work on the node processing infrastructure has been
> limited to s/ArrayRef/SubscriptingRef/g, but that's not nearly enough.
> SubscriptingRef needs to be regarded as an opportunity to invoke a
> user-defined function, which means that it now acts quite a bit like
> FuncExpr.  For example, the function-to-be-invoked needs to be checked for
> volatility, leakproofness, parallel safety, etc in operations that want to
> know about such things.
> 
> I noted yesterday, you're missing a lot of plan-time manipulations that
need
> to happen for a generic function call.

Yes, I totally missed these too. I'm going to improve this situation soon.

> Actually, after thinking about that a bit more: you've really squeezed
> *three* different APIs into one function.  Namely, subscript-reference
> parse analysis, array subscripting execution, and array assignment
> execution.  It would be cleaner, and would reduce runtime overhead a bit,
> if those were embodied as three separate functions.
>
> It might be possible to get away with having only one pg_type column,
> pointing at the parse-analysis function.  That function would generate
> a SubscriptingRef tree node containing the OID of the appropriate
> execution function, which execQual.c could call without ever knowing
> its name explicitly.
>
> This clearly would work for built-in types, since the parse-analysis
> function could rely on fmgroids.h for the OIDs of the execution functions.
> But I'm less sure if it would work in extensions, which won't have
> predetermined OIDs for their functions.  Is there a way for a function
> in an extension to find the OID of one of its sibling functions?
>
> On 24 January 2017 at 07:54, Jim Nasby  wrote:
>
> Obviously there's regprocedure (or it's C equivalent), but then you're
stuck
> re-computing at runtime. I've messed around with that a bit in an effort
to
> have an extension depend on another extension that allows the user to
specify
> it's schema. If you're already doing metaprogramming it's not an enormous
> problem... if you're not already doing that it sucks. Trying to make that
> work in C would be somewhere between impossible and a nightmare.

The idea of having one function that will generate SubscriptingRef node
sounds
good. But I'm afraid of consequences about using it for extensions
(especially
since the request for general subscripting implementation came also from
their side). Is there a way to make it less troublesome?

To summarize, right now there are three options to handle a
`SubscriptingRef`
node analysis, subscripting execution and assignment execution:

* one `pg_type` column with an OID of corresponding function for each
purpose
  (which isn't cool)

* one "controller" function that will call directly another function with
  required logic (which is a "squeezing" and it's the current
implementation)

* one function that will return `SubscriptingRef` with an OID of required
  function (which is potentially troublesome for extensions)

> BTW, a different approach that might be worth considering is to say that
> the nodetree representation of one of these things *is* a FuncExpr, and

Re: [HACKERS] [PATCH] Generic type subscription

[Dmitry Dolgov]

> On 4 January 2017 at 18:06, Artur Zakirov 
wrote:
> But I'm not convinced about how to distinguish ArrayRef node with new
> SubscriptingRef node.

I'm not sure I understood you correctly. You're talking about having two
nodes
`ArrayRef` and `SubscriptingRef` at the same time for the sake of backward
compatibility, am I right? But they're basically the same, since
`SubscriptingRef` name is used just to indicate more general purpose of this
node.

> Also Tom pointed that he had bad experience with using ArrayRef node:

Yes, but it was related to the idea of having `ArrayRef` and `JsonbRef`
nodes
for specific types. Since now there is generic `SubscriptingRef` node, I
think
it should be ok.

>> Hm...I already answered, that I managed to avoid compilation problems for
>> this particular extension using the `genparser` command again:

> I suppose that a separate node type could solve it.

Just to be clear - as far as I understood, these compilation problems were
caused not because the extension knew something about ArrayRef node in
particular, but because the extension tried to extract all nodes to generate
code from them. It means any change will require "refetching", so I think
it's
natural for this extension.

Re: [HACKERS] [PATCH] Generic type subscription

[Dmitry Dolgov]

> On 27 December 2016 at 16:09, Aleksander Alekseev <
a.aleks...@postgrespro.ru> wrote:
> until it breaks existing extensions.

Hm...I already answered, that I managed to avoid compilation problems for
this particular extension
using the `genparser` command again:

> On Thu, Nov 17, 2016 at 10:56 PM, Dmitry Dolgov
<9erthalion6(at)gmail(dot)com>
> wrote:
>
> > On 15 November 2016 at 15:03, Aleksander Alekseev <
> a(dot)alekseev(at)postgrespro(dot)ru> wrote:
> > Hello.
> >
> > I took a look on the latest -v4 patch. I would like to note that this
> > patch breaks a backward compatibility. For instance sr_plan extension[1]
> > stop to compile with errors
>
> Thank you for the feedback.
>
> Well, if we're speaking about this particular extension, if I understood
> correctly, it fetches all parse tree nodes from Postgres and generates
code
> using this information. So to avoid compilation problems I believe you
need to
> run `make USE_PGXS=1 genparser` again (it worked for me, I don't see any
> mentions of `ArrayRef`).
>
> But speaking generally, I don't see how we can provide backward
> compatibility for those extensions, who are strongly coupled with
implementation details
> of parsing tree. I mean, in terms of interface it's mostly about to
replace
> `ArrayRef` to `SubscriptingRef`, but I think it's better to do it in the
extension code.

Or is there something else that I missed?

Re: [HACKERS] [PATCH] Generic type subscription

[Dmitry Dolgov]

> On 5 December 2016 at 12:03, Haribabu Kommi 
 wrote:

> > Moved to next CF with "needs review" status.

Looks like we stuck here little bit. Does anyone else have any
suggestions/improvements, or this patch is in good enough shape?

Re: [HACKERS] jsonb_delete with arrays

[Dmitry Dolgov]

> Attached is an implantation of jsonb_delete that instead of taking a
single key to remove accepts an array of keys

Since I already saw this patch, here is my small review.

Speaking about implementation of `jsonb_delete_array` - it's fine, but I
would like to suggest two modifications:

* create a separate helper function for jsonb delete operation, to use it
in both `jsonb_delete` and `jsonb_delete_array`. It will help to
concentrate related logic in one place.

* use variadic arguments for `jsonb_delete_array`. For rare cases, when
someone decides to use this function directly instead of corresponding
operator. It will be more consistent with `jsonb_delete` from my point of
view, because it's transition from `jsonb_delete(data, 'key')` to
`jsonb_delete(data, 'key1', 'key2')` is more smooth, than to
`jsonb_delete(data, '{key1, key2}')`.

I've attached a patch with these modifications. What do you think?
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 17ee4e4..aa8156e 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -149,6 +149,11 @@ static void setPathArray(JsonbIterator **it, Datum *path_elems,
 			 int level, Jsonb *newval, uint32 nelems, int op_type);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
+/* common workers for jsonb_delete functions */
+static Datum jsonb_delete_worker(Jsonb *in, Datum *elems,
+ bool *nulls, int len);
+
+
 /* state for json_object_keys */
 typedef struct OkeysState
 {
@@ -3395,14 +3400,8 @@ jsonb_delete(PG_FUNCTION_ARGS)
 {
 	Jsonb	   *in = PG_GETARG_JSONB(0);
 	text	   *key = PG_GETARG_TEXT_PP(1);
-	char	   *keyptr = VARDATA_ANY(key);
-	int			keylen = VARSIZE_ANY_EXHDR(key);
-	JsonbParseState *state = NULL;
-	JsonbIterator *it;
-	JsonbValue	v,
-			   *res = NULL;
-	bool		skipNested = false;
-	JsonbIteratorToken r;
+	Datum	   *keys = (Datum *) palloc(sizeof(Datum));
+	bool	   *nulls = (bool *) palloc(sizeof(bool));
 
 	if (JB_ROOT_IS_SCALAR(in))
 		ereport(ERROR,
@@ -3412,21 +3411,95 @@ jsonb_delete(PG_FUNCTION_ARGS)
 	if (JB_ROOT_COUNT(in) == 0)
 		PG_RETURN_JSONB(in);
 
+	keys[0] = PointerGetDatum(key);
+	nulls[0] = FALSE;
+
+	return jsonb_delete_worker(in, keys, nulls, 1);
+}
+
+/*
+ * SQL function jsonb_delete (jsonb, text[])
+ *
+ * return a copy of the jsonb with the indicated items
+ * removed.
+ */
+Datum
+jsonb_delete_array(PG_FUNCTION_ARGS)
+{
+	Jsonb	   *in = PG_GETARG_JSONB(0);
+	ArrayType  *keys = PG_GETARG_ARRAYTYPE_P(1);
+	Datum	   *keys_elems;
+	bool	   *keys_nulls;
+	int			keys_len;
+
+	if (ARR_NDIM(keys) > 1)
+		ereport(ERROR,
+(errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),
+ errmsg("wrong number of array subscripts")));
+
+	if (JB_ROOT_IS_SCALAR(in))
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot delete from scalar")));
+
+	if (JB_ROOT_COUNT(in) == 0)
+		PG_RETURN_JSONB(in);
+
+	deconstruct_array(keys, TEXTOID, -1, false, 'i',
+	  _elems, _nulls, _len);
+
+	return jsonb_delete_worker(in, keys_elems, keys_nulls, keys_len);
+}
+
+Datum
+jsonb_delete_worker(Jsonb *in, Datum *elems, bool *nulls, int len)
+{
+	JsonbParseState *state = NULL;
+	JsonbIterator *it;
+	JsonbValue	v,
+			   *res = NULL;
+	bool		skipNested = false;
+	JsonbIteratorToken r;
+
+	if (len == 0)
+		PG_RETURN_JSONB(in);
+
 	it = JsonbIteratorInit(>root);
 
 	while ((r = JsonbIteratorNext(, , skipNested)) != 0)
 	{
 		skipNested = true;
 
-		if ((r == WJB_ELEM || r == WJB_KEY) &&
-			(v.type == jbvString && keylen == v.val.string.len &&
-			 memcmp(keyptr, v.val.string.val, keylen) == 0))
+		if ((r == WJB_ELEM || r == WJB_KEY) && v.type == jbvString)
 		{
-			/* skip corresponding value as well */
-			if (r == WJB_KEY)
-JsonbIteratorNext(, , true);
+			int			i;
+			bool		found = false;
 
-			continue;
+			for (i = 0; i < len; i++)
+			{
+char	   *keyptr;
+int			keylen;
+
+if (nulls[i])
+	continue;
+
+keyptr = VARDATA_ANY(elems[i]);
+keylen = VARSIZE_ANY_EXHDR(elems[i]);
+if (keylen == v.val.string.len &&
+	memcmp(keyptr, v.val.string.val, keylen) == 0)
+{
+	found = true;
+	break;
+}
+			}
+			if (found)
+			{
+/* skip corresponding value as well */
+if (r == WJB_KEY)
+	JsonbIteratorNext(, , true);
+
+continue;
+			}
 		}
 
 		res = pushJsonbValue(, r, r < WJB_BEGIN_ARRAY ?  : NULL);
diff --git a/src/include/catalog/pg_operator.h b/src/include/catalog/pg_operator.h
index 26fa618..347b2e1 100644
--- a/src/include/catalog/pg_operator.h
+++ b/src/include/catalog/pg_operator.h
@@ -1820,6 +1820,8 @@ DATA(insert OID = 3284 (  "||"	   PGNSP PGUID b f f 3802 3802 3802 0 0 jsonb_con
 DESCR("concatenate");
 DATA(insert OID = 3285 (  "-"	   PGNSP PGUID b f f 3802 25 3802 0 0 3302 - - ));
 DESCR("delete object field");
+DATA(insert OID = 3344 (  "-"  PGNSP PGUID b f f 3802 1009 3802 0 0 3343 - -));
+DESCR("delete object fields");
 DATA(insert OID = 3286 (  "-"	   PGNSP PGUID 

Re: [HACKERS] jsonb_delete with arrays

[Dmitry Dolgov]

> On 15 November 2016 at 22:53, Magnus Hagander  wrote:
> Attached is an implantation of jsonb_delete that instead of taking a
single key to remove accepts an array of keys (it still does just keys, so
it's using the - operator, it's not like the path delete function that also
takes an array, but uses a different operator).
>
> In some simple testing of working through a real world usecases where we
needed to delete 7 keys from jsonb data, it shows approximately a 9x
speedup over calling the - operator multiple times. I'm guessing since we
copy a lot less and don't have to re-traverse the structure.

I wonder, is it worth it to create some sort of helper function to handle
both
deleting one key and deleting an array of keys (like `setPath` for
`jsonb_set`,
`jsonb_insert` and `jsonb_delete_path`)? At first glance it looks like
`jsonb_delete` and `jsonb_delete_array` can reuse some code.

Speaking about the performance I believe it's the same problem as here [1],
since for each key the full jsonb will be decompressed. Looks like we need a
new set of functions to read/update/delete an array of elements at once.

[1]:
https://www.postgresql.org/message-id/flat/1566eab8731.10193ac585742.5467876610052746443%40zohocorp.com

Re: [HACKERS] [PATCH] Generic type subscription

[Dmitry Dolgov]

> On 15 November 2016 at 15:03, Aleksander Alekseev <
a.aleks...@postgrespro.ru> wrote:
> Hello.
>
> I took a look on the latest -v4 patch. I would like to note that this
> patch breaks a backward compatibility. For instance sr_plan extension[1]
> stop to compile with errors

Thank you for the feedback.

Well, if we're speaking about this particular extension, if I understood
correctly, it fetches all parse tree nodes from Postgres and generates code
using this information. So to avoid compilation problems I believe you need
to
run `make USE_PGXS=1 genparser` again (it worked for me, I don't see any
mentions of `ArrayRef`).

But speaking generally, I don't see how we can provide backward
compatibility
for those extensions, who are strongly coupled with implementation details
of
parsing tree. I mean, in terms of interface it's mostly about to replace
`ArrayRef` to `SubscriptingRef`, but I think it's better to do it in the
extension code.

Re: [HACKERS] [PATCH] Generic type subscription

[Dmitry Dolgov]

On 5 October 2016 at 03:00, Oleg Bartunov  wrote:

>
> have you ever run 'make check' ?
>
> =
>  53 of 168 tests failed.
> =
>
>
Sure, how else could I write tests for this feature? But right now on my
machine
everything is ok (the same for `make installcheck`):

$ make check

===
 All 168 tests passed.
===

Re: [HACKERS] [Patch] Temporary tables that do not bloat pg_catalog (a.k.a fast temp tables)

[Dmitry Dolgov]

​Hi

I tried to dig into this patch (which seems pretty interesting) to help
bring
it in good shape. Here are few random notes, I hope they can be helpful:

> I think we actually need a real API here.
Definitely, there are plenty places in the new code with the same pattern:
 * figure out if it's an action related to the fast temporary tables based
on
   `ItemPointer`/relation OID/etc
 * if it is, add some extra logic or skip something in original
implementation

in `heapam.c`, `indexam.c`, `xact.c`, `dependency.c`. I believe it's
possible to
make it more generic (although it will contain almost the same logic), e.g.
separate regular and fasttable implementations completely, and decide which
one
we should choose in that particular case.

Btw, I'm wondering about the `heap_*` functions in `heapam.c` - some of
them are
wrapped and never used directly, although they're contains in
`INTERFACE_ROUTINES` (like `simple_heap_delete` -> `heap_delete`), some of
them
aren't. It looks like inconsistency in terms of function names, probably it
should be unified?

> What is needed is an overview of the approach, so that the reviewers can
read
> that first,
I feel lack of such information even in new version of this patch (but, I'm
probably a noob in these matters).  I noted that the `fasttab.c` file
contains some
general overview, but in terms of "what are we doing", and "why are we doing
this". I think general overview of "how are we doing this" also may be
useful.
And there are several slightly obvious commentaries like:

```
+ /* Is it a virtual TID? */
+ if (IsFasttabItemPointer(tid))
```

but I believe an introduction of a new API (see the previous note) will
solve
this eventually.

> Why do we need the new relpersistence value? ISTM we could easily got with
> just RELPERSISTENCE_TEMP, which would got right away of many chances as
the
> steps are exactly the same.
I agree, it looks like `RELPERSISTENCE_FAST_TEMP` hasn't any influence on
the
code.

> For example, suppose I create a fast temporary table and then I create a
> functional index on the fast temporary table that uses some SQL function
> defined in pg_proc.
Just to clarify, did you mean something like this?
```
create fast temp table fasttab(x int, s text);
create or replace function test_function_for_index(t text) returns text as
$$
begin
return lower(t);
end;
$$ language plpgsql immutable;
create index fasttab_s_idx on fasttab (test_function_for_index(s));
drop function test_function_for_index(t text);
```
As far as I understand dependencies should protect in case of fasttable too,
because everything is visible as in regular case, isn't it?

And finally one more question, why items of `FasttabIndexMethodsTable[]`
like
this one:
```
+ /* 2187, non-unique */
+ {InheritsParentIndexId, 1,
+ {Anum_pg_inherits_inhparent, 0, 0},
+ {CompareOid, CompareInvalid, CompareInvalid}
+ },
```
have this commentary before them? I assume it's an id and an extra
information,
and I'm concerned that they can easily become outdated inside commentary
block.

[HACKERS] Jsonb array-style subscripting, generic version

[Dmitry Dolgov]

Hi

With regard to previous conversations:

http://www.postgresql.org/message-id/flat/CA+q6zcV8qvGcDXurwwgUbwACV86Th7G80pnubg42e-p9gsSf=g...@mail.gmail.com#CA+q6zcV8qvGcDXurwwgUbwACV86Th7G80pnubg42e-p9gsSf=g...@mail.gmail.com

http://www.postgresql.org/message-id/flat/ca+q6zcx3mdxgcgdthzuyswh-apyhhm-g4ob1r0fn0j2hzqq...@mail.gmail.com#ca+q6zcx3mdxgcgdthzuyswh-apyhhm-g4ob1r0fn0j2hzqq...@mail.gmail.com

I want to try following approach to make the array-style subscripting more
generic and allow using it for different types with less effort:

* Introduce generic node type instead of the `ArrayRef` (and `JsonbRef` from
  patch) [SubscriptingRef]
* Make generic version of `transformArraySubscript` /
  `transformAssignmentSubscript` / `ExecEvalArrayRef` etc
  [transformSubscripting / ExecEvalSubscripting etc]
* Introduce a new pg_type column with type `regproc` to point out a
function to
  handle all type-related logic for generic `ExecEval` function
  [typsubscripting]. If value of this column is null, type doesn't support
  array-style subscripting

There is still question about indexing of such kind of expressions. To be
honest I haven't figured it out in details yet how to do it (except simple
btree index for an each path like `create index jsonb_data_idx on
jsonb_table ((jsonb_data['key']))`).
But I believe that this can be achieved subsequently, since in case of
getting
data using the array-style subscripting it's no more than alias or syntactic
sugar.

So I have few questions:

* Is it whole plan looks ok?
* Any suggestions about names (especially for column in pg_type)?
* Is it ok to implement indexing separately (since the main purpose of
  array-style subscripting for jsonb is an update operation)?

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

On 6 April 2016 at 03:29, Andrew Dunstan  wrote:

>
> Yeah, keeping it but rejecting update of an existing key is my preference
> too.
>
> cheers
>
> andrew
>

Yes, it sounds quite reasonable. Here is a new version of patch (it will
throw
an error for an existing key). Is it better now?
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 1bc9fbc..0d4b3a1 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10903,6 +10903,9 @@ table2-mapping
jsonb_set
   
   
+   jsonb_insert
+  
+  
jsonb_pretty
   
 
@@ -11184,6 +11187,39 @@ table2-mapping
 

   
+   
+   
+   jsonb_insert(target jsonb, path text[], new_value jsonb, insert_after boolean)
+   
+   
+   jsonb
+   
+ Returns target with
+ new_value inserted.  If
+ target section designated by
+ path is in a JSONB array,
+ new_value will be inserted before target or
+ after if insert_after is true (default is
+ false). If target section
+ designated by path is in JSONB object,
+ new_value will be inserted only if
+ target does not exist. As with the path
+ orientated operators, negative integers that appear in
+ path count from the end of JSON arrays.
+   
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"')
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"', true)
+   
+   
+   {"a": [0, "new_value", 1, 2]}
+ {"a": [0, 1, "new_value", 2]}
+
+   
+  
jsonb_pretty(from_json jsonb)
  
text
@@ -11235,10 +11271,11 @@ table2-mapping
   
 
   All the items of the path parameter of jsonb_set
-  must be present in the target, unless
-  create_missing is true, in which case all but the last item
-  must be present. If these conditions are not met the target
-  is returned unchanged.
+  as well as jsonb_insert except the last item must be present
+  in the target. If create_missing is false, all
+  items of the path parameter of jsonb_set must be
+  present. If these conditions are not met the target is
+  returned unchanged.
 
 
   If the last path item is an object key, it will be created if it
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 9ae1ef4..a6e661c 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -997,3 +997,11 @@ RETURNS text[]
 LANGUAGE INTERNAL
 STRICT IMMUTABLE
 AS 'parse_ident';
+
+CREATE OR REPLACE FUNCTION
+  jsonb_insert(jsonb_in jsonb, path text[] , replacement jsonb,
+insert_after boolean DEFAULT false)
+RETURNS jsonb
+LANGUAGE INTERNAL
+STRICT IMMUTABLE
+AS 'jsonb_insert';
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 97e0e8e..c1b8041 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -33,6 +33,15 @@
 #include "utils/memutils.h"
 #include "utils/typcache.h"
 
+/* Operations available for setPath */
+#define JB_PATH_NOOP	0x
+#define JB_PATH_CREATE	0x0001
+#define JB_PATH_DELETE	0x0002
+#define JB_PATH_INSERT_BEFORE			0x0004
+#define JB_PATH_INSERT_AFTER			0x0008
+#define JB_PATH_CREATE_OR_INSERT (JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER \
+		| JB_PATH_CREATE)
+
 /* semantic action functions for json_object_keys */
 static void okeys_object_field_start(void *state, char *fname, bool isnull);
 static void okeys_array_start(void *state);
@@ -130,14 +139,14 @@ static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
 static JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
 		bool *path_nulls, int path_len,
 		JsonbParseState **st, int level, Jsonb *newval,
-		bool create);
+		int op_type);
 static void setPathObject(JsonbIterator **it, Datum *path_elems,
 			  bool *path_nulls, int path_len, JsonbParseState **st,
 			  int level,
-			  Jsonb *newval, uint32 npairs, bool create);
+			  Jsonb *newval, uint32 npairs, int op_type);
 static void setPathArray(JsonbIterator **it, Datum *path_elems,
 			 bool *path_nulls, int path_len, JsonbParseState **st,
-			 int level, Jsonb *newval, uint32 nelems, bool create);
+			 int level, Jsonb *newval, uint32 nelems, int op_type);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
 /* state for json_object_keys */
@@ -3544,7 +3553,7 @@ jsonb_set(PG_FUNCTION_ARGS)
 	it = JsonbIteratorInit(>root);
 
 	res = setPath(, path_elems, path_nulls, path_len, ,
-  0, newval, create);
+  0, newval, create ? JB_PATH_CREATE : JB_PATH_NOOP);
 
 	Assert(res != NULL);
 
@@ -3588,7 +3597,52 @@ jsonb_delete_path(PG_FUNCTION_ARGS)
 
 	it = JsonbIteratorInit(>root);
 
-	res = setPath(, path_elems, path_nulls, path_len, , 0, NULL, false);
+	res = setPath(, 

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

On 31 March 2016 at 17:31, Vitaly Burovoy  wrote:

> it is logical to insert new value if "before", then current value, then new
> value if "after".
>

Oh, I see now. There is a slightly different logic: `v` is a current value
and `newval` is a new value.
So basically we insert a current item in case of "after", then a new value
(if it's not a delete operation),
then a current item in case of "before". But I agree, this code can be more
straightforward. I've attached
a new version, pls take a look (it contains the same logic that you've
mentioned).
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 1bc9fbc..8fb2624 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10903,6 +10903,9 @@ table2-mapping
jsonb_set
   
   
+   jsonb_insert
+  
+  
jsonb_pretty
   
 
@@ -11184,6 +11187,40 @@ table2-mapping
 

   
+   
+   
+   jsonb_insert(target jsonb, path text[], new_value jsonb, insert_after boolean)
+   
+   
+   jsonb
+   
+ Returns target with
+ new_value inserted.
+ If target section designated by
+ path is in a JSONB array,
+ new_value will be inserted before target or
+ after if insert_after is true (default is
+ false). If target section
+ designated by path is in JSONB object,
+ jsonb_insert behaves as
+ jsonb_set function where
+ create_missing is true. As with the
+ path orientated operators, negative integers that appear in
+ path count from the end of JSON arrays.
+   
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"')
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"', true)
+   
+   
+   {"a": [0, "new_value", 1, 2]}
+ {"a": [0, 1, "new_value", 2]}
+
+   
+  
jsonb_pretty(from_json jsonb)
  
text
@@ -11235,10 +11272,11 @@ table2-mapping
   
 
   All the items of the path parameter of jsonb_set
-  must be present in the target, unless
-  create_missing is true, in which case all but the last item
-  must be present. If these conditions are not met the target
-  is returned unchanged.
+  as well as jsonb_insert except the last item must be present
+  in the target. If create_missing is false, all
+  items of the path parameter of jsonb_set must be
+  present. If these conditions are not met the target is
+  returned unchanged.
 
 
   If the last path item is an object key, it will be created if it
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 9ae1ef4..a6e661c 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -997,3 +997,11 @@ RETURNS text[]
 LANGUAGE INTERNAL
 STRICT IMMUTABLE
 AS 'parse_ident';
+
+CREATE OR REPLACE FUNCTION
+  jsonb_insert(jsonb_in jsonb, path text[] , replacement jsonb,
+insert_after boolean DEFAULT false)
+RETURNS jsonb
+LANGUAGE INTERNAL
+STRICT IMMUTABLE
+AS 'jsonb_insert';
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 97e0e8e..97bb08e 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -33,6 +33,15 @@
 #include "utils/memutils.h"
 #include "utils/typcache.h"
 
+/* Operations available for setPath */
+#define JB_PATH_NOOP	0x
+#define JB_PATH_CREATE	0x0001
+#define JB_PATH_DELETE	0x0002
+#define JB_PATH_INSERT_BEFORE			0x0004
+#define JB_PATH_INSERT_AFTER			0x0008
+#define JB_PATH_CREATE_OR_INSERT (JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER \
+		| JB_PATH_CREATE)
+
 /* semantic action functions for json_object_keys */
 static void okeys_object_field_start(void *state, char *fname, bool isnull);
 static void okeys_array_start(void *state);
@@ -130,14 +139,14 @@ static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
 static JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
 		bool *path_nulls, int path_len,
 		JsonbParseState **st, int level, Jsonb *newval,
-		bool create);
+		int op_type);
 static void setPathObject(JsonbIterator **it, Datum *path_elems,
 			  bool *path_nulls, int path_len, JsonbParseState **st,
 			  int level,
-			  Jsonb *newval, uint32 npairs, bool create);
+			  Jsonb *newval, uint32 npairs, int op_type);
 static void setPathArray(JsonbIterator **it, Datum *path_elems,
 			 bool *path_nulls, int path_len, JsonbParseState **st,
-			 int level, Jsonb *newval, uint32 nelems, bool create);
+			 int level, Jsonb *newval, uint32 nelems, int op_type);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
 /* state for json_object_keys */
@@ -3544,7 +3553,7 @@ jsonb_set(PG_FUNCTION_ARGS)
 	it = JsonbIteratorInit(>root);
 
 	res = setPath(, path_elems, path_nulls, path_len, ,
-	

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

On 31 March 2016 at 05:04, Vitaly Burovoy  wrote:

> The documentation changes still has to be fixed.
>

Thanks for help. Looks like I'm not so good at text formulation. Fixed.


> Moreover it seems the logic in the code is correct


No - I see now, that I made the same mistake in two places (e.g. in case of
`setPathArray` a current item was pushed to parse state after
`JB_PATH_INSERT_BEFORE`, not a new one), so they were annihilated. Fixed.
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index ae93e69..3c3ff7a 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10904,6 +10904,9 @@ table2-mapping
jsonb_set
   
   
+   jsonb_insert
+  
+  
jsonb_pretty
   
 
@@ -11185,6 +11188,40 @@ table2-mapping
 

   
+   
+   
+   jsonb_insert(target jsonb, path text[], new_value jsonb, insert_after boolean)
+   
+   
+   jsonb
+   
+ Returns target with
+ new_value inserted.
+ If target section designated by
+ path is in a JSONB array,
+ new_value will be inserted before target or
+ after if insert_after is true (default is
+ false). If target section
+ designated by path is in JSONB object,
+ jsonb_insert behaves as
+ jsonb_set function where
+ create_missing is true. As with the
+ path orientated operators, negative integers that appear in
+ path count from the end of JSON arrays.
+   
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"')
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"', true)
+   
+   
+   {"a": [0, "new_value", 1, 2]}
+ {"a": [0, 1, "new_value", 2]}
+
+   
+  
jsonb_pretty(from_json jsonb)
  
text
@@ -11236,10 +11273,11 @@ table2-mapping
   
 
   All the items of the path parameter of jsonb_set
-  must be present in the target, unless
-  create_missing is true, in which case all but the last item
-  must be present. If these conditions are not met the target
-  is returned unchanged.
+  as well as jsonb_insert except the last item must be present
+  in the target. If create_missing is false, all
+  items of the path parameter of jsonb_set must be
+  present. If these conditions are not met the target is
+  returned unchanged.
 
 
   If the last path item is an object key, it will be created if it
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 9ae1ef4..a6e661c 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -997,3 +997,11 @@ RETURNS text[]
 LANGUAGE INTERNAL
 STRICT IMMUTABLE
 AS 'parse_ident';
+
+CREATE OR REPLACE FUNCTION
+  jsonb_insert(jsonb_in jsonb, path text[] , replacement jsonb,
+insert_after boolean DEFAULT false)
+RETURNS jsonb
+LANGUAGE INTERNAL
+STRICT IMMUTABLE
+AS 'jsonb_insert';
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 97e0e8e..df9fa5d 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -33,6 +33,15 @@
 #include "utils/memutils.h"
 #include "utils/typcache.h"
 
+/* Operations available for setPath */
+#define JB_PATH_NOOP	0x
+#define JB_PATH_CREATE	0x0001
+#define JB_PATH_DELETE	0x0002
+#define JB_PATH_INSERT_BEFORE			0x0004
+#define JB_PATH_INSERT_AFTER			0x0008
+#define JB_PATH_CREATE_OR_INSERT (JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER \
+		| JB_PATH_CREATE)
+
 /* semantic action functions for json_object_keys */
 static void okeys_object_field_start(void *state, char *fname, bool isnull);
 static void okeys_array_start(void *state);
@@ -130,14 +139,14 @@ static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
 static JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
 		bool *path_nulls, int path_len,
 		JsonbParseState **st, int level, Jsonb *newval,
-		bool create);
+		int op_type);
 static void setPathObject(JsonbIterator **it, Datum *path_elems,
 			  bool *path_nulls, int path_len, JsonbParseState **st,
 			  int level,
-			  Jsonb *newval, uint32 npairs, bool create);
+			  Jsonb *newval, uint32 npairs, int op_type);
 static void setPathArray(JsonbIterator **it, Datum *path_elems,
 			 bool *path_nulls, int path_len, JsonbParseState **st,
-			 int level, Jsonb *newval, uint32 nelems, bool create);
+			 int level, Jsonb *newval, uint32 nelems, int op_type);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
 /* state for json_object_keys */
@@ -3544,7 +3553,7 @@ jsonb_set(PG_FUNCTION_ARGS)
 	it = JsonbIteratorInit(>root);
 
 	res = setPath(, path_elems, path_nulls, path_len, ,
-  0, newval, create);
+  0, newval, create ? JB_PATH_CREATE : JB_PATH_NOOP);
 
 	Assert(res != NULL);

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

Here is a new version of path, I hope I didn't miss anything. Few notes:

> 4.
> or even create a new constant (there can be better name for it):
> #define JB_PATH_CREATE_OR_INSERT (JB_PATH_INSERT_BEFORE |
> JB_PATH_INSERT_AFTER | JB_PATH_CREATE)

Good idea, thanks.

> 5.
> > if (op_type != JB_PATH_DELETE)

Yes, I just missed that in previous patch.

> 7.
> Please, return the "skip" comment.

Well, I'm still not so sure about that, but if you're so confident then ok
=)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index ae93e69..dfed589 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10904,6 +10904,9 @@ table2-mapping
jsonb_set
   
   
+   jsonb_insert
+  
+  
jsonb_pretty
   
 
@@ -11185,6 +11188,40 @@ table2-mapping
 

   
+   
+   
+   jsonb_insert(target jsonb, path text[], new_value jsonb, after boolean)
+   
+   
+   jsonb
+   
+ Returns target with
+ new_value inserted.
+ If target section designated by
+ path is a JSONB array,
+ new_value will be inserted before target or
+ after if after is true (default is
+ false). If target section
+ designated by path is a JSONB object,
+ jsonb_insert will act like
+ jsonb_set function where
+ create_missing is true. As with the
+ path orientated operators, negative integers that appear in
+ path count from the end of JSON arrays.
+   
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"')
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"', true)
+   
+   
+   {"a": [0, "new_value", 1, 2]}
+ {"a": [0, 1, "new_value", 2]}
+
+   
+  
jsonb_pretty(from_json jsonb)
  
text
@@ -11236,10 +11273,11 @@ table2-mapping
   
 
   All the items of the path parameter of jsonb_set
-  must be present in the target, unless
-  create_missing is true, in which case all but the last item
-  must be present. If these conditions are not met the target
-  is returned unchanged.
+  as well as jsonb_insert except the last item must be present
+  in the target. If create_missing is false, all
+  items of the path parameter of jsonb_set must be
+  present. If these conditions are not met the target is
+  returned unchanged.
 
 
   If the last path item is an object key, it will be created if it
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 9ae1ef4..58ed3c3 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -997,3 +997,11 @@ RETURNS text[]
 LANGUAGE INTERNAL
 STRICT IMMUTABLE
 AS 'parse_ident';
+
+CREATE OR REPLACE FUNCTION
+  jsonb_insert(jsonb_in jsonb, path text[] , replacement jsonb,
+insert_before_after boolean DEFAULT false)
+RETURNS jsonb
+LANGUAGE INTERNAL
+STRICT IMMUTABLE
+AS 'jsonb_insert';
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 97e0e8e..5bd2165 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -33,6 +33,15 @@
 #include "utils/memutils.h"
 #include "utils/typcache.h"
 
+/* Operations available for setPath */
+#define JB_PATH_NOOP	0x0
+#define JB_PATH_CREATE	0x0001
+#define JB_PATH_DELETE	0x0002
+#define JB_PATH_INSERT_BEFORE			0x0004
+#define JB_PATH_INSERT_AFTER			0x0008
+#define JB_PATH_CREATE_OR_INSERT (JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER \
+		| JB_PATH_CREATE)
+
 /* semantic action functions for json_object_keys */
 static void okeys_object_field_start(void *state, char *fname, bool isnull);
 static void okeys_array_start(void *state);
@@ -130,14 +139,14 @@ static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
 static JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
 		bool *path_nulls, int path_len,
 		JsonbParseState **st, int level, Jsonb *newval,
-		bool create);
+		int op_type);
 static void setPathObject(JsonbIterator **it, Datum *path_elems,
 			  bool *path_nulls, int path_len, JsonbParseState **st,
 			  int level,
-			  Jsonb *newval, uint32 npairs, bool create);
+			  Jsonb *newval, uint32 npairs, int op_type);
 static void setPathArray(JsonbIterator **it, Datum *path_elems,
 			 bool *path_nulls, int path_len, JsonbParseState **st,
-			 int level, Jsonb *newval, uint32 nelems, bool create);
+			 int level, Jsonb *newval, uint32 nelems, int op_type);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
 /* state for json_object_keys */
@@ -3544,7 +3553,7 @@ jsonb_set(PG_FUNCTION_ARGS)
 	it = JsonbIteratorInit(>root);
 
 	res = setPath(, path_elems, path_nulls, path_len, ,
-  0, newval, create);
+  0, newval, create ? JB_PATH_CREATE : JB_PATH_NOOP);
 
 	Assert(res != NULL);
 
@@ 

Re: [HACKERS] jsonb array-style subscription

[Dmitry Dolgov]

> Do you have an updated patch ready?

No, I'm afraid it will not be ready for Monday.

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

> I still don't like that this works on path leading to an object given
that we can't fulfill the promise of inserting to an arbitrary position
there.

I'm thinking about this following way - `jsonb_insert` is just a function
to insert a new value, which has specific options to enforce arbitrary
position in case of jsonb array. I don't think this can confuse someone
since it's not something like `jsonb_insert_at_position` function. But of
course, if I'm wrong we can easy change the function name and make it
available only for arrays.

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

Hi Vitaly, thanks for the review. I've attached a new version of path with
improvements. Few notes:

> 7. Why did you remove "skip"? It is a comment what "true" means...

Actually, I thought that this comment was about skipping an element from
jsonb in order to change/delete it,
not about the last argument.  E.g. you can find several occurrences of
`JsonbIteratorNext` with `true` as the last
argument but without a "last argument is about skip" comment.
And there is a piece of code in the function `jsonb_delete` with a "skip
element" commentary:

```
/* skip corresponding value as well */
if (r == WJB_KEY)
JsonbIteratorNext(, , true);
```

So since in this patch it's not a simple skipping for setPathArray, I
removed that commentary. Am I wrong?

> 9. And finally... it does not work as expected in case of:

Yes, good catch, thanks.
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index c0b94bc..158e7fb 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10791,6 +10791,9 @@ table2-mapping
jsonb_set
   
   
+   jsonb_insert
+  
+  
jsonb_pretty
   
 
@@ -11072,6 +11075,39 @@ table2-mapping
 

   
+   
+   
+   jsonb_insert(target jsonb, path text[], new_value jsonb, after boolean)
+   
+   
+   jsonb
+   
+ Returns target with
+ new_value inserted.
+ Iftarget section designated by
+ path is a JSONB array,
+ new_value will be inserted before it, or
+ after if after is true (defailt is
+ false).  If target section
+ designated by path is a JSONB object,
+ new_value will be added just like a regular
+ key.  As with the path orientated operators, negative integers that
+ appear in path count from the end of JSON
+ arrays.
+   
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"')
+   
+   
+   jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"', true)
+   
+   
+   {"a": [0, "new_value", 1, 2]}
+ {"a": [0, 1, "new_value", 2]}
+
+   
+  
jsonb_pretty(from_json jsonb)
  
text
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index abf9a70..b1281e7 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -971,3 +971,11 @@ RETURNS jsonb
 LANGUAGE INTERNAL
 STRICT IMMUTABLE
 AS 'jsonb_set';
+
+CREATE OR REPLACE FUNCTION
+  jsonb_insert(jsonb_in jsonb, path text[] , replacement jsonb,
+insert_before_after boolean DEFAULT false)
+RETURNS jsonb
+LANGUAGE INTERNAL
+STRICT IMMUTABLE
+AS 'jsonb_insert';
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 88225aa..1c1da7c 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -33,6 +33,13 @@
 #include "utils/memutils.h"
 #include "utils/typcache.h"
 
+/* Operations available for setPath */
+#define JB_PATH_NOOP	0x0
+#define JB_PATH_CREATE	0x0001
+#define JB_PATH_DELETE	0x0002
+#define JB_PATH_INSERT_BEFORE			0x0004
+#define JB_PATH_INSERT_AFTER			0x0008
+
 /* semantic action functions for json_object_keys */
 static void okeys_object_field_start(void *state, char *fname, bool isnull);
 static void okeys_array_start(void *state);
@@ -130,14 +137,14 @@ static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
 static JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
 		bool *path_nulls, int path_len,
 		JsonbParseState **st, int level, Jsonb *newval,
-		bool create);
+		int op_type);
 static void setPathObject(JsonbIterator **it, Datum *path_elems,
 			  bool *path_nulls, int path_len, JsonbParseState **st,
 			  int level,
-			  Jsonb *newval, uint32 npairs, bool create);
+			  Jsonb *newval, uint32 npairs, int op_type);
 static void setPathArray(JsonbIterator **it, Datum *path_elems,
 			 bool *path_nulls, int path_len, JsonbParseState **st,
-			 int level, Jsonb *newval, uint32 nelems, bool create);
+			 int level, Jsonb *newval, uint32 nelems, int op_type);
 static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
 
 /* state for json_object_keys */
@@ -3544,7 +3551,7 @@ jsonb_set(PG_FUNCTION_ARGS)
 	it = JsonbIteratorInit(>root);
 
 	res = setPath(, path_elems, path_nulls, path_len, ,
-  0, newval, create);
+  0, newval, create ? JB_PATH_CREATE : JB_PATH_NOOP);
 
 	Assert(res != NULL);
 
@@ -3588,7 +3595,52 @@ jsonb_delete_path(PG_FUNCTION_ARGS)
 
 	it = JsonbIteratorInit(>root);
 
-	res = setPath(, path_elems, path_nulls, path_len, , 0, NULL, false);
+	res = setPath(, path_elems, path_nulls, path_len, ,
+  0, NULL, JB_PATH_DELETE);
+
+	Assert(res != NULL);
+
+	PG_RETURN_JSONB(JsonbValueToJsonb(res));
+}
+
+/*
+ * SQL function jsonb_insert(jsonb, text[], jsonb, boolean)
+ *
+ */
+Datum
+jsonb_insert(PG_FUNCTION_ARGS)
+{
+	

Re: [HACKERS] jsonb array-style subscription

[Dmitry Dolgov]

> If the patch were proposing a similar amount of new infrastructure to
> support some datatype-extensible concept of subscripting, I'd be much
> happier about it.

Well, actually, I agree with that. I can try to rework the patch to achieve
this goal.

Re: [HACKERS][REVIEW]: Password identifiers, protocol aging and SCRAM protocol

[Dmitry Dolgov]

On 1 March 2016 at 06:34, Michael Paquier  wrote:

> On Mon, Feb 29, 2016 at 8:43 PM, Valery Popov 
> wrote:
> > vpopov@vpopov-Ubuntu:~/Projects/pwdtest/postgresql$ git branch
>
> Thanks for the input!
>
> > 0001-Add-facility-to-store-multiple-password-verifiers.patch:2547:
> trailing
> > whitespace.
> > warning: 1 line adds whitespace errors.
> > 0003-Add-pg_auth_verifiers_sanitize.patch:87: indent with spaces.
> > if (!superuser())
> > warning: 1 line adds whitespace errors.
>
> Argh, yes. Those two ones have slipped though my successive rebases I
> think. Will fix in my tree, I don't think that it is worth sending
> again the whole series just for that though.
> --
> Michael
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>
Hi, Michael

Few questions about the documentation.

config.sgml:1200

>  
>   
>Specifies a comma-separated list of supported password formats by
>the server. Supported formats are currently plain and
>md5.
>   
>
>   
>When a password is specified in  or
>, this parameter determines if the
>password specified is authorized to be stored or not, returning
>an error message to caller if it is not.
>   
>
>   
>The default is plain,md5,scram, meaning that
MD5-encrypted
>passwords, plain passwords, and SCRAM-encrypted passwords are
accepted.
>   
>  

The default value contains "scram". Shouldn't be here also:

>Specifies a comma-separated list of supported password formats by
>the server. Supported formats are currently plain,
>md5 and scram.

Or I missed something?

And one more:

config.sgml:1284

>   
>db_user_namespace causes the client's and
>server's user name representation to differ.
>Authentication checks are always done with the server's user name
>so authentication methods must be configured for the
>server's user name, not the client's.  Because
>md5 uses the user name as salt on both the
>client and server, md5 cannot be used with
>db_user_namespace.
>   

Looks like the same (pls, correct me if I'm wrong) is applicable for
"scram" as I see from the code below. Shouldn't be "scram" mentioned here
also? Here's the code:

> diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
> index 28f9fb5..df0cc1d 100644
> --- a/src/backend/libpq/hba.c
> +++ b/src/backend/libpq/hba.c
> @@ -1184,6 +1184,19 @@ parse_hba_line(List *line, int line_num, char
*raw_line)
>  }
>  parsedline->auth_method = uaMD5;
>  }
>+ else if (strcmp(token->string, "scram") == 0)
>+ {
>+ if (Db_user_namespace)
>+ {
>+ ereport(LOG,
>+ (errcode(ERRCODE_CONFIG_FILE_ERROR),
>+ errmsg("SCRAM authentication is not supported when \"db_user_namespace\"
is enabled"),
>+ errcontext("line %d of configuration file \"%s\"",
>+ line_num, HbaFileName)));
>+ return NULL;
>+ }
>+ parsedline->auth_method = uaSASL;
>+ }
>  else if (strcmp(token->string, "pam") == 0)
> #ifdef USE_PAM
>  parsedline->auth_method = uaPAM;

Re: [HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

> I'd strongly prefer the jsonb_array_insert naming though
> I don't think it's a good idea to use set when this is used on object, I
think that we should throw error in that case.

Well, I thought it's wrong to introduce different functions and behaviour
patterns for objects and arrays, because it's the one data type after all.
But it's just my opinion of course.

> Also this patch needs documentation.

I've added new version in attachments, thanks.


jsonb_insert_v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] [PATH] Jsonb, insert a new value into an array at arbitrary position

[Dmitry Dolgov]

Hi

As far as I see there is one basic update function for jsonb, that can't be
covered by `jsonb_set` - insert a new value into an array at arbitrary
position.
Using `jsonb_set` function we can only append into array at the end/at the
beginning, and it looks more like a hack:

```
=# select jsonb_set('{"a": [1, 2, 3]}', '{a, 100}', '4');
  jsonb_set
-
 {"a": [1, 2, 3, 4]}
(1 row)


=# select jsonb_set('{"a": [1, 2, 3]}', '{a, -100}', '4');
  jsonb_set
-
 {"a": [4, 1, 2, 3]}
(1 row)
```

I think the possibility to insert into arbitrary position will be quite
useful,
something like `json_array_insert` in MySql:

```
mysql> set @j = '["a", {"b": [1, 2]}, [3, 4]]';
mysql> select json_array_insert(@j, '$[1].b[0]', 'x');

 json_array_insert(@j, '$[1].b[0]', 'x')
+-+
 ["a", {"b": ["x", 1, 2]}, [3, 4]]
```

It can look like `jsonb_insert` function in our case:

```
=# select jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"');
 jsonb_insert
---
 {"a": [0, "new_value", 1, 2]}
(1 row)
```

I attached possible implementation, which is basically quite small (all
logic-related
modifications is only about 4 lines in `setPath` function). This
implementation
assumes a flag to separate "insert before"/"insert after" operations, and an
alias to `jsonb_set` in case if we working with a jsonb object, not an
array.

What do you think about this?


jsonb_insert.patch
Description: Binary data

-- 
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] jsonb array-style subscription

[Dmitry Dolgov]

On 20 January 2016 at 02:14, Alvaro Herrera <alvhe...@2ndquadrant.com>
wrote:

> Dmitry Dolgov wrote:
>
> > I've cleaned up the code, created a separate JsonbRef node (and there
> are a
> > lot of small changes because of that), abandoned an idea of "deep
> nesting"
> > of assignments (because it doesn't relate to jsonb subscription, is more
> > about the
> > "jsonb_set" function, and anyway it's not a good idea). It looks fine for
> > me, and I need a little guidance - is it ok to propose this feature for
> > commitfest 2016-03 for a review?
>
> Has this patch been proposed in some commitfest previously?


Unfortunately, it wasn't. I just sent an intermediate version of this patch
to hackers several months ago to discuss it.

you can't add patches that are too invasive to the last one -- so your last
> chance for 9.6 was 2016-01.


Yes, that's what I was worried about (although I didn't know exactly about
this rule before).

[HACKERS] jsonb array-style subscription

[Dmitry Dolgov]

Hi,

Here is a reworked version of patch for jsonb subscription.
There weren't many changes in functionality:

=# create TEMP TABLE test_jsonb_subscript (
   id int,
   test_json jsonb
);

=# insert into test_jsonb_subscript values
(1, '{}'),
(2, '{}');

=# update test_jsonb_subscript set test_json['a'] = 42;
=# select * from test_jsonb_subscript;
 id |test_json
+--
  1 | {"a": 42}
  2 | {"a": 42}
(2 rows)

=# select test_json['a'] from test_jsonb_subscript;
 test_json

 {"a": 42}
 {"a": 42}
(2 rows)

I've cleaned up the code, created a separate JsonbRef node (and there are a
lot of small changes because of that), abandoned an idea of "deep nesting"
of assignments (because it doesn't relate to jsonb subscription, is more
about the
"jsonb_set" function, and anyway it's not a good idea). It looks fine for
me, and I need a little guidance - is it ok to propose this feature for
commitfest 2016-03 for a review?


jsonb_subscription.patch
Description: Binary data

-- 
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] jsonb - jsonb operators

[Dmitry Dolgov]

>  if there's any future intention to add a delete operator that removes
element/pair matches?

I think the operator (jsonb - jsonb) is logical because we have a shallow
concatenation function (something like a "union" operation), but we have
nothing like "set difference" and "intersection" functions. Actually, I
thought to implement these functions (at least for jsonbx). But of course
this function should be quite simple and consider only full key/value
matching as a target.

Re: [HACKERS] jsonb_set array append hack?

[Dmitry Dolgov]

>> For that matter, we should probably disallow NULL path elements also,
shouldn't we?
> I'd say yes.

Well, here is the new `setPath` function with this modification. Is it what
did you mean?


non_integer_in_path2.patch
Description: Binary data

-- 
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] jsonb_set array append hack?

[Dmitry Dolgov]

> I would expect some kind of error.  We're trying to address a position in
an array, and we're instead passing a key.  If it completes successfully,
the chances are it isn't what the user intended.

Thanks for the explanation. So, basically, it should be like this, am I
right?

postgres=# SELECT jsonb_set(
'{"name": "Joe", "vehicle_types": ["car", "van"]}'::jsonb,
'{vehicle_types, nonsense}',
'"motorcycle"', true);
ERROR:  path element at the position 2 is not an integer

On 20 September 2015 at 23:50, Thom Brown <t...@linux.com> wrote:

> On 20 September 2015 at 16:17, Dmitry Dolgov <9erthali...@gmail.com>
> wrote:
>
>> I'm sorry, but I'm not sure, what behavior is expected in this case?
>> Right now the following logic was implemented:
>> "we trying to set an element inside an array, but we've got a
>> non-integer path item
>> ("nonsense" in this particular case), so we're going to add a new
>> element at the end of array by default"
>>
>> If it's wrong, should we refuse to perform such kind of operations, or
>> should we replace
>> "vehicle_type": ["car", "van"]
>> to
>> "vehicle_type: {"nonsense": "motorcycle"}
>> ?
>>
>
> (please bottom-post)
>
> I would expect some kind of error.  We're trying to address a position in
> an array, and we're instead passing a key.  If it completes successfully,
> the chances are it isn't what the user intended.
>
> Thom
>


non_integer_in_path.patch
Description: Binary data

-- 
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] jsonb_set array append hack?

[Dmitry Dolgov]

I'm sorry, but I'm not sure, what behavior is expected in this case?
Right now the following logic was implemented:
"we trying to set an element inside an array, but we've got a
non-integer path item
("nonsense" in this particular case), so we're going to add a new
element at the end of array by default"

If it's wrong, should we refuse to perform such kind of operations, or
should we replace
"vehicle_type": ["car", "van"]
to
"vehicle_type: {"nonsense": "motorcycle"}
?

On 15 September 2015 at 01:59, Andrew Dunstan  wrote:

>
>
> On 09/14/2015 01:29 PM, Thom Brown wrote:
>
>> Hi,
>>
>> I've noticed that if you use a string for an element key in jsonb_set
>> with create_missing set to true, you can use it to append to an array:
>>
>> postgres=# SELECT jsonb_set(
>> '{"name": "Joe", "vehicle_types": ["car","van"]}'::jsonb,
>>'{vehicle_types,nonsense}',
>>'"motorcycle"', true);
>> jsonb_set
>> 
>>  {"name": "Joe", "vehicle_types": ["car", "van", "motorcycle"]}
>> (1 row)
>>
>> What this really should match is a nested element inside "vehicle_types"
>> called "nonsense".  But this seems to be a hack to get an element added to
>> an array.  To do it properly currently requires specifying an arbitrary
>> number in the hope that it will exceed the number of elements you have in
>> the array.
>>
>
>
> That's a bug and we should fix it.
>
>
>
>> e.g.
>>
>> postgres=# SELECT jsonb_set(
>>'{"name": "Joe", "vehicle_types": ["car","van"]}'::jsonb,
>>'{vehicle_types,10}',
>>'"motorcycle"', true);
>> jsonb_set
>> 
>>  {"name": "Joe", "vehicle_types": ["car", "van", "motorcycle"]}
>> (1 row)
>>
>> But I'm guessing people shouldn't be relying on the hack in the first
>> example.  Isn't this a bug?  If so, wouldn't this also be a bug?:
>>
>> postgres=# SELECT jsonb_set(
>>'{"name": "Joe", "vehicle_types": ["car","van"]}'::jsonb,
>>array['vehicle_types',NULL],
>>'"motorcycle"', true);
>>
>>
>>
> I think that's a bug too.
>
> 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
>

[HACKERS] jsonb array-style subscripting

[Dmitry Dolgov]

Hi,

Some time ago the array-style subscripting for the jsonb data type was
discussed in this mailing list. I think it will be quite convenient to have
a such nice syntax to update jsonb objects, so I'm trying to implement
this. I created a patch, that allows doing something like this:


=# create TEMP TABLE test_jsonb_subscript (
   id int,
   test_json jsonb
);

=# insert into test_jsonb_subscript values
(1, '{}'),
(2, '{}');

=# update test_jsonb_subscript set test_json['a']['a1']['a2'] = 42;
=# select * from test_jsonb_subscript;
 id |test_json
+--
  1 | {a: {a1: {a2: 42}}}
  2 | {a: {a1: {a2: 42}}}
(2 rows)

=# select test_json['a']['a1'] from test_jsonb_subscript;
 test_json

 {a2: 42}
 {a2: 42}
(2 rows)


This patch has a status work in progress of course. Generally speaking,
this implementation extends the `ArrayRef` usage for the jsonb.
And I need some sort of advice about several questions:

* is it interesting for the community?
* is that a good idea to extend the `ArrayRef` for jsonb? If it's
appropriate, probably we can rename it to `ArrayJsonbRef` of something.
* what can be improved in the code at the top level (function placement,
probably, functionality duplication, etc.)?
* are there any special cases, that I should take care of in this
implementation?


jsonb_subscript2.patch
Description: Binary data

-- 
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] jsonb concatenate operator's semantics seem questionable

[Dmitry Dolgov]

 creates the specified path if it does not exist. Does it?

No, jsonb_replace() doesn't create an element, if it doesn't exist. I
think, otherwise it can be confusing, so probably jsonb_add() may be more
appropriate (and, actually, this function was already mentioned in previous
discussions).

On 18 May 2015 at 22:57, Ryan Pedela rped...@datalanche.com wrote:

 On Mon, May 18, 2015 at 8:41 AM, Ryan Pedela rped...@datalanche.com
 wrote:

 On Sun, May 17, 2015 at 9:41 PM, Josh Berkus j...@agliodbs.com wrote:

 Is there a particular reason why + makes more sense as shallow
 concatination and || makes more sense as deep concatination?  Like,
 something in JS or other client languages which would make that
 preference make more sense to users?


 As someone who uses JSON day-to-day in Javascript and Python, I
 personally don't think || or + matters much. Python uses json.loads() for
 JSON concat and you have use a 3rd-party library in Javascript if you want
 that functionality such as JQuery.extends(). I agree with Peter that we
 need deep concatenation, but I don't think there is any standard for the
 operator. I think the word shallow should be added to the docs though.

 What is far more important than shallow or deep concatenation for the
 document database use case is being able to delete or replace/update a
 specific, nested path in the JSON object. It looks like that is possible
 with the minus operator and jsonb_replace(). This is great, however it took
 me awhile to figure out the path syntax. I think adding a paragraph to the
 docs explaining the path syntax would help.


 Having looked at this more, I think I understand the problem Peter has
 identified and it is a significant usability problem in my opinion. I think
 the word concatenate has confused me because I think of it as a
 higher-level operation when I want to merge two, large JSON objects which
 isn't a very common operation, at least for me. What is absolutely required
 for the document database use case is the following:

 1. Get element at any arbitrary path. ( # operator )
 2. Delete any arbitrary path. ( minus operator )
 3. Replace/update element at any arbitrary path. ( jsonb_replace )
 4. Add element to any arbitrary path. ( ? )

 It is #4 that does not seem to exist unless jsonb_replace() creates the
 specified path if it does not exist. Does it? I am not currently at my desk
 to test it myself.

 If not, deep concatenation would solve this problem, but I can also see
 another solution. Use + for shallow concatenation since it really means
 add element to top-level path as Peter suggests. Then add another
 function: jsonb_add( target jsonb, path text[], new jsonb ) to add element
 at any arbitrary path. Then leave || for deep concatenation in 9.6 or
 whenever.

 If jsonb_replace() satisfies #4 then I think everything is fine. Without
 #4 however, jsonb would remain an incomplete document database solution in
 my opinion.

 Thanks,
 Ryan Pedela

Re: [HACKERS] jsonb concatenate operator's semantics seem questionable

[Dmitry Dolgov]

 Historical note: I think it's based on the nested hstore work, not on
current hstore, but Dmitry can answer on that.

Yes, you're right.
And I agree with thoughts above, that both concatenation modes (simple
and deep) definitely can be useful. I can try to figure out how much work
that would be to modify the IteratorConcat function (or adapt Ilya's
solution)

On 17 May 2015 at 21:16, Petr Jelinek p...@2ndquadrant.com wrote:

 On 17/05/15 16:04, Andrew Dunstan wrote:


 On 05/16/2015 10:56 PM, Peter Geoghegan wrote:

 Another thing that I noticed about the new jsonb stuff is that the
 concatenate operator is based on the hstore one. This works as
 expected:

 postgres=# select '{a:1}'::jsonb || '{a:2}';
 ?column?
 --
   {a: 2}
 (1 row)

 However, the nesting doesn't match up -- containers are not merged
 beyond the least-nested level:

 postgres=# select '{a:{nested:1}}'::jsonb || '{a:{also
 nested:2}}';
   ?column?
 ---
   {a: {also nested: 2}}
 (1 row)

 This feels wrong to me. When jsonb was initially introduced, we took
 inspiration for the *containment* (operator @ jsonb) semantics from
 hstore, but since jsonb is nested it worked in a nested fashion. At
 the top level and with no nested containers there was no real
 difference, but we had to consider the behavior of more nested levels
 carefully (the containment operator is clearly the most important
 jsonb operator). I had envisaged that with the concatenation of jsonb,
 concatenation would similarly behave in a nested fashion. Under this
 scheme, the above query would perform nested concatenation as follows:

 postgres=# select '{a:{nested:1}}'::jsonb || '{a:{also
 nested:2}}'; -- does not match actual current behavior
   ?column?
 ---
   {a: {nested:1, also nested: 2}}
 (1 row)

 Now, I think it's good that the minus operator (operator - text and
 friends) discussed on the nearby thread accepts a text (or int)
 argument and remove string elements/pairs at the top level only. This
 works exactly the same as existence (I happen to think that removing
 elements/pairs at a nested level is likely to be more trouble than
 it's worth, and so I don't really like the new jsonb - text[]
 operator much, because it accepts a Postgres (not JSON) array of texts
 that constitute a path, which feels odd). So I have no issue with at
 least the plain minus operators' semantics. But I think that the
 concatenate operator's current semantics are significantly less useful
 than they could be, and are not consistent with the overall design of
 jsonb.


 Historical note: I think it's based on the nested hstore work, not on
 current hstore, but Dmitry can answer on that.

 I didn't dismiss this because it was a bad idea, but because it was too
 late in the process. If there is a consensus that we need to address
 this now then I'm happy to reopen that, but given the recent amount of
 angst about process I'm certainly not going to make such a decision
 unilaterally.

 Personally, I think there is plenty of room for both operations, and I
 can see use cases for both. If I were designing I'd leave || as it is
 now and add a + operation to do a recursive merge. I'm not sure how much
 work that would be. Not huge but not trivial either.


 Agreed, if you look at jquery for example, the extend() method by default
 behaves like our current || and you have to specify that you want deep
 merge if you want the behavior described by Peter. So there is definitely
 point for both, at this time we just support only one of them, that's all.


 --
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, 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] GSoC 2015: Extra Jsonb functionality

[Dmitry Dolgov]

 Frankly, I think the whole proposal needs to be rethought with an eye
towards supporting and preserving nested elements instead of trying to just
flatten everything out.

Can you pls show me few examples what do you mean exactly?

On 21 March 2015 at 06:51, Jim Nasby jim.na...@bluetreble.com wrote:

 On 3/19/15 9:07 AM, Thom Brown wrote:

  jsonb_to_array
  --
 {a, 1, b, c, 2, d, 3, 4}

 Is there a use-case for the example you've given above, where you take
 JSON containing objects and arrays, and flatten them out into a
 one-dimensional array?


 There are a lot of things proposed here that are completely ignoring the
 idea of nested elements, which I think is a big mistake.

 Frankly, I think the whole proposal needs to be rethought with an eye
 towards supporting and preserving nested elements instead of trying to just
 flatten everything out. If a user wanted things flat they would have just
 started with that in the first place.
 --
 Jim Nasby, Data Architect, Blue Treble Consulting
 Data in Trouble? Get it in Treble! http://BlueTreble.com

Re: [HACKERS] GSoC 2015: Extra Jsonb functionality

[Dmitry Dolgov]

 Perhaph it's my misunderstanding, but this would seem to be more of an
intersection operation on keys rather than a delete.
Hm...why? We remove all elements, which are contains in the first and
second jsonb (f: [4, 5] in this case) from the first one.

 Could there be a corresponding jsonb_except function which does the
opposite (i.e. returns everything on the left side except where it matches
with the right)?
and if I understand your question correctly, this is exactly what the
jsonb_delete_jsonb will do, isn't it?.

 Is there a use-case for the example you've given above, where you take
JSON containing objects and arrays, and flatten them out into a
one-dimensional array?
Hm...actually I don't know about such use-cases. This function is analog of
the hstore_to_array (and the similar function hstore_to_matrix), which is
used sometimes, judging by github. So I thought this function should be
implemented (after this question I'm not so sure).

 What should happen if g or {g} were used instead?
Did you mean {g: key}? Hmm...but in any case, I suppose this new object
should be appended to the array as a regular element.
=# jsonb_add_to_path('{b: {c: [d, f]}}'::jsonb, {b, c}::text[],
'g'::jsonb);

jsonb_add_to_path
---
   {b: {c: [d, f, g]}}


 This is a bit strange.  Why did f get flattened out of d?
The main purpose if this function is to get values for required keys from
all nesting levels (actually, I thougth it will be not so convenient
otherwise and I didn't consider the implementation with path usage). If
this so confusing, I can remove this function from the list =)

On 20 March 2015 at 00:08, Alvaro Herrera alvhe...@2ndquadrant.com wrote:

 Thom Brown wrote:
  On 19 March 2015 at 14:35, Alvaro Herrera alvhe...@2ndquadrant.com
 wrote:
   Thom Brown wrote:
   On 19 March 2015 at 14:12, Alvaro Herrera alvhe...@2ndquadrant.com
 wrote:
Dmitry Dolgov wrote:
   
* jsonb_slice - extract a subset of an jsonb
  Example of usage:

  Okay, so it pulls it all parents?  So I guess you'd get this too:
 
  SELECT jsonb_slice('{a: 1, b: {c: 2}, d: {f: 3}, f:
  4}'::jsonb, ARRAY['b', 'f', 'x']);
 
jsonb_slice
  
   {a: 1, b: {c: 2}, d: {f: 3}, f: 4}

 Yeah, except a wouldn't be output, of course.  (The example gets more
 interesting if d contains more members than just f.  Those would not
 get output.)


   Although I'm still a bit confused about f being produced.
  
   I guess you could say that the second argument is an array of element
   paths, not key names.  So to get the result I suggest, you would have
 to
   use ARRAY['{b}', '{d,f}', '{x}'].  (Hm, this is a non-rectangular
   array actually... I guess I'd go for ARRAY['b', 'd//f', 'x'] instead,
 or
   whatever the convention is to specify a json path).
 
  I think that's where jsquery would come in handy.

 If that's what we think, then perhaps we shouldn't accept jsonb_slice at
 all because of ambiguous mode of operation.

 --
 Álvaro Herrerahttp://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services

Re: [HACKERS] GSoC 2015: Extra Jsonb functionality

[Dmitry Dolgov]

 Would this also be the case for this function?...
 # jsonb_add_to_path('{b: {c: [d, f]}}'::jsonb, {b, c}::text[],
 '{g:4}'::jsonb);
 jsonb_add_to_path
 
 {b: {c: [d, f, {g: 4}]}}

Yes, sure (the similar logic already implemented  for the jsonb_concat).

On 20 March 2015 at 18:39, Thom Brown t...@linux.com wrote:

 On 20 March 2015 at 11:21, Dmitry Dolgov 9erthali...@gmail.com wrote:
  Perhaph it's my misunderstanding, but this would seem to be more of an
  intersection operation on keys rather than a delete.
  Hm...why? We remove all elements, which are contains in the first and
 second
  jsonb (f: [4, 5] in this case) from the first one.

 On further thought, yes, I agree.

  Could there be a corresponding jsonb_except function which does the
  opposite (i.e. returns everything on the left side except where it
 matches
  with the right)?
  and if I understand your question correctly, this is exactly what the
  jsonb_delete_jsonb will do, isn't it?.

 Ah, yes, that's true.

  Is there a use-case for the example you've given above, where you take
  JSON containing objects and arrays, and flatten them out into a
  one-dimensional array?
  Hm...actually I don't know about such use-cases. This function is analog
 of
  the hstore_to_array (and the similar function hstore_to_matrix), which is
  used sometimes, judging by github. So I thought this function should be
  implemented (after this question I'm not so sure).

 Yeah, hstore was just key=value, so flattening it out resulted in a
 simple {key,value,key,value} array.  I don't think that's useful with
 json.

  What should happen if g or {g} were used instead?
  Did you mean {g: key}? Hmm...but in any case, I suppose this new
 object
  should be appended to the array as a regular element.
  =# jsonb_add_to_path('{b: {c: [d, f]}}'::jsonb, {b,
 c}::text[],
  'g'::jsonb);
 
  jsonb_add_to_path
  ---
 {b: {c: [d, f, g]}}
 

 Would this also be the case for this function?...

 # jsonb_add_to_path('{b: {c: [d, f]}}'::jsonb, {b, c}::text[],
 '{g:4}'::jsonb);
  jsonb_add_to_path
 
  {b: {c: [d, f, {g: 4}]}}

 --
 Thom

[HACKERS] GSoC 2015: Extra Jsonb functionality

[Dmitry Dolgov]

Hi, everyone

I'm Dmitry Dolgov, a phd student at the KemSU, Russia. I would like to
submit a proposal to the GSoC about additional jsonb functionality, and I
want to get any feedback and thougths about this.

Re: [HACKERS] GSoC 2015: Extra Jsonb functionality

[Dmitry Dolgov]

Synopsis: Althrough Jsonb was introduced in PostgreSQL 9.4, there are
several functions, that still missing. Partially this missing functionality
was implemented in this extension [1] and the corresponding patch [2]. The
purpose of this work is to implement the rest of functions accordingly to
importance.

Benefits: New functionality, than can made the usage of the jsonb more
convenient.

Deliverables: Implementation of the following functions (in the form of an
extension
* jsonb_delete_jsonb - delete key/value pairs based on the other jsonb.
  Example of usage:

=# jsonb_delete_jsonb('{a: 1, b: {c: 2, d: 3}, f: [4,
5]}'::jsonb, '{a: 4, f: [4, 5], c: 2}'::jsonb);

 jsonb_delete_jsonb
---
 {a: 1, b: {c: 2, d: 3}}

* jsonb_slice - extract a subset of an jsonb
  Example of usage:

=# jsonb_slice('{a: 1, b: {c: 2}, d: {f: 3}}'::jsonb,
ARRAY['b', 'f', 'x']);

   jsonb_slice
---
  {b: {c: 2}, f: 3}

* jsonb_to_array - get jsonb keys and values as an array
  Example of usage:

=# jsonb_to_array('{a: 1, b: {c: 2}, d: [3, 4]}'::jsonb);

jsonb_to_array
--
   {a, 1, b, c, 2, d, 3, 4}

* jsonb_keys - get jsonb keys as an array
  Example of usage:

=# jsonb_keys('{a: 1, b: {c: 2}}'::jsonb);

jsonb_keys
-
{a, b, c}

* jsonb_vals - get jsonb values as an array
  Example of usage:

=# jsonb_vals('{a: 1, b: {c: 2}, d: [3, 4]}'::jsonb);

jsonb_vals
--
   {1, 2, 3, 4}

* jsonb_add_to_path - append a new element to jsonb value at the
specific path
  Example of usage:

   =# jsonb_add_to_path('{a: 1, b: {c: [d, f]}}'::jsonb, {b,
c}::text[], '[g]'::jsonb);

   jsonb_add_to_path
---
   {a: 1, b: {c: [d, f, g]}}

* jsonb_intersection - extract intersecting key/value pairs
  Example of usage:

=# jsonb_intersection('{a: 1, b: 2, d: {f: 3}, g: [4,
5]}'::jsonb, '{b: 2, c: 3, f: 3, g: [4, 5]}'::jsonb);

 jsonb_intersection

{b: 2, g: [4, 5]}

Schedule: I suppose, this can take 2-3 months for me. First of all I'll
implement the jsonb_delete_jsonb, jsonb_slice, jsonb_to_array, jsonb_keys,
jsonb_vals functions (just because it almost clear how to implement them).
Each function will require tests, and certainly some time will be spent at
the finish on the improvements for extension as a whole.

Unfortunately, this proposal isn't submitted to the GSoC system yet (I'm
planning to do this in the next Tuesday).

[1]: https://github.com/erthalion/jsonbx
[2]: https://commitfest.postgresql.org/4/154/

On 19 March 2015 at 20:16, Dmitry Dolgov 9erthali...@gmail.com wrote:

 Hi, everyone

 I'm Dmitry Dolgov, a phd student at the KemSU, Russia. I would like to
 submit a proposal to the GSoC about additional jsonb functionality, and I
 want to get any feedback and thougths about this.

Re: [HACKERS] mogrify and indent features for jsonb

[Dmitry Dolgov]

Hi, Thom.

 Would this support deleting type and the value 'dd'

With this patch you can delete them one by one:

 select '{a: 1, b: 2, c: {type: json, stuff: test}, d:
[aa,bb,cc,dd]}'::jsonb - '{c, type}'::text[] - '{d, -1}'::text[];
 ?column?
---
 {a: 1, b: 2, c: {stuff: test}, d: [aa, bb, cc]}
(1 row)


 Is there a way to take the json:
 '{a: 1, b: 2, c: {type: json, stuff: test}, d:
[aa,bb,cc,dd]}'
 and add ee to d without replacing it?

No, looks like there is no way to add a new element to array with help of
this patch. I suppose this feature can be implemented easy enough inside
the jsonb_concat function:

 select '{a: 1, b: 2, c: {type: json, stuff: test}, d:
[aa,bb,cc,dd]}'::jsonb || '{d: [ee]}'::jsonb

but I'm not sure, that it will be the best way.


On 26 February 2015 at 01:13, Josh Berkus j...@agliodbs.com wrote:

 On 02/25/2015 03:13 AM, Thom Brown wrote:
  Can you think of a reasonable syntax for doing that via operators?  I
  can imagine that as a json_path function, i.e.:
 
  jsonb_add_to_path(jsonb, text[], jsonb)
 
  or where the end of the path is an array:
 
  jsonb_add_to_path(jsonb, text[], text|int|float|bool)
 
  But I simply can't imagine an operator syntax which would make it
 clear
  what the user intended.
 
 
  No, there probably isn't a sane operator syntax for such an operation.
  A function would be nice.  I'd just want to avoid hacking away at arrays
  by exploding them, adding a value then re-arraying them and replacing
  the value.

 Well, anyway, that doesn't seem like a reason to block the patch.
 Rather, it's a reason to create another one for 9.6 ...


 --
 Josh Berkus
 PostgreSQL Experts Inc.
 http://pgexperts.com


 --
 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] mogrify and indent features for jsonb

[Dmitry Dolgov]

Hi, Petr, thanks for the review.

 I think it would be better if the ident printing didn't put the start
of array ([) and start of dictionary ({) on separate line
Did you mean this?

[
{
a: 1,
b: 2
}
]

I tried to verify this in several ways (http://jsonprettyprint.com/,
JSON.stringify, json.too from python), and I always get this result
(new line after ([) and ({) ).

 I don't really understand the point of h_atoi() function.
Initially, this function was to combine the convertion logic and specific
verifications. But I agree, strtol is more correct way, I should improve
this.

 The code looks ok as well except maybe the replacePath could use couple
of comments
I already added several commentaries (and looks like I should add even more
in the nearest future) for this function in the jsonbx extension, and I
think we can update this patch one more time with that improvement.

 About the Assert(ARR_ELEMTYPE(path) == TEXTOID);
I based my work on the hstore extension, which contains such kind of
assertions. But I suppose, it's not required anymore, so I removed this
from the extension. And, I think, we can also remove this from patch.

On 18 February 2015 at 08:32, Petr Jelinek p...@2ndquadrant.com wrote:

 Hi,

 I looked at the patch and have several comments.

 First let me say that modifying the individual paths of the json is the
 feature I miss the most in the current implementation so I am happy that
 this patch was submitted.

 I would prefer slightly the patch split into two parts, one for the indent
 printing and one with the manipulation functions, but it's not too big
 patch so it's not too bad as it is.

 There is one compiler warning that I see:
 jsonfuncs.c:3533:1: warning: no previous prototype for ‘jsonb_delete_path’
 [-Wmissing-prototypes]
  jsonb_delete_path(PG_FUNCTION_ARGS)

 I think it would be better if the ident printing didn't put the start of
 array ([) and start of dictionary ({) on separate line since most pretty
 printing implementations outside of Postgres (like the JSON.stringify or
 python's json module) don't do that. This is purely about consistency with
 the rest of the world.

 The json_ident might be better named as json_pretty perhaps?

 I don't really understand the point of h_atoi() function. What's wrong
 with using strtol like pg_atoi does? Also there is no integer overflow
 check anywhere in that function.

 There is tons of end of line whitespace mess in jsonb_indent docs.

 Otherwise everything I tried so far works as expected. The code looks ok
 as well except maybe the replacePath could use couple of comments (for
 example the line which uses the h_atoi) to make it easier to follow.

 About the:

 +   /* XXX : why do we need this assertion? The functions is declared
 to take text[] */
 +   Assert(ARR_ELEMTYPE(path) == TEXTOID);


 I wonder about this also, some functions do that, some don't, I am not
 really sure what is the rule there myself.

 --
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, 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] GSoC 2015 - mentors, students and admins.

[Dmitry Dolgov]

I would like to participate as student.

On 10 February 2015 at 03:52, Thom Brown t...@linux.com wrote:

 Hi all,

 Google Summer of Code 2015 is approaching.  I'm intending on registering
 PostgreSQL again this year.

 Before I do that, I'd like to have an idea of how many people are
 interested in being either a student or a mentor.

 I've volunteered to be admin again, but if anyone else has a strong
 interest of seeing the projects through this year, please let yourself be
 known.

 Who would be willing to mentor projects this year?

 Project ideas are welcome, and I've copied many from last year's
 submissions into this year's wiki page.  Please feel free to add more (or
 delete any that stand no chance or are redundant):
 https://wiki.postgresql.org/wiki/GSoC_2015

 Students can find more information about GSoC here:
 http://www.postgresql.org/developer/summerofcode

 Thanks

 Thom

[HACKERS] JsonbValue to Jsonb conversion

[Dmitry Dolgov]

Hi all,

I'm faced with some troubles about the jsonb implementation, and I hope
I'll get little advice =)
If I understand correctly, an abstract function for jsonb modification
should have the following stages:

Jsonb - JsonbValue - Modification - JsonbValue - Jsonb

One can convert the *JsonbValue* to the *Jsonb* only by *JsonbValueToJsonb*
function. So, my question is can be *JsonbValue*, that contains few
*jbvBinary* elements, converted to *Jsonb* by this function? It will be
very useful, if you want modify only small part of your JsonbValue (e.g.
replace value of some key). But when I'm trying to do this, an exception
unknown type of jsonb container appears. Maybe I missed something? Or is
there another approach to do this conversion?

[HACKERS] Jsonb: jbvBinary usage in the convertJsonbValue?

[Dmitry Dolgov]

Hi all,

I'm little confused by the *convertJsonbValue* functon at *jsonb_utils.c*
Maybe I misunderstood something, so I need help =)

 if (IsAJsonbScalar(val) || val-type == jbvBinary)
 convertJsonbScalar(buffer, header, val);

As I can see, the *convertJsonbScalar* function is used for scalar and
binary jsonb values. But this function doesn't handle the jbvBinary type.

 switch (scalarVal-type)
 case jbvNull:
 ...
 case jbvString:
 ...
 case jbvNumeric:
 .
 case jbvBool:
 .
 default:
 elog(ERROR, invalid jsonb scalar type);

Does this mean, that binary type is incorrect for *convertJsonbValue *? Or
am I missed something?

Re: [HACKERS] Json(b) extension

[Dmitry Dolgov]

 If your goal is to make this functionality available as soon as
possible to users, an external extension is the way to go.
No, my primary goal is to create the useful contrib extension and help to
the community =)
So, I also want to do it as a pgxn extension.


On 25 April 2014 00:11, Josh Berkus j...@agliodbs.com wrote:

 On 04/24/2014 03:46 AM, Dmitry Dolgov wrote:
  Hi all,
 
  As you know, PostgreSQL introduced Json(b) support at the 9.4 version
 [1],
  and hstore v2.0 saved in separate repository [2]. But although PostgreSQL
  has this support at the core level, there are many useful functions,
 which
  wasn't ported to Json(b) from hstore v2.0 and json. Here [3], I've made a
  review of the missing Json(b) functions, which can be implemented. This
  list can be updated in the future, of course. I want to implement the
  missing functions in the form of extension (e.g. contrib/jsonx).

 Thanks for making this list!

 
  What do you think about this?

 I think you should do it as a pgxn extension, for now.  For it to be in
 contrib/ or core, we'd need to argue extensively about the names of
 operators and functions, which will take a while.If your goal is to
 make this functionality available as soon as possible to users, an
 external extension is the way to go.

 --
 Josh Berkus
 PostgreSQL Experts Inc.
 http://pgexperts.com


 --
 Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Json(b) extension

[Dmitry Dolgov]

Hi all,

As you know, PostgreSQL introduced Json(b) support at the 9.4 version [1],
and hstore v2.0 saved in separate repository [2]. But although PostgreSQL
has this support at the core level, there are many useful functions, which
wasn't ported to Json(b) from hstore v2.0 and json. Here [3], I've made a
review of the missing Json(b) functions, which can be implemented. This
list can be updated in the future, of course. I want to implement the
missing functions in the form of extension (e.g. contrib/jsonx).

What do you think about this?


[1]: http://obartunov.livejournal.com/177247.html
[2]: http://www.sigaev.ru/git/gitweb.cgi?p=hstore.git;a=summary
[3]:
https://gist.githubusercontent.com/erthalion/10890778/raw/f042872a92a1ab22ed7eb753fee2358b60d99d4a/hstore_to_jsonb.rst