Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1siq4m-00A19y-EV for pgsql-hackers@arkaria.postgresql.org; Tue, 27 Aug 2024 06:53:12 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1siq4j-002pcc-Vn for pgsql-hackers@arkaria.postgresql.org; Tue, 27 Aug 2024 06:53:10 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1siq4j-002pa9-8Z for pgsql-hackers@lists.postgresql.org; Tue, 27 Aug 2024 06:53:10 +0000 Received: from mail-yw1-x112d.google.com ([2607:f8b0:4864:20::112d]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1siq4g-001dyq-D2 for pgsql-hackers@lists.postgresql.org; Tue, 27 Aug 2024 06:53:08 +0000 Received: by mail-yw1-x112d.google.com with SMTP id 00721157ae682-6b4432b541aso45099317b3.1 for ; Mon, 26 Aug 2024 23:53:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724741585; x=1725346385; darn=lists.postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=CJ5GFPRNbPdGYq96dKaqG/Ba3/XvCr3f5+g6DTKTn+M=; b=UOxmmoQD/OC7W8ftCF95WOT/xpqpp3jMOpAdPP67Fc9jSRqqhvikJ8btU0bMu3tYcQ 7HJqXSi+8hjMQmwy5B+7bacnKRKJUUB6FTAijCMZsLRLX7eLukGAlueGOFu48UzTriHm Q7TGdfTQveWG7+vTP6Uk9w6Fm8xNTKR0N6ayGONEjikvkJXXRC8Mg9D/v8MAWcwYJqsp sqetqGsbHuOXQ7vlPsu/3vLQ776nXrnzeaOkyB7oRD6bmcnIV85QI51eMq1Yh1fbbDah dMSYwF9hglRkaYwfUNMSophAliXMrI7dCEIxBpUf3Zx2dG8EzIjty8YNT35qrLxgNnhn lzew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724741585; x=1725346385; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=CJ5GFPRNbPdGYq96dKaqG/Ba3/XvCr3f5+g6DTKTn+M=; b=O+cwS+JqjuwTWeb65hPM3mtZzeacz8zMybyfr85F9RuT/WIIrjk5j94rmb3cy5KY5I pBHEunQCfT0EK2d7v6taHTzw6VZ6l9D5m8QPQEpOYROLS9e0qcpzYVr4FvUzZ82eLKZN ErjUNGWOJuSTdSjVFysVUTerb9xtjTsWkZL+etiX7sp4EGwk/1hcoSBSZFPW/Fx2aQCh eXAAniyvUqDMXzcl/p7Kyax6+5uAn0lAGcBgdErxUu+nKb8ROURzMVl88VenZENsZRBH 6GbBOYaPyh20yESD2GHz6eyUSsR5PNrHNiJ8XBChYj8tRnyOvOKc/kCO3Mjsvwlxzzsd 6YXQ== X-Forwarded-Encrypted: i=1; AJvYcCWQNt8co7KykYCkWhdbrzsF2+1KBQu4dNhTId0M3s8SyyixSA7qFxs6UfoHqsc5FNhVJyDAQ01z2pcN832j@lists.postgresql.org X-Gm-Message-State: AOJu0Yxb6fSRAoTSUWw4wCz6XQMVOz2UgnOVkaDnpYuF5QSqhq1CBa2F sYQ9Z3MM0P4X/iGSH6CTEL1U0Fmf0f5HbIL+HuWEip782ZTKFVJhXBlD/OGgd9xNoIocHmfYjjc Bi7HBaNndp/wI+Egt/Qykf8xG+qY= X-Google-Smtp-Source: AGHT+IHYA5Gl1sAmzoldoO7MTwlroKB40LKH0cxIfSjdXpk8ximydfFGaIq6BaRwIUddkS2sTzJcZX0zyLPx/l4X/LM= X-Received: by 2002:a05:690c:4585:b0:65f:dfd9:b672 with SMTP id 00721157ae682-6c624fb6c9emr111466087b3.11.1724741585367; Mon, 26 Aug 2024 23:53:05 -0700 (PDT) MIME-Version: 1.0 References: <20200924035637.GF28585@paquier.xyz> <20201001033824.GC8130@paquier.xyz> <51a9a68e8a998d04df17417d45c1dbd4@xs4all.nl> <89817942c99da01cd5e7850fe418436b@xs4all.nl> <56ca532c37eb0b540961f74a7bd5db39@xs4all.nl> <8181bd3abc647bdae5a4f78e71e62478a98c75f4.camel@cybertec.at> <9e67d49deb18270eddb95e602c83f02b98459843.camel@cybertec.at> <3b662dc5b615d4c20a55e8e2fbe6fc00fe00609d.camel@cybertec.at> <6996931e8c9edf3b82223e74e92326a7ed06c1d6.camel@cybertec.at> <67aa68a7e6dfb44c0cbbdf7f97cadfede4269ce5.camel@cybertec.at> <04ec666686e9e21cb515617df06885c66f3d34ce.camel@cybertec.at> In-Reply-To: <04ec666686e9e21cb515617df06885c66f3d34ce.camel@cybertec.at> From: Pavel Stehule Date: Tue, 27 Aug 2024 08:52:27 +0200 Message-ID: Subject: Re: proposal: schema variables To: Laurenz Albe Cc: Erik Rijkers , Michael Paquier , Amit Kapila , DUVAL REMI , PostgreSQL Hackers Content-Type: multipart/alternative; boundary="0000000000008679440620a4b1de" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --0000000000008679440620a4b1de Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi =C3=BAt 27. 8. 2024 v 8:15 odes=C3=ADlatel Laurenz Albe napsal: > On Thu, 2024-08-15 at 07:55 +0200, Pavel Stehule wrote: > > =C3=BAt 30. 7. 2024 v 21:46 odes=C3=ADlatel Laurenz Albe > napsal: > > > - A general reminder: single line comments should start with a lower > case > > > letter and have to period in the end: > > > > Should it be "have not to period in the end" ? > > I made a typo. I mean "have *no* period in the end". > > > I fixed almost all without parts related to psql and executor - there > almost all > > current comments break the mentioned rule. So I use the style used in > these files. > > I can fix it (if you like it) - or it can be fixed generally in a > separated patch set. > > Thanks. I also noticed that a lot of existing comments break that rule, > so I think that it is fine if you stick with the way that the surrounding > code does it. > > > > - Variable as parameter: > > > > > > CREATE VARIABLE var AS date; > > > LET var =3D current_date; > > > PREPARE stmt(date) AS SELECT $1; > > > EXECUTE stmt(var); > > > ERROR: paramid of PARAM_VARIABLE param is out of range > > > > > > Is that working as intended? I don't understand the error message. > > > > fixed in 0002 patch (variables cannot be used as EXECUTE argument) and > 0014 (enable usage variables as an argument of EXECUTE) > > Thanks. > > > > > --- a/src/backend/catalog/namespace.c > > > > +++ b/src/backend/catalog/namespace.c > > > > [...] > > > > +/* > > > > + * IdentifyVariable - try to find variable identified by list of > names. > > > > [...] > > > > > > Perhaps part of the reason why this function is so complicated is > that > > > you support things like this: > > > > > > CREATE SCHEMA sch; > > > CREATE TYPE cp AS (a integer, b integer); > > > CREATE VARIABLE sch.v AS cp; > > > LET sch.v =3D (1, 2); > > > SELECT sch.v.b; > > > b > > > =E2=95=90=E2=95=90=E2=95=90 > > > 2 > > > (1 row) > > > > > > test=3D# SELECT test.sch.v.b; > > > b > > > =E2=95=90=E2=95=90=E2=95=90 > > > 2 > > > (1 row) > > > > > > We don't support that for tables: > > > > > > CREATE TABLE tab (c cp); > > > INSERT INTO tab VALUES (ROW(42, 43)); > > > SELECT tab.c.b FROM tab; > > > ERROR: cross-database references are not implemented: tab.c.b > > > > > > You have to use extra parentheses: > > > > > > SELECT (tab.c).b FROM tab; > > > b > > > =E2=95=90=E2=95=90=E2=95=90=E2=95=90 > > > 43 > > > (1 row) > > > > > > I'd say that this should be the same for session variables. > > > What do you think? > > > > I prefer the current state, but I don't have a very strong opinion abou= t > it. > > It can save 115 lines of almost trivial code, but I think the user > experience > > can be much worse. Using composite types in tables is not too common a > > pattern (and when I read some recommendations for Oracle, it is an > antipattern), > > but usage of composite variables is common (it can hold a row). Moreove= r, > > we talked long about possible identifier's collisions, and the pattern > > schema.variable is very good protection against possible collisions. > > Probably the pattern catalog.schema.variable.field is not too interesti= ng > > but the support has 50 lines. > > > > More, the plpgsql supports pattern label.variable.field, so can be > little bit > > unfriendly if session variables doesn't support "similar" pattern > > I see your point, and I'm fine with leaving it as it is. > > > > > > > > - src/backend/commands/session_variable.c > > > > > > There is one comment that confuses me and that I did not edit: > > > > > > > +Datum > > > > +GetSessionVariable(Oid varid, bool *isNull, Oid *typid) > > > > +{ > > > > + SVariable svar; > > > > + > > > > + svar =3D get_session_variable(varid); > > > > + > > > > + /* > > > > + * Although svar is freshly validated in this point, the > svar->is_valid can > > > > + * be false, due possible accepting invalidation message > inside domain > > > > + * check. Now, the validation is done after lock, that can > also accept > > > > + * invalidation message, so validation should be trustful. > > > > + * > > > > + * For now, we don't need to repeat validation. Only svar > should be valid > > > > + * pointer. > > > > + */ > > > > This comment is related to assertions. Before I had there > `Assert(svar->is_valid)`, > > because I expected it. But it was not always true. And although it is > true, > > we don't need to validate a variable, because at this moment, the > variable > > should be locked, and then we can return content safely. > > I guess my main problem is the word "trustful". I don't recognize that > word. > Perhaps you can reword the comment along the lines of your above > explanation. > I'll try to change it > > > > > > - src/backend/executor/execMain.c > > > > > > > @@ -196,6 +198,51 @@ standard_ExecutorStart(QueryDesc *queryDesc, > int eflags) > > > > Assert(queryDesc->sourceText !=3D NULL); > > > > estate->es_sourceText =3D queryDesc->sourceText; > > > > > > > > + /* > > > > + * The executor doesn't work with session variables directly. > Values of > > > > + * related session variables are copied to dedicated array, > and this array > > > > + * is passed to executor. > > > > + */ > > > > > > Why? Is that a performance measure? The comment should explain > that. > > > > Session variables are volatile internally - some function that uses LET > statements > > can be called more times inside a query. This behavior is not consisten= t > with > > plpgsql's variables or external parameters. And if we want to support > parallel > > queries, then volatile session variables can be pretty messy (and > unusable). > > If we want the same behaviour for queries with or without parallel > support, > > then the session variables should be "stable" (like stable functions). > > Simple implementation is using "snapshot" of values of used session > variables > > when query is started. This "snapshot" is immutable, so we don't need t= o > make > > a copy more times. > > > > I changed this comment > > Thanks. > > > > - parallel safety > > > > > > > --- a/src/backend/optimizer/util/clauses.c > > > > +++ b/src/backend/optimizer/util/clauses.c > > > > @@ -935,6 +936,13 @@ max_parallel_hazard_walker(Node *node, > max_parallel_hazard_context *context) > > > > if (param->paramkind =3D=3D PARAM_EXTERN) > > > > return false; > > > > > > > > + /* We doesn't support passing session variables to worker= s > */ > > > > + if (param->paramkind =3D=3D PARAM_VARIABLE) > > > > + { > > > > + if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED, > context)) > > > > + return true; > > > > + } > > > > > > Even if a later patch alleviates that restriction, this patch shoul= d > document that > > > session variables imply "parallel restricted". I have added that t= o > doc/src/sgml/parallel.sgml. > > > > > > > merged (and removed in 0015) > > Thanks. > > > I hope I can do some more review at some point in the future... > > I sincerely hope that this patch set gets merged at some point. > The big obstacle is that it is so large. That's probably because it is > pretty > feature-complete (but, as we have found, not totally free of bugs). > Any feature that builds its own system catalog cannot be short. The first two patches have 300KB, like all others and just basic support for reading and writing. All other features have 300KB. On second hand almost all code is simple, and there are no changes in critical parts of Postgres. The size and complexity of JSONPath is level higher. I can throw 200KB from another 300KB patch set which can be better for review, but it can be harder to maintain this patch set. I'll try it, and I'll send a reduced version. > Judging from the amount of time I put into my review so far, I guess that > this > patch set would keep a committer busy for several weeks. Perhaps the onl= y > way to > get this done would be to make you a committer... > :-) Unfortunately, I am very bad at languages, I had a lot of problems in basic school just with Czech lang, Russian and English was more terrible) - so I very appreciate your work on this patch. Thank you very much Pavel > Yours, > Laurenz Albe > --0000000000008679440620a4b1de Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

