Re: Implement <null treatment> for window functions

On 11/12/20 11:35 PM, Krasiyan Andreev wrote:

Hi, it looks like Vik Fearing's patch does not apply anymore, because there
are many conflicts with recent changes, fixed patch attached.

Thanks! I've been away from this list for a while and I have some
catching up to do.
--
Vik Fearing

Fixed patch attached, after new introduced conflicts.
Vik, can you add it to the next commitfest, to be able to test it.
Also, all tests from Oliver Ford's old patch also passed successfully.

На пт, 13.11.2020 г. в 0:44 ч. Vik Fearing <vik@postgresfriends.org> написа:

On 11/21/20 10:07 AM, Krasiyan Andreev wrote:

Fixed patch attached, after new introduced conflicts.
Vik, can you add it to the next commitfest, to be able to test it.

I have done this now. Thanks!
--
Vik Fearing

Hi, after latest committed patches about multirange datatypes, I get a
compilation error,
when I try to apply a patch about respect/ignore null for window functions.
Without it applied, it complies clean and all checks are passed.

[krasiyan@localhost build]$ /home/krasiyan/pgsql/postgresql/configure
--with-openssl --with-libxml --with-libxslt --with-systemd --with-selinux
--with-perl --with-python --enable-cassert --prefix=/var/lib/pgsql/14
...
gcc -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Werror=vla -Wendif-labels
-Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type
-Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard
-Wno-format-truncation -Wno-stringop-truncation -O2 -I../../../src/include
-I/home/krasiyan/pgsql/postgresql/src/include -D_GNU_SOURCE
-I/usr/include/libxml2 -c -o typecmds.o
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c: In
function ‘makeMultirangeConstructors’:
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1849:9:
warning: passing argument 17 of ‘ProcedureCreate’ makes integer from
pointer without a cast [-Wint-conversion]
1849 | argtypes, /* parameterTypes */
| ^~~~~~~~
| |
| oidvector *
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:206:16: note:
expected ‘char’ but argument is of type ‘oidvector *’
206 | char parallel,
| ~~~~~^~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:32:
/home/krasiyan/pgsql/postgresql/src/include/postgres.h:556:29: warning:
passing argument 18 of ‘ProcedureCreate’ makes pointer from integer without
a cast [-Wint-conversion]
556 | #define PointerGetDatum(X) ((Datum) (X))
| ~^~~~~~~~~~~~
| |
| long unsigned int
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1850:9:
note: in expansion of macro ‘PointerGetDatum’
1850 | PointerGetDatum(NULL), /* allParameterTypes */
| ^~~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:207:22: note:
expected ‘oidvector *’ but argument is of type ‘long unsigned int’
207 | oidvector *parameterTypes,
| ~~~~~~~~~~~^~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/include/access/tupdesc.h:19,
from
/home/krasiyan/pgsql/postgresql/src/include/utils/relcache.h:18,
from
/home/krasiyan/pgsql/postgresql/src/include/access/genam.h:21,
from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:34:
/home/krasiyan/pgsql/postgresql/src/include/nodes/pg_list.h:65:19: warning:
passing argument 21 of ‘ProcedureCreate’ makes integer from pointer without
a cast [-Wint-conversion]
65 | #define NIL ((List *) NULL)
| ~^~~~~~~~~~~~~~
| |
| List *
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1853:9:
note: in expansion of macro ‘NIL’
1853 | NIL, /* parameterDefaults */
| ^~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:210:17: note:
expected ‘Datum’ {aka ‘long unsigned int’} but argument is of type ‘List *’
210 | Datum parameterNames,
| ~~~~~~^~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:32:
/home/krasiyan/pgsql/postgresql/src/include/postgres.h:556:29: warning:
passing argument 22 of ‘ProcedureCreate’ makes pointer from integer without
a cast [-Wint-conversion]
556 | #define PointerGetDatum(X) ((Datum) (X))
| ~^~~~~~~~~~~~
| |
| long unsigned int
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1854:9:
note: in expansion of macro ‘PointerGetDatum’
1854 | PointerGetDatum(NULL), /* trftypes */
| ^~~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:211:17: note:
expected ‘List *’ but argument is of type ‘long unsigned int’
211 | List *parameterDefaults,
| ~~~~~~^~~~~~~~~~~~~~~~~
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1833:11:
error: too few arguments to function ‘ProcedureCreate’
1833 | myself = ProcedureCreate(name, /* name: same as multirange type */
| ^~~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:190:22: note:
declared here
190 | extern ObjectAddress ProcedureCreate(const char *procedureName,
| ^~~~~~~~~~~~~~~
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1892:9:
warning: passing argument 17 of ‘ProcedureCreate’ makes integer from
pointer without a cast [-Wint-conversion]
1892 | argtypes, /* parameterTypes */
| ^~~~~~~~
| |
| oidvector *
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:206:16: note:
expected ‘char’ but argument is of type ‘oidvector *’
206 | char parallel,
| ~~~~~^~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:32:
/home/krasiyan/pgsql/postgresql/src/include/postgres.h:556:29: warning:
passing argument 18 of ‘ProcedureCreate’ makes pointer from integer without
a cast [-Wint-conversion]
556 | #define PointerGetDatum(X) ((Datum) (X))
| ~^~~~~~~~~~~~
| |
| long unsigned int
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1893:9:
note: in expansion of macro ‘PointerGetDatum’
1893 | PointerGetDatum(NULL), /* allParameterTypes */
| ^~~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:207:22: note:
expected ‘oidvector *’ but argument is of type ‘long unsigned int’
207 | oidvector *parameterTypes,
| ~~~~~~~~~~~^~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/include/access/tupdesc.h:19,
from
/home/krasiyan/pgsql/postgresql/src/include/utils/relcache.h:18,
from
/home/krasiyan/pgsql/postgresql/src/include/access/genam.h:21,
from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:34:
/home/krasiyan/pgsql/postgresql/src/include/nodes/pg_list.h:65:19: warning:
passing argument 21 of ‘ProcedureCreate’ makes integer from pointer without
a cast [-Wint-conversion]
65 | #define NIL ((List *) NULL)
| ~^~~~~~~~~~~~~~
| |
| List *
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1896:9:
note: in expansion of macro ‘NIL’
1896 | NIL, /* parameterDefaults */
| ^~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:210:17: note:
expected ‘Datum’ {aka ‘long unsigned int’} but argument is of type ‘List *’
210 | Datum parameterNames,
| ~~~~~~^~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:32:
/home/krasiyan/pgsql/postgresql/src/include/postgres.h:556:29: warning:
passing argument 22 of ‘ProcedureCreate’ makes pointer from integer without
a cast [-Wint-conversion]
556 | #define PointerGetDatum(X) ((Datum) (X))
| ~^~~~~~~~~~~~
| |
| long unsigned int
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1897:9:
note: in expansion of macro ‘PointerGetDatum’
1897 | PointerGetDatum(NULL), /* trftypes */
| ^~~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:211:17: note:
expected ‘List *’ but argument is of type ‘long unsigned int’
211 | List *parameterDefaults,
| ~~~~~~^~~~~~~~~~~~~~~~~
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1876:11:
error: too few arguments to function ‘ProcedureCreate’
1876 | myself = ProcedureCreate(name, /* name: same as multirange type */
| ^~~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:190:22: note:
declared here
190 | extern ObjectAddress ProcedureCreate(const char *procedureName,
| ^~~~~~~~~~~~~~~
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1932:9:
warning: passing argument 17 of ‘ProcedureCreate’ makes integer from
pointer without a cast [-Wint-conversion]
1932 | argtypes, /* parameterTypes */
| ^~~~~~~~
| |
| oidvector *
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:206:16: note:
expected ‘char’ but argument is of type ‘oidvector *’
206 | char parallel,
| ~~~~~^~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:32:
/home/krasiyan/pgsql/postgresql/src/include/postgres.h:556:29: warning:
passing argument 18 of ‘ProcedureCreate’ makes pointer from integer without
a cast [-Wint-conversion]
556 | #define PointerGetDatum(X) ((Datum) (X))
| ~^~~~~~~~~~~~
| |
| long unsigned int
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1933:9:
note: in expansion of macro ‘PointerGetDatum’
1933 | PointerGetDatum(allParameterTypes), /* allParameterTypes */
| ^~~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:207:22: note:
expected ‘oidvector *’ but argument is of type ‘long unsigned int’
207 | oidvector *parameterTypes,
| ~~~~~~~~~~~^~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/include/access/tupdesc.h:19,
from
/home/krasiyan/pgsql/postgresql/src/include/utils/relcache.h:18,
from
/home/krasiyan/pgsql/postgresql/src/include/access/genam.h:21,
from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:34:
/home/krasiyan/pgsql/postgresql/src/include/nodes/pg_list.h:65:19: warning:
passing argument 21 of ‘ProcedureCreate’ makes integer from pointer without
a cast [-Wint-conversion]
65 | #define NIL ((List *) NULL)
| ~^~~~~~~~~~~~~~
| |
| List *
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1936:9:
note: in expansion of macro ‘NIL’
1936 | NIL, /* parameterDefaults */
| ^~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:210:17: note:
expected ‘Datum’ {aka ‘long unsigned int’} but argument is of type ‘List *’
210 | Datum parameterNames,
| ~~~~~~^~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:32:
/home/krasiyan/pgsql/postgresql/src/include/postgres.h:556:29: warning:
passing argument 22 of ‘ProcedureCreate’ makes pointer from integer without
a cast [-Wint-conversion]
556 | #define PointerGetDatum(X) ((Datum) (X))
| ~^~~~~~~~~~~~
| |
| long unsigned int
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1937:9:
note: in expansion of macro ‘PointerGetDatum’
1937 | PointerGetDatum(NULL), /* trftypes */
| ^~~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:211:17: note:
expected ‘List *’ but argument is of type ‘long unsigned int’
211 | List *parameterDefaults,
| ~~~~~~^~~~~~~~~~~~~~~~~
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:1916:11:
error: too few arguments to function ‘ProcedureCreate’
1916 | myself = ProcedureCreate(name, /* name: same as multirange type */
| ^~~~~~~~~~~~~~~
In file included from
/home/krasiyan/pgsql/postgresql/src/backend/commands/typecmds.c:52:
/home/krasiyan/pgsql/postgresql/src/include/catalog/pg_proc.h:190:22: note:
declared here
190 | extern ObjectAddress ProcedureCreate(const char *procedureName,
| ^~~~~~~~~~~~~~~
make[3]: *** [<builtin>: typecmds.o] Error 1
make[3]: Leaving directory '/home/krasiyan/pgsql/build/src/backend/commands'
make[2]: *** [/home/krasiyan/pgsql/postgresql/src/backend/common.mk:39:
commands-recursive] Error 2
make[2]: Leaving directory '/home/krasiyan/pgsql/build/src/backend'
make[1]: *** [Makefile:42: all-backend-recurse] Error 2
make[1]: Leaving directory '/home/krasiyan/pgsql/build/src'
make: *** [GNUmakefile:11: all-src-recurse] Error 2
[krasiyan@localhost build]$

На вт, 8.12.2020 г. в 16:27 ч. Vik Fearing <vik@postgresfriends.org> написа:

On Wed, Dec 30, 2020 at 09:32:26PM +0200, Krasiyan Andreev wrote:

Hi, after latest committed patches about multirange datatypes, I get a
compilation error,

Oh, right. I'd been meaning to send a patch to fix that. Here it is.

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

It works - now it compiles clean and all checks are passed, thank you. I
will continue with more complex tests.

На ср, 30.12.2020 г. в 21:50 ч. David Fetter <david@fetter.org> написа:

Hi, it looks like cfbot.cputube.org didn't recognize and can't apply a
patch, so I resend it now with a different format, no other changes.

На ср, 30.12.2020 г. в 22:16 ч. Krasiyan Andreev <krasiyan@gmail.com>
написа:

Krasiyan:
Happy New Year.

For WinGetFuncArgInPartition():

+           if (target > 0)
+               step = 1;
+           else if (target < 0)
+               step = -1;
+           else
+               step = 0;

When would the last else statement execute ? Since the above code is
for WINDOW_SEEK_CURRENT, I wonder why step should be 0.

Similar question for the last else statement below
in WinGetFuncArgInFrame():

+       else if (seektype == WINDOW_SEEK_TAIL)
+           step = -1;
+       else
+           step = 0;

Thanks

On Fri, Jan 1, 2021 at 12:59 PM Krasiyan Andreev <krasiyan@gmail.com> wrote:

On Fri, Jan 01, 2021 at 01:21:10PM -0800, Zhihong Yu wrote:

Krasiyan:
Happy New Year.

For WinGetFuncArgInPartition():

+           if (target > 0)
+               step = 1;
+           else if (target < 0)
+               step = -1;
+           else
+               step = 0;

When would the last else statement execute ? Since the above code is
for WINDOW_SEEK_CURRENT, I wonder why step should be 0.

If it does actually need step to be one of those three choices, it
might be shorter (well, less branchy) to write as

step = (target > 0) - (target < 0);

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On 1/1/21 10:21 PM, Zhihong Yu wrote:

Krasiyan:
Happy New Year.

For WinGetFuncArgInPartition():

+           if (target > 0)
+               step = 1;
+           else if (target < 0)
+               step = -1;
+           else
+               step = 0;

When would the last else statement execute ? Since the above code is
for WINDOW_SEEK_CURRENT, I wonder why step should be 0.

Hi.

"lag(expr, 0) over w" is useless but valid.
--
Vik Fearing

Hi, the building warning below is fixed now, no other changes. Also, I can
confirm that the corner case with offset=0 in lead and lag works correctly.

gcc -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Werror=vla -Wendif-labels
-Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type
-Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard
-Wno-format-truncation -Wno-stringop-truncation -O2 -I../../../src/include
-I/home/krasiyan/pgsql/postgresql/src/include -D_GNU_SOURCE
-I/usr/include/libxml2 -c -o nodeWindowAgg.o
/home/krasiyan/pgsql/postgresql/src/backend/executor/nodeWindowAgg.c
/home/krasiyan/pgsql/postgresql/src/backend/executor/nodeWindowAgg.c: In
function ‘WinGetFuncArgInPartition’:
/home/krasiyan/pgsql/postgresql/src/backend/executor/nodeWindowAgg.c:3274:10:
warning: ‘step’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
3274 | relpos += step;
| ~~~~~~~^~~~~~~
/home/krasiyan/pgsql/postgresql/src/backend/executor/nodeWindowAgg.c: In
function ‘WinGetFuncArgInFrame’:
/home/krasiyan/pgsql/postgresql/src/backend/executor/nodeWindowAgg.c:3531:10:
warning: ‘step’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
3531 | relpos += step;
| ~~~~~~~^~~~~~~

На пт, 8.01.2021 г. в 2:02 ч. Vik Fearing <vik@postgresfriends.org> написа:

Hi, patch applies and compiles, all included and external tests and
building of the docs pass.
After the last run of the cfbot, there are no any building warnings.
I am using last version in our testing environment with real data and I
didn't find any bugs,
so I'm marking this patch as ready for the committer in the commitfest app.

На сб, 9.01.2021 г. в 13:30 ч. Krasiyan Andreev <krasiyan@gmail.com> написа:

Hi,
For WinGetFuncArgInFrame():

+   if (winobj->null_treatment == NULL_TREATMENT_IGNORE)
    {
...
+       if (target > 0)
+           step = 1;
+       else if (target < 0)
+           step = -1;
+       else if (seektype == WINDOW_SEEK_HEAD)
+           step = 1;
+       else if (seektype == WINDOW_SEEK_TAIL)
+           step = -1;
+       else
+           step = 0;
...
+       relpos = 0;
+   }

Why is relpos always set to 0 ?
In similar code in WinGetFuncArgInPartition(), I saw the following:

+           if (target > 0)
+               step = 1;
+           else if (target < 0)
+               step = -1;
+           else
+               step = 0;
+           relpos = step;

Maybe add a comment above the relpos assignment.

Thanks

On Sat, Jan 9, 2021 at 3:31 AM Krasiyan Andreev <krasiyan@gmail.com> wrote:

I started to look through this patch, and the first thing I'm wondering
about is why bother with a new pg_proc column, ie why not just apply the
behavior to any window function? The fact that the SQL committee
restricts this syntax to a few window functions is just their usual
design tic of creating one-off syntax that could be much more general.
We've not hesitated to generalize in similar situations in the past.

The main thing I can see against that position is that it's not very
clear what to do if the window function has more than one window-ized
argument --- or at least, the only plausible interpretation would be
to ignore rows in which any of those arguments is null, which this
implementation is incapable of doing (since we don't know exactly
which arguments the function will try to treat as window-ized).
However, having a pronulltreatment column isn't helping that
situation at all: somebody could mark a multi-input window function
as ignoring nulls, and we'd silently do the wrong thing in any case
where those inputs weren't nulls at exactly the same rows.

My thought, therefore, is to drop pg_proc.pronulltreatment and instead
enforce an implementation restriction that when IGNORE NULLS is specified,
WinGetFuncArgInPartition and WinGetFuncArgInFrame throw an error if
asked about any argument position other than the first one. As long
as only the first argument is window-ized, the implementation you have
here will act correctly. If anybody ever finds that annoying, they can
figure out how to relax the restriction at that time.

The need for a TREAT NULLS option to CREATE FUNCTION would thereby
also go away, which is good because I don't think this patch has
fully implemented that (notably, I don't see any pg_dump changes).

As far as the actual implementation goes:

* The undocumented relationship between "relpos" (which used to be
constant and now isn't) and "target" and "step" makes my head hurt.
I'm sure this could be redesigned to be simpler, or if not, at
least it should be commented a lot more thoroughly.

* I'm quite concerned about performance; it looks like this will
degrade to O(N^2) in practical situations, which isn't going to
satisfy anyone. I think we need to track how many nulls we've
already seen so that we aren't re-visiting earlier rows over and
over. That should make it possible to un-disable the set_mark
optimization, which is something that's independently catastrophic
for performance. While I've not stopped to design this fully, maybe
we could keep state along the lines of "there are j rows with null
values of the window-ized argument before row k of the partition."
Updating that by dead reckoning as we navigate would be enough to
fix the O(N^2) problem for typical scenarios. I think.

regards, tom lane

Hi Vik,

On 1/11/21 5:00 PM, Tom Lane wrote:

I started to look through this patch...

I see you moved this patch to PG15. If you won't be working on the patch
in this CF perhaps it would be better to close it as Returned with
Feedback for now and reopen it when you have a new patch?

I'll do that on March 23 unless I hear arguments to the contrary.

Regards,
--
-David
david@pgmasters.net

On 3/18/21 4:12 PM, David Steele wrote:

Hi Vik,

On 1/11/21 5:00 PM, Tom Lane wrote:

I started to look through this patch...

I see you moved this patch to PG15. If you won't be working on the patch
in this CF

Correct. I won't be working on this again until I finish my review of
the system versioning patch.

perhaps it would be better to close it as Returned with
Feedback for now and reopen it when you have a new patch?

If that is preferred over moving it to PG15, then no objection. As long
as people don't think I've abandoned it.
--
Vik Fearing

On 3/18/21 12:03 PM, Vik Fearing wrote:

On 3/18/21 4:12 PM, David Steele wrote:

Hi Vik,

On 1/11/21 5:00 PM, Tom Lane wrote:

I started to look through this patch...

I see you moved this patch to PG15. If you won't be working on the patch
in this CF

Correct. I won't be working on this again until I finish my review of
the system versioning patch.

perhaps it would be better to close it as Returned with
Feedback for now and reopen it when you have a new patch?

If that is preferred over moving it to PG15, then no objection.

It is, because it means it doesn't need to be looked at again until you
have had time to work on it.

As long
as people don't think I've abandoned it.

This declaration should prevent that.

Regards,
--
-David
david@pgmasters.net