Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options - Mailing list pgsql-hackers

From Krasiyan Andreev
Subject Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options
Date
Msg-id CAN1PwokSDfSkOmDk6sfdEOUz_mTm333-gkLOBPK=JrVsMJFZ1g@mail.gmail.com
Whole thread Raw
In response to Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options  (Oliver Ford <ojford@gmail.com>)
List pgsql-hackers
Hi,
I was able to reproduce exactly the problem, with clean compile and --enable-cassert:
test=# SELECT x,y,lead(y) IGNORE NULLS OVER (ORDER BY x) FROM (VALUES(1,NULL),(2,2),(3,NULL)) AS v(x,y);
 x | y | lead
---+---+------
 1 |   |    2
 2 | 2 |    2
 3 |   |    2
(3 rows)

test=#
Also, make check errors out at window test (without --enable-cassert it was passed in previous compile):
krasiyan@fedora:~/pgsql-src/postgresql$ cat /home/krasiyan/pgsql-src/postgresql/src/test/regress/regression.diffs
diff -U3 /home/krasiyan/pgsql-src/postgresql/src/test/regress/expected/window.out /home/krasiyan/pgsql-src/postgresql/src/test/regress/results/window.out
--- /home/krasiyan/pgsql-src/postgresql/src/test/regress/expected/window.out 2025-01-22 21:25:47.114508215 +0200
+++ /home/krasiyan/pgsql-src/postgresql/src/test/regress/results/window.out 2025-01-23 07:58:26.784659592 +0200
@@ -5477,12 +5477,12 @@
   name   | orbit | lead  | lead_respect | lead_ignore
 ---------+-------+-------+--------------+-------------
  earth   |       |  4332 |         4332 |        4332
- jupiter |  4332 |       |              |          88
+ jupiter |  4332 |       |              |            
  mars    |       |    88 |           88 |          88
  mercury |    88 | 60182 |        60182 |       60182
  neptune | 60182 | 90560 |        90560 |       90560
  pluto   | 90560 | 24491 |        24491 |       24491
- saturn  | 24491 |       |              |         224
+ saturn  | 24491 |       |              |            
  uranus  |       |   224 |          224 |         224
  venus   |   224 |       |              |            
  xyzzy   |       |       |              |            
@@ -5577,13 +5577,13 @@
   name   | orbit | first_value | last_value | nth_value | lead_ignore | lag_ignore
 ---------+-------+-------------+------------+-----------+-------------+------------
  earth   |       |        4332 |       4332 |           |        4332 |          
- jupiter |  4332 |          88 |         88 |           |          88 |          
- mars    |       |        4332 |      60182 |        88 |          88 |       4332
- mercury |    88 |        4332 |      90560 |     60182 |       60182 |       4332
+ jupiter |  4332 |          88 |         88 |           |       60182 |          
+ mars    |       |          88 |      60182 |     60182 |       60182 |       4332
+ mercury |    88 |        4332 |      90560 |     90560 |       90560 |       4332
  neptune | 60182 |          88 |      24491 |     90560 |       90560 |         88
- pluto   | 90560 |          88 |      24491 |     60182 |       24491 |      60182
- saturn  | 24491 |       60182 |        224 |     90560 |         224 |      90560
- uranus  |       |       90560 |        224 |     24491 |         224 |      24491
+ pluto   | 90560 |          88 |      24491 |     60182 |       60182 |      60182
+ saturn  | 24491 |       60182 |        224 |     90560 |       90560 |      90560
+ uranus  |       |       90560 |        224 |     24491 |       24491 |      24491
  venus   |   224 |       24491 |      24491 |           |             |      24491
  xyzzy   |       |         224 |        224 |           |             |        224
 (10 rows)
@@ -5646,14 +5646,14 @@
   name   | orbit | first_value | last_value | nth_value | lead_ignore | lag_ignore
 ---------+-------+-------------+------------+-----------+-------------+------------
  earth   |       |             |            |           |          88 |          
- jupiter |       |          88 |         88 |           |          88 |          
- mars    |       |          88 |      60182 |     60182 |          88 |          
+ jupiter |       |          88 |         88 |           |       60182 |          
+ mars    |       |          88 |      60182 |     60182 |       60182 |          
  mercury |    88 |          88 |      90560 |     60182 |       60182 |          
- neptune | 60182 |          88 |      24491 |     60182 |       90560 |         88
- pluto   | 90560 |          88 |      24491 |     60182 |       24491 |      60182
- saturn  | 24491 |       60182 |        224 |     90560 |         224 |      90560
- uranus  |       |       90560 |        224 |     24491 |         224 |      24491
- venus   |   224 |       24491 |        224 |       224 |             |      24491
+ neptune | 60182 |          88 |      24491 |     60182 |       60182 |         88
+ pluto   | 90560 |          88 |      24491 |     60182 |       60182 |      60182
+ saturn  | 24491 |       60182 |        224 |     90560 |       90560 |      90560
+ uranus  |       |       90560 |        224 |     24491 |       24491 |      24491
+ venus   |   224 |       24491 |        224 |       224 |         224 |      24491
  xyzzy   |       |         224 |        224 |           |             |        224
 (10 rows)
 

На чт, 23.01.2025 г. в 6:25 Tatsuo Ishii <ishii@postgresql.org> написа:
> Hello,
> I also played with the v4 patch and it produces correct result:
> test=# SELECT x,y,lead(y) IGNORE NULLS OVER (ORDER BY x) FROM
> (VALUES(1,NULL),(2,2),(3,NULL)) AS v(x,y);
>  x | y | lead
> ---+---+------
>  1 |   |    2
>  2 | 2 |
>  3 |   |
> (3 rows)
>
> test=#
> It is from today's git, clean compile and install with only v4 patch
> applied, make check also passes without errors.

I guess you are just lucky. In my case I enabled --enable-cassert to
build PostgreSQL and it automatically turn on CLOBBER_FREED_MEMORY and
freed memory area is scrambled. If I look the patch closer, I found a
problem:

+void
+WinCheckAndInitializeNullTreatment(WindowObject winobj,
:
:
+               winobj->win_nonnulls = palloc_array(int64, 16);

WinCheckAndInitializeNullTreatment is called in each built-in window
function. Window functions are called in the per tuple memory context,
which means win_nonnulls disappears when next tuple is supplied to the
window function. If my understanding is correct, winobj->win_nonnulls
needs to survive across processing tuples.

Best reagards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

pgsql-hackers by date:

Previous
From: "Zhou, Zhiguo"
Date:
Subject: Re: [RFC] Lock-free XLog Reservation from WAL
Next
From: Robert Haas
Date:
Subject: Re: Eagerly scan all-visible pages to amortize aggressive vacuum