=C3=BAt 27. 8. 2024 v=C2=A08:15 odes=C3=ADlat= el Laurenz Albe <laurenz.alb= e@cybertec.at> napsal:
On Thu, 2024-08-15 at 07:55 +0200, Pavel Stehule wrote:
> =C3=BAt 30. 7. 2024 v=C2=A021:46 odes=C3=ADlatel Laurenz Albe <laurenz.albe@cybe= rtec.at> napsal:
> > - A general reminder: single line comments should start with a lo= wer case
> > =C2=A0 letter and have to period in the end:
>
> Should it be "have not to period in the end" ?

I made a typo.=C2=A0 I mean "have *no* period in the end".

> I fixed almost all without parts related to psql and executor - there = almost all
> current comments break the mentioned rule. So I use the style used in = these files.
> I can fix it (if you like it) - or it can be fixed generally in a sepa= rated patch set.

Thanks.=C2=A0 I also noticed that a lot of existing comments break that rul= e,
so I think that it is fine if you stick with the way that the surrounding code does it.

> > - Variable as parameter:
> >
> > =C2=A0 CREATE VARIABLE var AS date;
> > =C2=A0 LET var =3D current_date;
> > =C2=A0 PREPARE stmt(date) AS SELECT $1;
> > =C2=A0 EXECUTE stmt(var);
> > =C2=A0 ERROR:=C2=A0 paramid of PARAM_VARIABLE param is out of ran= ge
> >
> > =C2=A0 Is that working as intended?=C2=A0 I don't understand = the error message.
>
> fixed in 0002 patch (variables cannot be used as EXECUTE argument) and= 0014 (enable usage variables as an argument of EXECUTE)

