Thread: [PATCH] Implement INSERT SET syntax
Hello, Attached is a patch that adds the option of using SET clause to specify the columns and values in an INSERT statement in the same manner as that of an UPDATE statement. A simple example that uses SET instead of a VALUES() clause: INSERT INTO t SET c1 = 'foo', c2 = 'bar', c3 = 'baz'; Values may also be sourced from a CTE using a FROM clause: WITH x AS ( SELECT 'foo' AS c1, 'bar' AS c2, 'baz' AS c3 ) INSERT INTO t SET c1 = x.c1, c2 = x.c2, c3 = x.c3 FROM x; The advantage of using the SET clause style is that the column and value are kept together, which can make changing or removing a column or value from a large list easier. Internally the grammar parser converts INSERT SET without a FROM clause into the equivalent INSERT with a VALUES clause. When using a FROM clause it becomes the equivalent of INSERT with a SELECT statement. There was a brief discussion regarding INSERT SET on pgsql-hackers in late August 2009 [1]. INSERT SET is not part of any SQL standard (that I am aware of), however this syntax is also implemented by MySQL [2]. Their implementation does not support specifying a FROM clause. Patch also contains regression tests and documentation. Regards, Gareth [1] https://www.postgresql.org/message-id/flat/2c5ef4e30908251010s46d9d566m1da21357891bab3d%40mail.gmail.com [2] https://dev.mysql.com/doc/refman/8.0/en/insert.html
Attachment
On Wed, Jul 17, 2019 at 7:30 AM Gareth Palmer <gareth@internetnz.net.nz> wrote:
Attached is a patch that adds the option of using SET clause to specify
the columns and values in an INSERT statement in the same manner as that
of an UPDATE statement.
Cool! Thanks for working on this, I'd love to see the syntax in PG.
There was a brief discussion regarding INSERT SET on pgsql-hackers in late
August 2009 [1].
There was also at least one slightly more recent adventure: https://www.postgresql.org/message-id/709e06c0-59c9-ccec-d216-21e38cb5ed61%40joh.to
You might want to check that thread too, in case any of the criticism there applies to this patch as well.
.m
Hi Marko, > On 17/07/2019, at 5:52 PM, Marko Tiikkaja <marko@joh.to> wrote: > > On Wed, Jul 17, 2019 at 7:30 AM Gareth Palmer <gareth@internetnz.net.nz> wrote: > Attached is a patch that adds the option of using SET clause to specify > the columns and values in an INSERT statement in the same manner as that > of an UPDATE statement. > > Cool! Thanks for working on this, I'd love to see the syntax in PG. > > There was a brief discussion regarding INSERT SET on pgsql-hackers in late > August 2009 [1]. > > There was also at least one slightly more recent adventure: https://www.postgresql.org/message-id/709e06c0-59c9-ccec-d216-21e38cb5ed61%40joh.to > > You might want to check that thread too, in case any of the criticism there applies to this patch as well. Thank-you for the pointer to that thread. I think my version avoids issue raised there by doing the conversion of the SET clause as part of the INSERT grammar rules. Gareth
Hello. At Thu, 18 Jul 2019 11:30:04 +1200, Gareth Palmer <gareth@internetnz.net.nz> wrote in <D50A93EB-11F3-4ED2-8192-0328DF901BBA@internetnz.net.nz> > Hi Marko, > > > On 17/07/2019, at 5:52 PM, Marko Tiikkaja <marko@joh.to> wrote: > > > > On Wed, Jul 17, 2019 at 7:30 AM Gareth Palmer <gareth@internetnz.net.nz> wrote: > > Attached is a patch that adds the option of using SET clause to specify > > the columns and values in an INSERT statement in the same manner as that > > of an UPDATE statement. > > > > Cool! Thanks for working on this, I'd love to see the syntax in PG. > > > > There was a brief discussion regarding INSERT SET on pgsql-hackers in late > > August 2009 [1]. > > > > There was also at least one slightly more recent adventure: https://www.postgresql.org/message-id/709e06c0-59c9-ccec-d216-21e38cb5ed61%40joh.to > > > > You might want to check that thread too, in case any of the criticism there applies to this patch as well. > > Thank-you for the pointer to that thread. > > I think my version avoids issue raised there by doing the conversion of the SET clause as part of the INSERT grammar rules. If I'm not missing something, "SELECT <targetlist>" without having FROM clause doesn't need to be tweaked. Thus insert_set_clause is useless and all we need here would be something like the following. (and the same for OVERRIDING.) + | SET set_clause_list from_clause + { + SelectStmt *n = makeNode(SelectStmt); + n->targetList = $2; + n->fromClause = $3; + $$ = makeNode(InsertStmt); + $$->selectStmt = (Node *)n; + $$->cols = $2; + } regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Hi Kyotaro, Thank-you for looking at the patch. > On 18/07/2019, at 6:54 PM, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote: > > Hello. > > If I'm not missing something, "SELECT <targetlist>" without > having FROM clause doesn't need to be tweaked. Thus > insert_set_clause is useless and all we need here would be > something like the following. (and the same for OVERRIDING.) > > + | SET set_clause_list from_clause > + { > + SelectStmt *n = makeNode(SelectStmt); > + n->targetList = $2; > + n->fromClause = $3; > + $$ = makeNode(InsertStmt); > + $$->selectStmt = (Node *)n; > + $$->cols = $2; > + } While that would mostly work, it would prevent setting the column to its default value using the DEFAULT keyword. Only expressions specified in valuesLists allow DEFAULT to be used. Those in targetList do not because transformInsertStmt() treats that as a general SELECT statement and the grammar does not allow the use of DEFAULT there. So this would generate a "DEFAULT is not allowed in this context" error if only targetList was used: INSERT INTO t set c1 = DEFAULT; Regards, Gareth > regards. > > -- > Kyotaro Horiguchi > NTT Open Source Software Center
Patch conflict with this assertion Assert(pstate->p_expr_kind == EXPR_KIND_UPDATE_SOURCE); src/backend/parser/parse_expr.c line 1570 The new status of this patch is: Waiting on Author
Hi Ibrar, > On 16/08/2019, at 7:14 AM, Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote: > > Patch conflict with this assertion > Assert(pstate->p_expr_kind == EXPR_KIND_UPDATE_SOURCE); > > src/backend/parser/parse_expr.c line 1570 > > The new status of this patch is: Waiting on Author Thank-you for reviewing the patch. Attached is version 2 of the patch that fixes the above by allowing p_expr_kind to be EXPR_KIND_VALUES_SINGLE as well. Gareth
Attachment
On Wed, Jul 17, 2019 at 10:00 AM Gareth Palmer <gareth@internetnz.net.nz> wrote: > > Hello, > > Attached is a patch that adds the option of using SET clause to specify > the columns and values in an INSERT statement in the same manner as that > of an UPDATE statement. > > A simple example that uses SET instead of a VALUES() clause: > > INSERT INTO t SET c1 = 'foo', c2 = 'bar', c3 = 'baz'; > > Values may also be sourced from a CTE using a FROM clause: > > WITH x AS ( > SELECT 'foo' AS c1, 'bar' AS c2, 'baz' AS c3 > ) > INSERT INTO t SET c1 = x.c1, c2 = x.c2, c3 = x.c3 FROM x; > > The advantage of using the SET clause style is that the column and value > are kept together, which can make changing or removing a column or value from > a large list easier. > > Internally the grammar parser converts INSERT SET without a FROM clause into > the equivalent INSERT with a VALUES clause. When using a FROM clause it becomes > the equivalent of INSERT with a SELECT statement. > > There was a brief discussion regarding INSERT SET on pgsql-hackers in late > August 2009 [1]. > > INSERT SET is not part of any SQL standard (that I am aware of), however this > syntax is also implemented by MySQL [2]. Their implementation does not support > specifying a FROM clause. > I think this can be a handy feature in some cases as pointed by you, but do we really want it for PostgreSQL? In the last round of discussions as pointed by you, there doesn't seem to be a consensus that we want this feature. I guess before spending too much time into reviewing this feature, we should first build a consensus on whether we need this. Along with users, I request some senior hackers/committers to also weigh in about the desirability of this feature. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
On Fri, Aug 16, 2019 at 8:19 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
On Wed, Jul 17, 2019 at 10:00 AM Gareth Palmer <gareth@internetnz.net.nz> wrote:
>
> Hello,
>
> Attached is a patch that adds the option of using SET clause to specify
> the columns and values in an INSERT statement in the same manner as that
> of an UPDATE statement.
>
> A simple example that uses SET instead of a VALUES() clause:
>
> INSERT INTO t SET c1 = 'foo', c2 = 'bar', c3 = 'baz';
>
> Values may also be sourced from a CTE using a FROM clause:
>
> WITH x AS (
> SELECT 'foo' AS c1, 'bar' AS c2, 'baz' AS c3
> )
> INSERT INTO t SET c1 = x.c1, c2 = x.c2, c3 = x.c3 FROM x;
>
> The advantage of using the SET clause style is that the column and value
> are kept together, which can make changing or removing a column or value from
> a large list easier.
>
> Internally the grammar parser converts INSERT SET without a FROM clause into
> the equivalent INSERT with a VALUES clause. When using a FROM clause it becomes
> the equivalent of INSERT with a SELECT statement.
>
> There was a brief discussion regarding INSERT SET on pgsql-hackers in late
> August 2009 [1].
>
> INSERT SET is not part of any SQL standard (that I am aware of), however this
> syntax is also implemented by MySQL [2]. Their implementation does not support
> specifying a FROM clause.
>
I think this can be a handy feature in some cases as pointed by you,
but do we really want it for PostgreSQL? In the last round of
discussions as pointed by you, there doesn't seem to be a consensus
that we want this feature. I guess before spending too much time into
reviewing this feature, we should first build a consensus on whether
we need this.
I agree with you Amit, that we need a consensus on that. Do we really need that
feature or not. In the previous discussion, there was no resistance to have that
in PostgreSQL, but some problem with the patch. Current patch is very simple
and not invasive, but still, we need a consensus on that.
Along with users, I request some senior hackers/committers to also
weigh in about the desirability of this feature.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Ibrar Ahmed
On 2019-08-16 05:19, Amit Kapila wrote: > I think this can be a handy feature in some cases as pointed by you, > but do we really want it for PostgreSQL? In the last round of > discussions as pointed by you, there doesn't seem to be a consensus > that we want this feature. I guess before spending too much time into > reviewing this feature, we should first build a consensus on whether > we need this. I think the problem this is attempting to solve is valid. What I don't like about the syntax is that it kind of breaks the notional processing model of INSERT in a fundamental way. The model is INSERT INTO $target $table_source where $table_source could be VALUES, SELECT, possibly others in theory. The proposed syntax changes this to only allow a single row to be specified via the SET syntax, and the SET syntax does not function as a row or table source in other contexts. Let's think about how we can achieve this using existing concepts in SQL. What we really need here at a fundamental level is an option to match $target to $table_source by column *name* rather than column *position*. There is existing syntax in SQL for that, namely a UNION b vs a UNION CORRESPONDING b I think this could be used for INSERT as well. And then you need a syntax to assign column names inside the VALUES rows. I think you could do either of the following: VALUES (a => 1, b => 2) or VALUES (1 AS a, 2 AS b) Another nice effect of this would be that you could so something like INSERT INTO tbl2 CORRESPONDING SELECT * FROM tbl1; which copies the contents of tbl1 to tbl2 if they have the same column names but allowing for a different column order. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 18/08/2019 11:03, Peter Eisentraut wrote: > > a UNION b > > vs > > a UNION CORRESPONDING b I have a WIP patch for CORRESPONDING [BY]. Is there any interest in me continuing it? If so, I'll start another thread for it. -- Vik Fearing
Vik Fearing <vik.fearing@2ndquadrant.com> writes: > On 18/08/2019 11:03, Peter Eisentraut wrote: >> a UNION b >> vs >> a UNION CORRESPONDING b > I have a WIP patch for CORRESPONDING [BY]. Is there any interest in me > continuing it? If so, I'll start another thread for it. CORRESPONDING is in the SQL standard, so in theory we ought to provide it. I think the hard question is how big/complicated the patch would be --- if the answer is "complicated", maybe it's not worth it. People have submitted patches for it before that didn't go anywhere, suggesting that the tradeoffs are not very good ... but maybe you'll think of a better way. regards, tom lane
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: > What I don't like about the syntax is that it kind of breaks the > notional processing model of INSERT in a fundamental way. Agreed. I really don't like that this only works for a VALUES-like case (and only the one-row form at that). It's hard to see it as anything but a wart pasted onto the syntax. > Let's think about how we can achieve this using existing concepts in > SQL. What we really need here at a fundamental level is an option to > match $target to $table_source by column *name* rather than column > *position*. There is existing syntax in SQL for that, namely > a UNION b > vs > a UNION CORRESPONDING b A potential issue here --- and something that applies to Vik's question as well, now that I think about it --- is that CORRESPONDING breaks down in the face of ALTER TABLE RENAME COLUMN. Something that had been a legal query before the rename might be invalid, or mean something quite different, afterwards. This is really nasty for stored views/rules, because we have neither a mechanism for forbidding input-table renames nor a mechanism for revalidating views/rules afterwards. Maybe we could make it go by resolving CORRESPONDING in the rewriter or planner, rather than in parse analysis; but that seems quite unpleasant as well. Changing our conclusions about the data types coming out of a UNION really shouldn't happen later than parse analysis. The SET-style syntax doesn't have that problem, since it's explicit about which values go into which columns. Perhaps the way to resolve Peter's objection is to make the syntax more fully like UPDATE: INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z (with the patch as-submitted corresponding to the case with an empty FROM clause, hence no variables in the expressions-to-be-assigned). Of course, this is not functionally distinct from INSERT INTO target(c1,c2,...) SELECT x, y+z, ... FROM tables-providing-x-y-z and it's fair to question whether it's worth supporting a nonstandard syntax just to allow the target column names to be written closer to the expressions-to-be-assigned. regards, tom lane
Hi Tom, > On 19/08/2019, at 3:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: >> What I don't like about the syntax is that it kind of breaks the >> notional processing model of INSERT in a fundamental way. > > Agreed. I really don't like that this only works for a VALUES-like case > (and only the one-row form at that). It's hard to see it as anything > but a wart pasted onto the syntax. > >> Let's think about how we can achieve this using existing concepts in >> SQL. What we really need here at a fundamental level is an option to >> match $target to $table_source by column *name* rather than column >> *position*. There is existing syntax in SQL for that, namely >> a UNION b >> vs >> a UNION CORRESPONDING b > > A potential issue here --- and something that applies to Vik's question > as well, now that I think about it --- is that CORRESPONDING breaks down > in the face of ALTER TABLE RENAME COLUMN. Something that had been a > legal query before the rename might be invalid, or mean something quite > different, afterwards. This is really nasty for stored views/rules, > because we have neither a mechanism for forbidding input-table renames > nor a mechanism for revalidating views/rules afterwards. Maybe we could > make it go by resolving CORRESPONDING in the rewriter or planner, rather > than in parse analysis; but that seems quite unpleasant as well. > Changing our conclusions about the data types coming out of a UNION > really shouldn't happen later than parse analysis. > > The SET-style syntax doesn't have that problem, since it's explicit > about which values go into which columns. > > Perhaps the way to resolve Peter's objection is to make the syntax > more fully like UPDATE: > > INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z > > (with the patch as-submitted corresponding to the case with an empty > FROM clause, hence no variables in the expressions-to-be-assigned). > > Of course, this is not functionally distinct from > > INSERT INTO target(c1,c2,...) SELECT x, y+z, ... FROM tables-providing-x-y-z > > and it's fair to question whether it's worth supporting a nonstandard > syntax just to allow the target column names to be written closer to > the expressions-to-be-assigned. Thanks for the feedback. Attached is version 3 of the patch that makes the syntax work more like an UPDATE statement when a FROM clause is used. So, an updated summary of the new syntax is: 1. Equivalent to VALUES(...): INSERT INTO t SET c1 = x, c2 = y, c3 = z; 2. Equivalent to INSERT INTO ... SELECT ...: INSERT INTO t SET c1 = sum(x.c1) FROM x WHERE x.c1 < y AND x.c2 != z GROUP BY x.c3 ORDER BY x.c4 ASC LIMIT a OFFSET b; Gareth > regards, tom lane
Attachment
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation: not tested Patch looks to me and works on my machine 73025140885c889410b9bfc4a30a3866396fc5db - HEAD I have not reviewed the documentaionchanges. The new status of this patch is: Ready for Committer
On Sun, Aug 18, 2019 at 11:00 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Perhaps the way to resolve Peter's objection is to make the syntax > more fully like UPDATE: > > INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z > > (with the patch as-submitted corresponding to the case with an empty > FROM clause, hence no variables in the expressions-to-be-assigned). > > Of course, this is not functionally distinct from > > INSERT INTO target(c1,c2,...) SELECT x, y+z, ... FROM tables-providing-x-y-z > > and it's fair to question whether it's worth supporting a nonstandard > syntax just to allow the target column names to be written closer to > the expressions-to-be-assigned. For what it's worth, I think this would be useful enough to justify its existence. Back in days of yore when dragons roamed the earth and I wrote database-driven applications instead of hacking on the database itself, I often wondered why I had to write two completely-different looking SQL statements, one to insert the data which a user had entered into a webform into the database, and another to update previously-entered data. This feature would allow those queries to be written in the same way, which would have pleased me, back in the day. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, Nov 1, 2019 at 6:31 PM Robert Haas <robertmhaas@gmail.com> wrote:
On Sun, Aug 18, 2019 at 11:00 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Perhaps the way to resolve Peter's objection is to make the syntax
> more fully like UPDATE:
>
> INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z
>
> (with the patch as-submitted corresponding to the case with an empty
> FROM clause, hence no variables in the expressions-to-be-assigned).
>
> Of course, this is not functionally distinct from
>
> INSERT INTO target(c1,c2,...) SELECT x, y+z, ... FROM tables-providing-x-y-z
>
> and it's fair to question whether it's worth supporting a nonstandard
> syntax just to allow the target column names to be written closer to
> the expressions-to-be-assigned.
For what it's worth, I think this would be useful enough to justify
its existence. Back in days of yore when dragons roamed the earth and
I wrote database-driven applications instead of hacking on the
database itself, I often wondered why I had to write two
completely-different looking SQL statements, one to insert the data
which a user had entered into a webform into the database, and another
to update previously-entered data. This feature would allow those
queries to be written in the same way, which would have pleased me,
back in the day.
I still do, and this would be a big help. I don't care if it's non-standard.
.m
Gareth Palmer <gareth@internetnz.net.nz> writes: >> On 19/08/2019, at 3:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Perhaps the way to resolve Peter's objection is to make the syntax >> more fully like UPDATE: >> INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z >> (with the patch as-submitted corresponding to the case with an empty >> FROM clause, hence no variables in the expressions-to-be-assigned). > Thanks for the feedback. Attached is version 3 of the patch that makes > the syntax work more like an UPDATE statement when a FROM clause is used. Since nobody has objected to this, I'm supposing that there's general consensus that that design sketch is OK, and we can move on to critiquing implementation details. I took a look, and didn't like much of what I saw. * In the grammar, there's no real need to have separate productions for the cases with FROM and without. The way you have it is awkward, and it arbitrarily rejects combinations that work fine in plain SELECT, such as WHERE without FROM. You should just do insert_set_clause: SET set_clause_list from_clause where_clause group_clause having_clause window_clause opt_sort_clause opt_select_limit relying on the ability of all those symbols (except set_clause_list) to reduce to empty. * This is randomly inconsistent with select_no_parens, and not in a good way, because you've omitted the option that's actually most likely to be useful, namely for_locking_clause. I wonder whether it's practical to refactor select_no_parens so that the stuff involving optional trailing clauses can be separated out into a production that insert_set_clause could also use. Might not be worth the trouble, but I'm concerned about select_no_parens growing additional clauses that we then forget to also add to insert_set_clause. * I'm not sure if it's worth also refactoring simple_select so that the "into_clause ... window_clause" business could be shared. But it'd likely be a good idea to at least have a comment there noting that any changes in that production might need to be applied to insert_set_clause as well. * In kind of the same vein, it feels like the syntax documentation is awkwardly failing to share commonality that it ought to be able to share with the SELECT man page. * I dislike the random hacking you did in transformMultiAssignRef. That weakens a useful check for error cases, and it's far from clear why the new assertion is OK. It also raises the question of whether this is really the only place you need to touch in parse analysis. Perhaps it'd be better to consider inventing new EXPR_KIND_ values for this situation; you'd then have to run around and look at all the existing EXPR_KIND uses, but that seems like a useful cross-check activity anyway. Or maybe we need to take two steps back and understand why that change is needed at all. I'd imagined that this patch would be only syntactic sugar for something you can do already, so it's not quite clear to me why we need additional changes. (If it's *not* just syntactic sugar, then the scope of potential problems becomes far greater, eg does ruleutils.c need to know how to reconstruct a valid SQL command from a querytree like this. If we're not touching ruleutils.c, we need to be sure that every command that can be written this way can be written old-style.) * Other documentation gripes: the lone example seems insufficient, and there needs to be an entry under COMPATIBILITY pointing out that this is not per SQL spec. * Some of the test cases seem to be expensively repeating construction/destruction of tables that they could have shared with existing test cases. I do not consider it a virtue for new tests added to an existing test script to be resolutely independent of what's already in that script. I'm setting this back to Waiting on Author. regards, tom lane
On Thu, Nov 14, 2019 at 9:20 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Gareth Palmer <gareth@internetnz.net.nz> writes:
>> On 19/08/2019, at 3:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Perhaps the way to resolve Peter's objection is to make the syntax
>> more fully like UPDATE:
>> INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z
>> (with the patch as-submitted corresponding to the case with an empty
>> FROM clause, hence no variables in the expressions-to-be-assigned).
> Thanks for the feedback. Attached is version 3 of the patch that makes
> the syntax work more like an UPDATE statement when a FROM clause is used.
Since nobody has objected to this, I'm supposing that there's general
consensus that that design sketch is OK, and we can move on to critiquing
implementation details. I took a look, and didn't like much of what I saw.
...
I'm setting this back to Waiting on Author.
regards, tom lane
Regarding syntax and considering that it makes INSERT look like UPDATE: there is another difference between INSERT and UPDATE. INSERT allows SELECT with ORDER BY and OFFSET/LIMIT (or FETCH FIRST), e.g.:
INSERT INTO t (a,b)
SELECT a+10. b+10
FROM t
ORDER BY a
LIMIT 3;
But UPDATE doesn't. I suppose the proposed behaviour of INSERT .. SET will be the same as standard INSERT. So we'll need a note for the differences between INSERT/SET and UPDATE/SET syntax.
On a related not, column aliases can be used in ORDER BY, e.g:
insert into t (a, b)
select
select
a + 20,
b - 2 * a as f
from t
order by f desc
limit 3 ;
from t
order by f desc
limit 3 ;
Would that be expressed as follows?:
insert into t
set
set
a = a + 20,
b = b - 2 * a as f
from t
order by f desc
limit 3 ;
from t
order by f desc
limit 3 ;
Best regards,
Pantelis Theodosiou
Pantelis Theodosiou <ypercube@gmail.com> writes: > On 19/08/2019, at 3:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Perhaps the way to resolve Peter's objection is to make the syntax >>> more fully like UPDATE: >>> INSERT INTO target SET c1 = x, c2 = y+z, ... FROM >>> tables-providing-x-y-z > Regarding syntax and considering that it makes INSERT look like UPDATE: > there is another difference between INSERT and UPDATE. INSERT allows SELECT > with ORDER BY and OFFSET/LIMIT (or FETCH FIRST), e.g.: ... > But UPDATE doesn't. I suppose the proposed behaviour of INSERT .. SET will > be the same as standard INSERT. So we'll need a note for the differences > between INSERT/SET and UPDATE/SET syntax. I was supposing that this syntax should be just another way to spell INSERT INTO target (columnlist) SELECT ... So everything past FROM would work exactly like it does in SELECT. > On a related not, column aliases can be used in ORDER BY, e.g: As proposed, there's no option equivalent to writing output-column aliases in the INSERT ... SELECT form, so the question doesn't come up. regards, tom lane
> On 15/11/2019, at 10:20 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Gareth Palmer <gareth@internetnz.net.nz> writes: >>> On 19/08/2019, at 3:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Perhaps the way to resolve Peter's objection is to make the syntax >>> more fully like UPDATE: >>> INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z >>> (with the patch as-submitted corresponding to the case with an empty >>> FROM clause, hence no variables in the expressions-to-be-assigned). > >> Thanks for the feedback. Attached is version 3 of the patch that makes >> the syntax work more like an UPDATE statement when a FROM clause is used. > > Since nobody has objected to this, I'm supposing that there's general > consensus that that design sketch is OK, and we can move on to critiquing > implementation details. I took a look, and didn't like much of what I saw. > > * In the grammar, there's no real need to have separate productions > for the cases with FROM and without. The way you have it is awkward, > and it arbitrarily rejects combinations that work fine in plain > SELECT, such as WHERE without FROM. You should just do > > insert_set_clause: > SET set_clause_list from_clause where_clause > group_clause having_clause window_clause opt_sort_clause > opt_select_limit > > relying on the ability of all those symbols (except set_clause_list) to > reduce to empty. There are two separate productions to match the two different types of inserts: INSERT with VALUES and INSERT with SELECT. The former has to store the the values in valuesLists so that DEFAULT can still be used. Allowing a WHERE without a FROM also mean that while this would work: INSERT INTO t SET c = DEFAULT; But this would fail with 'DEFAULT is not allowed in this context': INSERT INTO t SET c = DEFAULT WHERE true; I should have put a comment explaining why there are two rules. It could be combined into one production but there would have to be a check that $4 .. $9 are NULL to determine what type of INSERT to use. transformInsertStmt() also has an optimisation for the case of a single valueLists entry. > * This is randomly inconsistent with select_no_parens, and not in a > good way, because you've omitted the option that's actually most likely > to be useful, namely for_locking_clause. I wonder whether it's practical > to refactor select_no_parens so that the stuff involving optional trailing > clauses can be separated out into a production that insert_set_clause > could also use. Might not be worth the trouble, but I'm concerned > about select_no_parens growing additional clauses that we then forget > to also add to insert_set_clause. > > * I'm not sure if it's worth also refactoring simple_select so that > the "into_clause ... window_clause" business could be shared. But > it'd likely be a good idea to at least have a comment there noting > that any changes in that production might need to be applied to > insert_set_clause as well. I can add opt_for_locking_clause and a comment to simple_select to start with while the format of insert_set_clause is still being worked out. > * In kind of the same vein, it feels like the syntax documentation > is awkwardly failing to share commonality that it ought to be > able to share with the SELECT man page. I could collapse the from clause to just '[ FROM from_clause ]' and have it refer to the from clause and everything after it in SELECT. > * I dislike the random hacking you did in transformMultiAssignRef. > That weakens a useful check for error cases, and it's far from clear > why the new assertion is OK. It also raises the question of whether > this is really the only place you need to touch in parse analysis. > Perhaps it'd be better to consider inventing new EXPR_KIND_ values > for this situation; you'd then have to run around and look at all the > existing EXPR_KIND uses, but that seems like a useful cross-check > activity anyway. Or maybe we need to take two steps back and > understand why that change is needed at all. I'd imagined that this > patch would be only syntactic sugar for something you can do already, > so it's not quite clear to me why we need additional changes. > > (If it's *not* just syntactic sugar, then the scope of potential > problems becomes far greater, eg does ruleutils.c need to know > how to reconstruct a valid SQL command from a querytree like this. > If we're not touching ruleutils.c, we need to be sure that every > command that can be written this way can be written old-style.) It was intended to just be syntatic sugar, however because set_clause_list is being re-used the ability to do multi-assignment in an INSERT's targetList 'came along for the ride' which has no equivalent in the current INSERT syntax. That would be why those EXPR_KIND's are now appearing in transformMultiAssignRef(). There are 3 things that could be done here: 1. Update ruletutils.c to emit INSERT SET in get_insert_query_def() if query->hasSubLinks is true. 2. Add a new production similar to set_clause_list which doesn't allow multi-assignment. 3. Re-use set_clause_list but reject targetLists that contain multi-assignment. Keeping that feature is probably desirable at least for consistency with other SET clauses. I will work on getting get_insert_query_def() to correctly reconstruct the new syntax. > * Other documentation gripes: the lone example seems insufficient, > and there needs to be an entry under COMPATIBILITY pointing out > that this is not per SQL spec. I will add something to in the compatibility section. > * Some of the test cases seem to be expensively repeating > construction/destruction of tables that they could have shared with > existing test cases. I do not consider it a virtue for new tests > added to an existing test script to be resolutely independent of > what's already in that script. Those test cases will be changed to share the those tables. > I'm setting this back to Waiting on Author. > > regards, tom lane
> On 15/11/2019, at 10:20 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Gareth Palmer <gareth@internetnz.net.nz> writes: >>> On 19/08/2019, at 3:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Perhaps the way to resolve Peter's objection is to make the syntax >>> more fully like UPDATE: >>> INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z >>> (with the patch as-submitted corresponding to the case with an empty >>> FROM clause, hence no variables in the expressions-to-be-assigned). > >> Thanks for the feedback. Attached is version 3 of the patch that makes >> the syntax work more like an UPDATE statement when a FROM clause is used. > > Since nobody has objected to this, I'm supposing that there's general > consensus that that design sketch is OK, and we can move on to critiquing > implementation details. I took a look, and didn't like much of what I saw. > > * In the grammar, there's no real need to have separate productions > for the cases with FROM and without. The way you have it is awkward, > and it arbitrarily rejects combinations that work fine in plain > SELECT, such as WHERE without FROM. You should just do > > insert_set_clause: > SET set_clause_list from_clause where_clause > group_clause having_clause window_clause opt_sort_clause > opt_select_limit > > relying on the ability of all those symbols (except set_clause_list) to > reduce to empty. > > * This is randomly inconsistent with select_no_parens, and not in a > good way, because you've omitted the option that's actually most likely > to be useful, namely for_locking_clause. I wonder whether it's practical > to refactor select_no_parens so that the stuff involving optional trailing > clauses can be separated out into a production that insert_set_clause > could also use. Might not be worth the trouble, but I'm concerned > about select_no_parens growing additional clauses that we then forget > to also add to insert_set_clause. > > * I'm not sure if it's worth also refactoring simple_select so that > the "into_clause ... window_clause" business could be shared. But > it'd likely be a good idea to at least have a comment there noting > that any changes in that production might need to be applied to > insert_set_clause as well. > > * In kind of the same vein, it feels like the syntax documentation > is awkwardly failing to share commonality that it ought to be > able to share with the SELECT man page. > > * I dislike the random hacking you did in transformMultiAssignRef. > That weakens a useful check for error cases, and it's far from clear > why the new assertion is OK. It also raises the question of whether > this is really the only place you need to touch in parse analysis. > Perhaps it'd be better to consider inventing new EXPR_KIND_ values > for this situation; you'd then have to run around and look at all the > existing EXPR_KIND uses, but that seems like a useful cross-check > activity anyway. Or maybe we need to take two steps back and > understand why that change is needed at all. I'd imagined that this > patch would be only syntactic sugar for something you can do already, > so it's not quite clear to me why we need additional changes. > > (If it's *not* just syntactic sugar, then the scope of potential > problems becomes far greater, eg does ruleutils.c need to know > how to reconstruct a valid SQL command from a querytree like this. > If we're not touching ruleutils.c, we need to be sure that every > command that can be written this way can be written old-style.) So it appears as though it may not require any changes to ruleutils.c as the parser is converting the multi-assignments into separate columns, eg: CREATE RULE r1 AS ON INSERT TO tab1 DO INSTEAD INSERT INTO tab2 SET (col2, col1) = (new.col2, 0), col3 = tab3.col3 FROM tab3 The rule generated is: r1 AS ON INSERT TO tab1 DO INSTEAD INSERT INTO tab2 (col2, col1, col3) SELECT new.col2, 0 AS col1, tab3.col3 FROM tab3 It will trigger that Assert() though, as EXPR_KIND_SELECT_TARGET is now also being passed to transformMultiassignRef(). > * Other documentation gripes: the lone example seems insufficient, > and there needs to be an entry under COMPATIBILITY pointing out > that this is not per SQL spec. > > * Some of the test cases seem to be expensively repeating > construction/destruction of tables that they could have shared with > existing test cases. I do not consider it a virtue for new tests > added to an existing test script to be resolutely independent of > what's already in that script. > > I'm setting this back to Waiting on Author. > > regards, tom lane
> On 19/11/2019, at 5:05 PM, Gareth Palmer <gareth@internetnz.net.nz> wrote: >> >> Since nobody has objected to this, I'm supposing that there's general >> consensus that that design sketch is OK, and we can move on to critiquing >> implementation details. I took a look, and didn't like much of what I saw. Attached is an updated patch with for_locking_clause added, test-cases re-use existing tables and the comments and documentation have been expanded. >> I'm setting this back to Waiting on Author.
Attachment
On Fri, Nov 22, 2019 at 12:24:15PM +1300, Gareth Palmer wrote: > Attached is an updated patch with for_locking_clause added, test-cases > re-use existing tables and the comments and documentation have been > expanded. Per the automatic patch tester, documentation included in the patch does not build. Could you please fix that? I have moved the patch to next CF, waiting on author. -- Michael
Attachment
On Sun, Dec 1, 2019 at 4:32 PM Michael Paquier <michael@paquier.xyz> wrote: > > On Fri, Nov 22, 2019 at 12:24:15PM +1300, Gareth Palmer wrote: > > Attached is an updated patch with for_locking_clause added, test-cases > > re-use existing tables and the comments and documentation have been > > expanded. > > Per the automatic patch tester, documentation included in the patch > does not build. Could you please fix that? I have moved the patch to > next CF, waiting on author. Attached is a fixed version. > -- > Michael
Attachment
Hi Tom, On 12/3/19 4:44 AM, Gareth Palmer wrote: > On Sun, Dec 1, 2019 at 4:32 PM Michael Paquier <michael@paquier.xyz> wrote: >> >> On Fri, Nov 22, 2019 at 12:24:15PM +1300, Gareth Palmer wrote: >>> Attached is an updated patch with for_locking_clause added, test-cases >>> re-use existing tables and the comments and documentation have been >>> expanded. >> >> Per the automatic patch tester, documentation included in the patch >> does not build. Could you please fix that? I have moved the patch to >> next CF, waiting on author. > > Attached is a fixed version. Does this version of the patch address your concerns? Regards, -- -David david@pgmasters.net
David Steele <david@pgmasters.net> writes: > On 12/3/19 4:44 AM, Gareth Palmer wrote: >> Attached is a fixed version. > Does this version of the patch address your concerns? No. I still find the reliance on a FROM clause being present to be pretty arbitrary. Also, I don't believe that ruleutils.c requires no changes, because it's not going to be possible to transform every usage of this syntax to old-style. I tried to prove the point with this trivial example: regression=# create table foo (f1 int ,f2 int, f3 int); CREATE TABLE regression=# create table bar (f1 int ,f2 int, f3 int); CREATE TABLE regression=# create rule r1 as on insert to foo do instead regression-# insert into bar set (f1,f2,f3) = (select f1,f2,f3 from foo); intending to show that the rule decompilation was bogus, but I didn't get that far because the parser crashed: TRAP: FailedAssertion("pstate->p_multiassign_exprs == NIL", File: "parse_target.c", Line: 287) postgres: postgres regression [local] CREATE RULE(ExceptionalCondition+0x55)[0x8fb6e5] postgres: postgres regression [local] CREATE RULE[0x5bd0c3] postgres: postgres regression [local] CREATE RULE[0x583def] postgres: postgres regression [local] CREATE RULE(transformStmt+0x2d5)[0x582665] postgres: postgres regression [local] CREATE RULE(transformRuleStmt+0x2ad)[0x5bf2ad] postgres: postgres regression [local] CREATE RULE(DefineRule+0x17)[0x793847] If I do it like this, I get a different assertion: regression=# insert into bar set (f1,f2,f3) = (select f1,f2,f3) from foo; server closed the connection unexpectedly TRAP: FailedAssertion("exprKind == EXPR_KIND_UPDATE_SOURCE", File: "parse_target.c", Line: 209) postgres: postgres regression [local] INSERT(ExceptionalCondition+0x55)[0x8fb6e5] postgres: postgres regression [local] INSERT(transformTargetList+0x1a7)[0x5bd277] postgres: postgres regression [local] INSERT(transformStmt+0xbe0)[0x582f70] postgres: postgres regression [local] INSERT[0x5839f3] postgres: postgres regression [local] INSERT(transformStmt+0x2d5)[0x582665] postgres: postgres regression [local] INSERT(transformTopLevelStmt+0xd)[0x58411d] postgres: postgres regression [local] INSERT(parse_analyze+0x69)[0x584269] No doubt that's all fixable, but the realization that some cases of this syntax are *not* just syntactic sugar for standards-compliant syntax is giving me pause. Do we really want to get out front of the SQL committee on extending INSERT in an incompatible way? regards, tom lane
I wrote: > No doubt that's all fixable, but the realization that some cases of > this syntax are *not* just syntactic sugar for standards-compliant > syntax is giving me pause. Do we really want to get out front of > the SQL committee on extending INSERT in an incompatible way? One compromise that might be worth thinking about is to disallow multiassignments in this syntax, so as to (1) avoid the possibility of generating something that can't be represented by standard INSERT and (2) get something done in time for v13. The end of March is not that far off. Perhaps somebody would come back and extend it later, or perhaps not. A slightly more ambitious compromise would be to allow multiassignment only when the source can be pulled apart into independent subexpressions, comparable to the restriction we used to have in UPDATE itself (before 8f889b108 or thereabouts). In either case the transformation could be done right in gram.y and a helpful error thrown for unsupported cases. regards, tom lane
On 2020-03-24 18:57, Tom Lane wrote: > No doubt that's all fixable, but the realization that some cases of > this syntax are*not* just syntactic sugar for standards-compliant > syntax is giving me pause. Do we really want to get out front of > the SQL committee on extending INSERT in an incompatible way? What is the additional functionality that we are considering adding here? The thread started out proposing a more convenient syntax, but it seems to go deeper now and perhaps not everyone is following. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: > On 2020-03-24 18:57, Tom Lane wrote: >> No doubt that's all fixable, but the realization that some cases of >> this syntax are*not* just syntactic sugar for standards-compliant >> syntax is giving me pause. Do we really want to get out front of >> the SQL committee on extending INSERT in an incompatible way? > What is the additional functionality that we are considering adding here? > The thread started out proposing a more convenient syntax, but it seems > to go deeper now and perhaps not everyone is following. AIUI, the proposal is to allow INSERT commands to be written using an UPDATE-like syntax, for example INSERT INTO table SET col1 = value1, col2 = value2, ... [ FROM ... ] where everything after FROM is the same as it is in SELECT. My initial belief was that this was strictly equivalent to what you could do with a target-column-names list in standard INSERT, viz INSERT INTO table (col1, col2, ...) VALUES (value1, value2, ...); or INSERT INTO table (col1, col2, ...) SELECT value1, value2, ... FROM ... but it's arguably more legible/convenient because the column names are written next to their values. However, that rewriting falls down for certain multiassignment cases where you have a row source that can't be decomposed, such as my example INSERT INTO table SET (col1, col2) = (SELECT value1, value2 FROM ...), ... [ FROM ... ] So, just as we found for UPDATE, multiassignment syntax is strictly stronger than plain column-by-column assignment. There are some secondary issues about which variants of this syntax will allow a column value to be written as DEFAULT, and perhaps about whether set-returning functions work. But the major point right now is about whether its's possible to rewrite to standard syntax. regards, tom lane
> On 26/03/2020, at 3:17 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: >> On 2020-03-24 18:57, Tom Lane wrote: >>> No doubt that's all fixable, but the realization that some cases of >>> this syntax are*not* just syntactic sugar for standards-compliant >>> syntax is giving me pause. Do we really want to get out front of >>> the SQL committee on extending INSERT in an incompatible way? > >> What is the additional functionality that we are considering adding here? >> The thread started out proposing a more convenient syntax, but it seems >> to go deeper now and perhaps not everyone is following. > > AIUI, the proposal is to allow INSERT commands to be written > using an UPDATE-like syntax, for example > > INSERT INTO table SET col1 = value1, col2 = value2, ... [ FROM ... ] > > where everything after FROM is the same as it is in SELECT. My initial > belief was that this was strictly equivalent to what you could do with > a target-column-names list in standard INSERT, viz > > INSERT INTO table (col1, col2, ...) VALUES (value1, value2, ...); > or > INSERT INTO table (col1, col2, ...) SELECT value1, value2, ... FROM ... > > but it's arguably more legible/convenient because the column names > are written next to their values. > > However, that rewriting falls down for certain multiassignment cases > where you have a row source that can't be decomposed, such as my > example > > INSERT INTO table SET (col1, col2) = (SELECT value1, value2 FROM ...), > ... [ FROM ... ] > > So, just as we found for UPDATE, multiassignment syntax is strictly > stronger than plain column-by-column assignment. > > There are some secondary issues about which variants of this syntax > will allow a column value to be written as DEFAULT, and perhaps > about whether set-returning functions work. But the major point > right now is about whether its's possible to rewrite to standard > syntax. > > regards, tom lane Attached is v6 of the patch. As per the suggestion the SET clause list is checked for any MultiAssigmentRef nodes and to report an error if any are found. For example, the rule definition that previously caused a parser crash would now produce the following error: vagrant=> create rule r1 as on insert to foo do instead vagrant-> insert into bar set (f1,f2,f3) = (select f1,f2,f3 from foo); ERROR: INSERT SET syntax does not support multi-assignment of columns. LINE 2: insert into bar set (f1,f2,f3) = (select f1,f2,f3 from foo); ^ HINT: Specify the column assignments separately. Requiring a FROM clause was a way to differentiate between an INSERT with VALUES() which does allow DEFAULT and an INSERT with SELECT which does not. The idea was that it would help the user understand that they were writing a different type of query and that DEFAULT would not be allowed in that context. To show what it would look like without that requirement I have removed it from the v6 patch. In the first example works but the second one will generate an error. INSERT INTO t SET c1 = 1 WHERE true; INSERT INTO t SET c1 = DEFAULT WHERE true;
Attachment
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation: tested, passed It builds failed by applying to the latest code version, and I try head '73025140885c889410b9bfc4a30a3866396fc5db' which work well. The new status of this patch is: Waiting on Author
Hi Movead, > On 22/04/2020, at 2:40 PM, movead li <movead.li@highgo.ca> wrote: > > The following review has been posted through the commitfest application: > make installcheck-world: tested, passed > Implements feature: tested, passed > Spec compliant: tested, passed > Documentation: tested, passed > > It builds failed by applying to the latest code version, and I try head > '73025140885c889410b9bfc4a30a3866396fc5db' which work well. > > The new status of this patch is: Waiting on Author Thank you for the review, attached is v7 of the patch which should apply correcly to HEAD. This version now uses it's own production rule for the SET clause to avoid the issue with MultiAssigmentRef nodes in the targetList.
Attachment
> On 4/23/20 8:04 PM, Gareth Palmer wrote: > > > > Thank you for the review, attached is v7 of the patch which should > > apply correcly to HEAD. > > Hello Gareth, This patch no longer applies to HEAD, can you please submit a rebased version? Thanks, Rachel
Hello Rachel, On Wed, 22 Sept 2021 at 17:13, Rachel Heaton <rachelmheaton@gmail.com> wrote: > > > On 4/23/20 8:04 PM, Gareth Palmer wrote: > > > > > > Thank you for the review, attached is v7 of the patch which should > > > apply correcly to HEAD. > > > > > Hello Gareth, > > This patch no longer applies to HEAD, can you please submit a rebased version? Attached is a rebased version that should apply to HEAD. Gareth > Thanks, > Rachel > > > >
Attachment
Since this feature adds INSERT OVERRIDING SET syntax, it is recommended to add some related testcases. Regards Wenjing > 2021年9月22日 07:38,Rachel Heaton <rachelmheaton@gmail.com> 写道: > >> On 4/23/20 8:04 PM, Gareth Palmer wrote: >>> >>> Thank you for the review, attached is v7 of the patch which should >>> apply correcly to HEAD. >>> > > Hello Gareth, > > This patch no longer applies to HEAD, can you please submit a rebased version? > > Thanks, > Rachel > >
Attachment
Justin Pryzby <pryzby@telsasoft.com> writes: > You have to either include the pre-requisite patches as 0001, and your patch as > 0002 (as I'm doing now), or name your patch something other than *.diff or > *.patch, so cfbot doesn't think it's a new version of the patch to be tested. This patch has been basically ignored for a full two years now. (Remarkably, it's still passing in the cfbot.) I have to think that that means there's just not enough interest to justify committing it. Should we mark it rejected and move on? If not, what needs to happen to get it unstuck? regards, tom lane
On Wed, Mar 23, 2022 at 5:33 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Justin Pryzby <pryzby@telsasoft.com> writes: > > You have to either include the pre-requisite patches as 0001, and your patch as > > 0002 (as I'm doing now), or name your patch something other than *.diff or > > *.patch, so cfbot doesn't think it's a new version of the patch to be tested. > > This patch has been basically ignored for a full two years now. > (Remarkably, it's still passing in the cfbot.) > > I have to think that that means there's just not enough interest > to justify committing it. Should we mark it rejected and move on? > If not, what needs to happen to get it unstuck? I can help with review and/or other work here. Please give me a couple of weeks. .m
On Thu, Apr 7, 2022 at 11:29 AM Marko Tiikkaja <marko@joh.to> wrote: > I can help with review and/or other work here. Please give me a > couple of weeks. Hi Marko, did you get a chance to pick up this patchset? If not, no worries; I can mark this RwF and we can try again in a future commitfest. Thanks, --Jacob
Hello, Here is a new version of the patch that applies to HEAD. It also adds some regression tests for overriding {system,user} values based on Wenjing Zeng's work. Gareth On Thu, 14 Jul 2022 at 22:40, Jacob Champion <jchampion@timescale.com> wrote: > > On Thu, Apr 7, 2022 at 11:29 AM Marko Tiikkaja <marko@joh.to> wrote: > > I can help with review and/or other work here. Please give me a > > couple of weeks. > > Hi Marko, did you get a chance to pick up this patchset? If not, no > worries; I can mark this RwF and we can try again in a future > commitfest. > > Thanks, > --Jacob > > > >
Attachment
As discussed in [1], we're taking this opportunity to return some patchsets that don't appear to be getting enough reviewer interest. This is not a rejection, since we don't necessarily think there's anything unacceptable about the entry, but it differs from a standard "Returned with Feedback" in that there's probably not much actionable feedback at all. Rather than code changes, what this patch needs is more community interest. You might - ask people for help with your approach, - see if there are similar patches that your code could supplement, - get interested parties to agree to review your patch in a CF, or - possibly present the functionality in a way that's easier to review overall. (Doing these things is no guarantee that there will be interest, but it's hopefully better than endlessly rebasing a patchset that is not receiving any feedback from the community.) Once you think you've built up some community support and the patchset is ready for review, you (or any interested party) can resurrect the patch entry by visiting https://commitfest.postgresql.org/38/2218/ and changing the status to "Needs Review", and then changing the status again to "Move to next CF". (Don't forget the second step; hopefully we will have streamlined this in the near future!) Thanks, --Jacob [1] https://postgr.es/m/flat/0ab66589-2f71-69b3-2002-49e821740b0d@timescale.com