Hi čt 19. 12. 2019 v 15:20 odesílatel Dmitry Dolgov <9erthali...@gmail.com> napsal:
> > On Sun, Nov 10, 2019 at 01:32:08PM +0100, Dmitry Dolgov wrote: > > > > > I had to write new assignment logic reusing only some parts of > setPath(), > > > because the loop in setPath() should be broken on every level. During > this > > > process, I decided to implement assignment behavior similar to > PostgreSQL's > > > array behavior and added two new features: > > > - creation of jsonb arrays/objects container from NULL values > > > - appending/prepending array elements on the specified position, gaps > filled > > > with nulls (JavaScript has similar behavior) > > > > What is the reason for the last one? > > I've splitted the last patch into polymorphic itself and jsonb array > behaviour changes, since I'm afraid it could be a questionable part. > I tested last set of patches. I like patch 0006 - filling gaps by NULLs - it fixed my objections if I remember correctly. Patch 0005 - polymorphic subscribing - I had not a idea, what is a use case? Maybe can be good to postpone this patch. I have not strong opinion about it, but generally is good to reduce size of initial patch. I have nothing against a compatibility with SQL, but this case doesn't looks too realistic for me, and can be postponed without future compatibility issues. I did some notes: It needs rebase, I had to fix some issues. I miss deeper comments for +static Oid +findTypeSubscriptingFunction(List *procname, Oid typeOid, bool parseFunc) +/* Callback function signatures --- see xsubscripting.sgml for more info. */ +typedef SubscriptingRef * (*SubscriptingPrepare) (bool isAssignment, SubscriptingRef *sbsef); + +typedef SubscriptingRef * (*SubscriptingValidate) (bool isAssignment, SubscriptingRef *sbsef, +<-><--><--><--><--><--><--><--><--><--><--><--> struct ParseState *pstate); + +typedef Datum (*SubscriptingFetch) (Datum source, struct SubscriptingRefState *sbsrefstate); + +typedef Datum (*SubscriptingAssign) (Datum source, struct SubscriptingRefState *sbrsefstate); + +typedef struct SubscriptRoutines +{ +<->SubscriptingPrepare><-->prepare; #### . +<->SubscriptingValidate<-->validate; +<->SubscriptingFetch<-><-->fetch; +<->SubscriptingAssign<><-->assign; + +} SubscriptRoutines; + regress tests fails +Datum +array_subscript_fetch(Datum containerSource, SubscriptingRefState *sbstate) there is a variable "is_slice". Original code had not this variable. Personally I think so original code was better readable without this variable. so instead +<->if (is_slice) +<->{ +<-><-->for(i = 0; i < sbstate->numlower; i++) +<-><--><-->l_index.indx[i] = DatumGetInt32(sbstate->lowerindex[i]); +<->} is more readable if (sbstate->numlower > 0) { /* read lower part of indexes */ for (i = 0; i < sbstate->numlower; ... I miss comments (what is checked here - some like - subscript have to be int4 and number of subscripts should be less than MAXDIM) + +SubscriptingRef * +array_subscript_prepare(bool isAssignment, SubscriptingRef *sbsref) +SubscriptingRef * +array_subscript_validate(bool isAssignment, SubscriptingRef *sbsref, +<-><--><--><--><--> ParseState *pstate) Regression tests fails - see a attachment I really miss a PLpgSQL support postgres=# do $$ declare j jsonb = '{"a":10, "b":20}'; begin raise notice '%', j; raise notice '%', j['a']; j['a'] = '20'; raise notice '%', j; end; $$; NOTICE: {"a": 10, "b": 20} NOTICE: 10 ERROR: subscripted object is not an array CONTEXT: PL/pgSQL function inline_code_block line 6 at assignment With PLpgSQL support it will be great patch, and really important functionality. It can perfectly cover some gaps of plpgsql. Regards Pavel
regression.out
Description: Binary data
regression.diffs
Description: Binary data