Thanks.

> > =C2=A0 > --- a/src/backend/catalog/namespace.c
> > =C2=A0 > +++ b/src/backend/catalog/namespace.c
> > =C2=A0 > [...]
> > =C2=A0 > +/*
> > =C2=A0 > + * IdentifyVariable - try to find variable identifie= d by list of names.
> > =C2=A0 > [...]
> >
> > =C2=A0 Perhaps part of the reason why this function is so complic= ated is that
> > =C2=A0 you support things like this:
> >
> > =C2=A0 =C2=A0 CREATE SCHEMA sch;
> > =C2=A0 =C2=A0 CREATE TYPE cp AS (a integer, b integer);
> > =C2=A0 =C2=A0 CREATE VARIABLE sch.v AS cp;
> > =C2=A0 =C2=A0 LET sch.v =3D (1, 2);
> > =C2=A0 =C2=A0 SELECT sch.v.b;
> > =C2=A0 =C2=A0 =C2=A0b
> > =C2=A0 =C2=A0 =E2=95=90=E2=95=90=E2=95=90
> > =C2=A0 =C2=A0 =C2=A02
> > =C2=A0 =C2=A0 (1 row)
> >
> > =C2=A0 =C2=A0 test=3D# SELECT test.sch.v.b;
> > =C2=A0 =C2=A0 =C2=A0b
> > =C2=A0 =C2=A0 =E2=95=90=E2=95=90=E2=95=90
> > =C2=A0 =C2=A0 =C2=A02
> > =C2=A0 =C2=A0 (1 row)
> >
> > =C2=A0 We don't support that for tables:
> >
> > =C2=A0 =C2=A0 CREATE TABLE tab (c cp);
> > =C2=A0 =C2=A0 INSERT INTO tab VALUES (ROW(42, 43));
> > =C2=A0 =C2=A0 SELECT tab.c.b FROM tab;
> > =C2=A0 =C2=A0 ERROR:=C2=A0 cross-database references are not impl= emented: tab.c.b
> >
> > =C2=A0 You have to use extra parentheses:
> >
> > =C2=A0 =C2=A0 SELECT (tab.c).b FROM tab;
> > =C2=A0 =C2=A0 =C2=A0b=C2=A0
> > =C2=A0 =C2=A0 =E2=95=90=E2=95=90=E2=95=90=E2=95=90
> > =C2=A0 =C2=A0 =C2=A043
> > =C2=A0 =C2=A0 (1 row)
> >
> > =C2=A0 I'd say that this should be the same for session varia= bles.
> > =C2=A0 What do you think?
>
> I prefer the current state, but I don't have a very strong opinion= about it.
> It can save 115 lines of almost trivial code, but I think the user exp= erience
> can be much worse.=C2=A0 Using composite types in tables is not too co= mmon a
> pattern (and when I read some recommendations for Oracle, it is an ant= ipattern),
> but usage of composite variables is common (it can hold a row). Moreov= er,
> we talked long about possible identifier's collisions, and the pat= tern
> schema.variable is very good protection against possible collisions. > Probably the pattern catalog.schema.variable.field is not too interest= ing
> but the support has 50 lines.
>
> More, the plpgsql supports pattern label.variable.field, so can be lit= tle bit
> unfriendly if session variables doesn't support "similar"= ; pattern

I see your point, and I'm fine with leaving it as it is.

>
>
> > - src/backend/commands/session_variable.c
> >
> > =C2=A0 There is one comment that confuses me and that I did not e= dit:
> >
> > =C2=A0 > +Datum
> > =C2=A0 > +GetSessionVariable(Oid varid, bool *isNull, Oid *typ= id)
> > =C2=A0 > +{
> > =C2=A0 > +=C2=A0 =C2=A0SVariable=C2=A0 =C2=A0svar;
> > =C2=A0 > +
> > =C2=A0 > +=C2=A0 =C2=A0svar =3D get_session_variable(varid); > > =C2=A0 > +
> > =C2=A0 > +=C2=A0 =C2=A0/*
> > =C2=A0 > +=C2=A0 =C2=A0 * Although svar is freshly validated i= n this point, the svar->is_valid can
> > =C2=A0 > +=C2=A0 =C2=A0 * be false, due possible accepting inv= alidation message inside domain
> > =C2=A0 > +=C2=A0 =C2=A0 * check. Now, the validation is done a= fter lock, that can also accept
> > =C2=A0 > +=C2=A0 =C2=A0 * invalidation message, so validation = should be trustful.
> > =C2=A0 > +=C2=A0 =C2=A0 *
> > =C2=A0 > +=C2=A0 =C2=A0 * For now, we don't need to repeat= validation. Only svar should be valid
> > =C2=A0 > +=C2=A0 =C2=A0 * pointer.
> > =C2=A0 > +=C2=A0 =C2=A0 */
>
> This comment is related to assertions. Before I had there `Assert(svar= ->is_valid)`,
> because I expected it. But it was not always true. And although it is = true,
> we don't need to validate a variable, because at this moment, the = variable
> should be locked, and then we can return content safely.

I guess my main problem is the word "trustful".=C2=A0 I don't= recognize that word.
Perhaps you can reword the comment along the lines of your above explanatio= n.

I'll try to change it
= =C2=A0

>
> > - src/backend/executor/execMain.c
> >
> > =C2=A0 > @@ -196,6 +198,51 @@ standard_ExecutorStart(QueryDesc= *queryDesc, int eflags)
> > =C2=A0 >=C2=A0 =C2=A0 =C2=A0Assert(queryDesc->sourceText != =3D NULL);
> > =C2=A0 >=C2=A0 =C2=A0 =C2=A0estate->es_sourceText =3D query= Desc->sourceText;
> > =C2=A0 >
> > =C2=A0 > +=C2=A0 =C2=A0/*
> > =C2=A0 > +=C2=A0 =C2=A0 * The executor doesn't work with s= ession variables directly. Values of
> > =C2=A0 > +=C2=A0 =C2=A0 * related session variables are copied= to dedicated array, and this array
> > =C2=A0 > +=C2=A0 =C2=A0 * is passed to executor.
> > =C2=A0 > +=C2=A0 =C2=A0 */
> >
> > =C2=A0 Why?=C2=A0 Is that a performance measure?=C2=A0 The commen= t should explain that.
>
> Session variables are volatile internally - some function that uses LE= T statements
> can be called more times inside a query. This behavior is not consiste= nt with
> plpgsql's variables or external parameters. And if we want to supp= ort parallel
> queries, then volatile session variables can be pretty messy (and unus= able).
> If we want the same behaviour for queries with or without parallel sup= port,
> then the session variables should be "stable" (like stable f= unctions).
> Simple implementation is using "snapshot" of values of used = session variables
> when query is started. This "snapshot" is immutable, so we d= on't need to make
> a copy more times.
>
> I changed this comment

Thanks.

> > - parallel safety
> >
> > =C2=A0 > --- a/src/backend/optimizer/util/clauses.c
> > =C2=A0 > +++ b/src/backend/optimizer/util/clauses.c
> > =C2=A0 > @@ -935,6 +936,13 @@ max_parallel_hazard_walker(Node = *node, max_parallel_hazard_context *context)
> > =C2=A0 >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (param->paramk= ind =3D=3D PARAM_EXTERN)
> > =C2=A0 >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return= false;
> > =C2=A0 >
> > =C2=A0 > +=C2=A0 =C2=A0 =C2=A0 =C2=A0/* We doesn't support= passing session variables to workers */
> > =C2=A0 > +=C2=A0 =C2=A0 =C2=A0 =C2=A0if (param->paramkind = =3D=3D PARAM_VARIABLE)
> > =C2=A0 > +=C2=A0 =C2=A0 =C2=A0 =C2=A0{
> > =C2=A0 > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (max_par= allel_hazard_test(PROPARALLEL_RESTRICTED, context))
> > =C2=A0 > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0return true;
> > =C2=A0 > +=C2=A0 =C2=A0 =C2=A0 =C2=A0}
> >
> > =C2=A0 Even if a later patch alleviates that restriction, this pa= tch should document that
> > =C2=A0 session variables imply "parallel restricted".= =C2=A0 I have added that to doc/src/sgml/parallel.sgml.
> >
>
> merged (and removed in 0015)

Thanks.


I hope I can do some more review at some point in the future...

I sincerely hope that this patch set gets merged at some point.
The big obstacle is that it is so large.=C2=A0 That's probably because = it is pretty
feature-complete (but, as we have found, not totally free of bugs).

Any feature that builds its own system catalog= cannot be short. The first two patches have 300KB, like all others and jus= t basic support for reading and writing. All other features have 300KB. On = second hand almost all code is simple, and there are no changes in critical= parts of Postgres. The size and complexity of JSONPath is level higher. I = can throw 200KB from another 300KB patch set which can be better for review= , but it can be harder to maintain this patch set. I'll try it, and I&#= 39;ll send a reduced version.


Judging from the amount of time I put into my review so far, I guess that t= his
patch set would keep a committer busy for several weeks.=C2=A0 Perhaps the = only way to
get this done would be to make you a committer...

=
:-)
=C2=A0
Unfortunately, I am very bad = at languages, I had a lot of problems in basic school just with Czech lang,= Russian and English was more terrible) - so I very appreciate your work on= this patch.

Thank you very much
Pavel


Yours,
Laurenz Albe
--0000000000008679440620a4b1de--