public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v15 6/8] Row pattern recognition patch (docs).
2+ messages / 2 participants
[nested] [flat]
* [PATCH v15 6/8] Row pattern recognition patch (docs).
@ 2024-03-28 10:30 Tatsuo Ishii <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Tatsuo Ishii @ 2024-03-28 10:30 UTC (permalink / raw)
---
doc/src/sgml/advanced.sgml | 80 ++++++++++++++++++++++++++++++++++++
doc/src/sgml/func.sgml | 54 ++++++++++++++++++++++++
doc/src/sgml/ref/select.sgml | 38 ++++++++++++++++-
3 files changed, 170 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/advanced.sgml b/doc/src/sgml/advanced.sgml
index 755c9f1485..cf18dd887e 100644
--- a/doc/src/sgml/advanced.sgml
+++ b/doc/src/sgml/advanced.sgml
@@ -537,6 +537,86 @@ WHERE pos < 3;
<literal>rank</literal> less than 3.
</para>
+ <para>
+ Row pattern common syntax can be used to perform row pattern recognition
+ in a query. Row pattern common syntax includes two sub
+ clauses: <literal>DEFINE</literal>
+ and <literal>PATTERN</literal>. <literal>DEFINE</literal> defines
+ definition variables along with an expression. The expression must be a
+ logical expression, which means it must
+ return <literal>TRUE</literal>, <literal>FALSE</literal>
+ or <literal>NULL</literal>. The expression may comprise column references
+ and functions. Window functions, aggregate functions and subqueries are
+ not allowed. An example of <literal>DEFINE</literal> is as follows.
+
+<programlisting>
+DEFINE
+ LOWPRICE AS price <= 100,
+ UP AS price > PREV(price),
+ DOWN AS price < PREV(price)
+</programlisting>
+
+ Note that <function>PREV</function> returns the price column in the
+ previous row if it's called in a context of row pattern recognition. So in
+ the second line the definition variable "UP" is <literal>TRUE</literal>
+ when the price column in the current row is greater than the price column
+ in the previous row. Likewise, "DOWN" is <literal>TRUE</literal> when when
+ the price column in the current row is lower than the price column in the
+ previous row.
+ </para>
+ <para>
+ Once <literal>DEFINE</literal> exists, <literal>PATTERN</literal> can be
+ used. <literal>PATTERN</literal> defines a sequence of rows that satisfies
+ certain conditions. For example following <literal>PATTERN</literal>
+ defines that a row starts with the condition "LOWPRICE", then one or more
+ rows satisfy "UP" and finally one or more rows satisfy "DOWN". Note that
+ "+" means one or more matches. Also you can use "*", which means zero or
+ more matches. If a sequence of rows which satisfies the PATTERN is found,
+ in the starting row of the sequence of rows all window functions and
+ aggregates are shown in the target list. Note that aggregations only look
+ into the matched rows, rather than whole frame. In the second or
+ subsequent rows all window functions and aggregates are NULL. For rows
+ that do not match the PATTERN, all window functions and aggregates are
+ shown AS NULL too, except count which shows 0. This is because the
+ unmatched rows are in an empty frame. Example of
+ a <literal>SELECT</literal> using the <literal>DEFINE</literal>
+ and <literal>PATTERN</literal> clause is as follows.
+
+<programlisting>
+SELECT company, tdate, price,
+ first_value(price) OVER w,
+ max(price) OVER w,
+ count(price) OVER w
+FROM stock,
+ WINDOW w AS (
+ PARTITION BY company
+ ORDER BY tdate
+ ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ AFTER MATCH SKIP PAST LAST ROW
+ INITIAL
+ PATTERN (LOWPRICE UP+ DOWN+)
+ DEFINE
+ LOWPRICE AS price <= 100,
+ UP AS price > PREV(price),
+ DOWN AS price < PREV(price)
+);
+</programlisting>
+<screen>
+ company | tdate | price | first_value | max | count
+----------+------------+-------+-------------+-----+-------
+ company1 | 2023-07-01 | 100 | 100 | 200 | 4
+ company1 | 2023-07-02 | 200 | | |
+ company1 | 2023-07-03 | 150 | | |
+ company1 | 2023-07-04 | 140 | | |
+ company1 | 2023-07-05 | 150 | | | 0
+ company1 | 2023-07-06 | 90 | 90 | 130 | 4
+ company1 | 2023-07-07 | 110 | | |
+ company1 | 2023-07-08 | 130 | | |
+ company1 | 2023-07-09 | 120 | | |
+ company1 | 2023-07-10 | 130 | | | 0
+</screen>
+ </para>
+
<para>
When a query involves multiple window functions, it is possible to write
out each one with a separate <literal>OVER</literal> clause, but this is
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 93b0bc2bc6..d25eeb3327 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -22637,6 +22637,7 @@ SELECT count(*) FROM sometable;
returns <literal>NULL</literal> if there is no such row.
</para></entry>
</row>
+
</tbody>
</tgroup>
</table>
@@ -22676,6 +22677,59 @@ SELECT count(*) FROM sometable;
Other frame specifications can be used to obtain other effects.
</para>
+ <para>
+ Row pattern recognition navigation functions are listed in
+ <xref linkend="functions-rpr-navigation-table"/>. These functions
+ can be used to describe DEFINE clause of Row pattern recognition.
+ </para>
+
+ <table id="functions-rpr-navigation-table">
+ <title>Row Pattern Navigation Functions</title>
+ <tgroup cols="1">
+ <thead>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ Function
+ </para>
+ <para>
+ Description
+ </para></entry>
+ </row>
+ </thead>
+
+ <tbody>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>prev</primary>
+ </indexterm>
+ <function>prev</function> ( <parameter>value</parameter> <type>anyelement</type> )
+ <returnvalue>anyelement</returnvalue>
+ </para>
+ <para>
+ Returns the column value at the previous row;
+ returns NULL if there is no previous row in the window frame.
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>next</primary>
+ </indexterm>
+ <function>next</function> ( <parameter>value</parameter> <type>anyelement</type> )
+ <returnvalue>anyelement</returnvalue>
+ </para>
+ <para>
+ Returns the column value at the next row;
+ returns NULL if there is no next row in the window frame.
+ </para></entry>
+ </row>
+
+ </tbody>
+ </tgroup>
+ </table>
+
<note>
<para>
The SQL standard defines a <literal>RESPECT NULLS</literal> or
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 066aed44e6..8f18718d58 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -969,8 +969,8 @@ WINDOW <replaceable class="parameter">window_name</replaceable> AS ( <replaceabl
The <replaceable class="parameter">frame_clause</replaceable> can be one of
<synopsis>
-{ RANGE | ROWS | GROUPS } <replaceable>frame_start</replaceable> [ <replaceable>frame_exclusion</replaceable> ]
-{ RANGE | ROWS | GROUPS } BETWEEN <replaceable>frame_start</replaceable> AND <replaceable>frame_end</replaceable> [ <replaceable>frame_exclusion</replaceable> ]
+{ RANGE | ROWS | GROUPS } <replaceable>frame_start</replaceable> [ <replaceable>frame_exclusion</replaceable> ] [row_pattern_common_syntax]
+{ RANGE | ROWS | GROUPS } BETWEEN <replaceable>frame_start</replaceable> AND <replaceable>frame_end</replaceable> [ <replaceable>frame_exclusion</replaceable> ] [row_pattern_common_syntax]
</synopsis>
where <replaceable>frame_start</replaceable>
@@ -1077,6 +1077,40 @@ EXCLUDE NO OTHERS
a given peer group will be in the frame or excluded from it.
</para>
+ <para>
+ The
+ optional <replaceable class="parameter">row_pattern_common_syntax</replaceable>
+ defines the <firstterm>row pattern recognition condition</firstterm> for
+ this
+ window. <replaceable class="parameter">row_pattern_common_syntax</replaceable>
+ includes following subclauses. <literal>AFTER MATCH SKIP PAST LAST
+ ROW</literal> or <literal>AFTER MATCH SKIP TO NEXT ROW</literal> controls
+ how to proceed to next row position after a match
+ found. With <literal>AFTER MATCH SKIP PAST LAST ROW</literal> (the
+ default) next row position is next to the last row of previous match. On
+ the other hand, with <literal>AFTER MATCH SKIP TO NEXT ROW</literal> next
+ row position is always next to the last row of previous
+ match. <literal>DEFINE</literal> defines definition variables along with a
+ boolean expression. <literal>PATTERN</literal> defines a sequence of rows
+ that satisfies certain conditions using variables defined
+ in <literal>DEFINE</literal> clause. If the variable is not defined in
+ the <literal>DEFINE</literal> clause, it is implicitly assumed
+ following is defined in the <literal>DEFINE</literal> clause.
+
+<synopsis>
+<literal>variable_name</literal> AS TRUE
+</synopsis>
+
+ Note that the maximu number of variables defined
+ in <literal>DEFINE</literal> clause is 26.
+
+<synopsis>
+[ AFTER MATCH SKIP PAST LAST ROW | AFTER MATCH SKIP TO NEXT ROW ]
+PATTERN <replaceable class="parameter">pattern_variable_name</replaceable>[+] [, ...]
+DEFINE <replaceable class="parameter">definition_varible_name</replaceable> AS <replaceable class="parameter">expression</replaceable> [, ...]
+</synopsis>
+ </para>
+
<para>
The purpose of a <literal>WINDOW</literal> clause is to specify the
behavior of <firstterm>window functions</firstterm> appearing in the query's
--
2.25.1
----Next_Part(Thu_Mar_28_19_59_25_2024_076)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v15-0007-Row-pattern-recognition-patch-tests.patch"
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: Buffer locking is special (hints, checksums, AIO writes)
@ 2026-01-15 06:22 Chao Li <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Chao Li @ 2026-01-15 06:22 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Kirill Reshke <[email protected]>; Heikki Linnakangas <[email protected]>; Melanie Plageman <[email protected]>; Matthias van de Meent <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Noah Misch <[email protected]>; Robert Haas <[email protected]>; Michael Paquier <[email protected]>
> On Jan 15, 2026, at 08:04, Chao Li <[email protected]> wrote:
>
>
>
>> On Jan 15, 2026, at 07:37, Andres Freund <[email protected]> wrote:
>>
>> Hi,
>>
>> On 2026-01-15 07:20:27 +0800, Chao Li wrote:
>>>> On Jan 15, 2026, at 00:30, Andres Freund <[email protected]> wrote:
>>>> On 2026-01-14 11:41:19 +0800, Chao Li wrote:
>>>>> Basically, code changes in 0003 is straightforward, just a couple of small comments:
>>>>>
>>>>> 1
>>>>> ```
>>>>> - * refcounts in buf_internals.h. This limitation could be lifted by using a
>>>>> - * 64bit state; but it's unlikely to be worthwhile as 2^18-1 backends exceed
>>>>> - * currently realistic configurations. Even if that limitation were removed,
>>>>> - * we still could not a) exceed 2^23-1 because inval.c stores the ProcNumber
>>>>> - * as a 3-byte signed integer, b) INT_MAX/4 because some places compute
>>>>> - * 4*MaxBackends without any overflow check. We check that the configured
>>>>> - * number of backends does not exceed MAX_BACKENDS in InitializeMaxBackends().
>>>>> + * refcounts in buf_internals.h. This limitation could be lifted, but it's
>>>>> ```
>>>>>
>>>>> Before this patch, there was room for lifting the limitation. With this
>>>>> patch, state is 64bit already, but the significant 32bit will be used for
>>>>> buffer locking as stated in buf_internals.h, in other words, there is no
>>>>> room for lifting the limitation now. If that’s true, then I think we can
>>>>> remove the statements about lifting limitation.
>>>>
>>>> I'm not following - there's plenty space for more bits if we need that:
>>>>
>>>> * State of the buffer itself (in order):
>>>> * - 18 bits refcount
>>>> * - 4 bits usage count
>>>> * - 12 bits of flags
>>>> * - 18 bits share-lock count
>>>> * - 1 bit share-exclusive locked
>>>> * - 1 bit exclusive locked
>>>>
>>>> That's 54 bits in total. Which part is in the lower and which in the upper
>>>> 32bit isn't relevant for anything afaict?
>>>
>>> Because I saw the comment in buf_internals.h:
>>> ```
>>> * NB: A future commit will use a significant portion of the remaining bits to
>>> * implement buffer locking as part of the state variable.
>>> ```
>>> That seems to indicate all the significant 32 bits will be used for buffer locking.
>>
>> A significant portion != all. As the above excerpt from the comment shows, the
>> locking uses 20 bits. We could increase max backends by 5 bits without running
>> out of bits (we'd need space both in the refcount bitspace as well as the
>> share-lock bitspace).
>
> Make sense. I think I misread the comment.
>
>>
>>
>>> Also, there is an assert that concretes the impression:
>>> ```
>>> StaticAssertDecl(BUF_REFCOUNT_BITS + BUF_USAGECOUNT_BITS + BUF_FLAG_BITS == 32,
>>> "parts of buffer state space need to equal 32");
>>> ```
>>
>> You can see that being relaxed in the subsequent commit, when we start to use
>> more bits.
>>
>
> Sure. I plan to review 0003-0005 today. I believe I will get better understanding.
I have finished reviewing 0003-0005. Basically, 0003 and 0004 just delete some unused functions. I only searched over the source tree to ensure no usages of them. I spent time on 0005. The code logic are solid already, I didn't find any correctness issue and only got some small comments:
1 - 0004 - commit message
```
subsequent commits fixing a typo an a parameter name.
```
Typo: a typo an a parameter name => a typo in a parameter name
2 - 0005 - bufmgr.c
```
- * Pin it, share-lock it, write it. (FlushBuffer will do nothing if the
- * buffer is clean by the time we've locked it.)
+ * Pin it, share-exclusive-lock it, write it. (FlushBuffer will do
+ * nothing if the buffer is clean by the time we've locked it.)
*/
PinBuffer_Locked(bufHdr);
```
I think we don’t need to mention lock type in this comment, because the logic belongs to FlushUnlockedBuffer(). Also, FlushBuffer is misleading here, because we call FlushUnlockedBuffer() here, and FlushUnlockedBuffer() in turn calls FlushBuffer().
So, I think we can simplify the comment as something like “Pin it and flush the buffer"
3 - 0005 - bufmgr.c
```
+inline void
+MarkBufferDirtyHint(Buffer buffer, bool buffer_std)
```
It’s quite uncommon to extern an inline function. I think usually if we want to make an inline function accessible from external, we define it “static inline” in a header file. So, I guess “inline” is a typo here.
4 - 0005 - bufmgr.c
```
/*
* MarkBufferDirtyHint
*
* Mark a buffer dirty for non-critical changes.
*
* This is essentially the same as MarkBufferDirty, except:
*
* 1. The caller does not write WAL; so if checksums are enabled, we may need
* to write an XLOG_FPI_FOR_HINT WAL record to protect against torn pages.
* 2. The caller might have only a share-exclusive-lock instead of an
* exclusive-lock on the buffer's content lock.
```
For point 2, do we need to mention “For shared buffers”. Because this function also handles local buffer that doesn’t require a lock.
5 - 0005 - bufmgr.c
```
+/*
+ * Helper for BufferBeginSetHintBits() and BufferSetHintBits16().
+static inline bool
+SharedBufferBeginSetHintBits(Buffer buffer, BufferDesc *buf_hdr, uint64 *lockstate)
```
In the previous function comment:
```
- * MarkBufferDirtyHint
+ * Shared-buffer only helper for MarkBufferDirtyHint() and
+ * BufferSetHintBits16().
```
It mentions “Shared-buffer only helper”. I think SharedBufferBeginSetHintBits is also only for shared buffer, maybe also add “Shared-buffer only” before “helper” in the comment.
6 - 0005 - bufmgr.c
```
+ }
+
+}
```
Nit: In function SharedBufferBeginSetHintBits, the last empty line is not needed.
7 - 0005 - bufmgr.c
```
+ * This checks if the current lock mode already suffices to allow hint bits
+ * being set and, if not, whether the current lock can be upgraded.
+ */
+static inline bool
+SharedBufferBeginSetHintBits(Buffer buffer, BufferDesc *buf_hdr, uint64 *lockstate)
```
Nit: "if not, whether the current lock can be upgraded” might be read as “lock can be upgraded, so the caller still need to take some action to upgrade the lock”, but the function has upgraded the lock when returning true. So, how about explicitly state something like: "if not, it attempts to atomically upgrade it to share-exclusive. Returns true if hint bits may be set (with or without an upgrade), false otherwise."
8 - 0005 - fsmpage.c
```
* needs to be updated. exclusive_lock_held should be set to true if the
* caller is already holding an exclusive lock, to avoid extra work.
```
The function comment of fsm_search_avail() needs to be updated. exclusive_lock_held should be set to true if the caller is already holding a **share-exclusive or** exclusive lock.
Maybe the parameter name “exclusive_lock_held” can be enhanced as well.
9 - 0005 - fsmpage.c
```
+ if (!exclusive_lock_held)
+ BufferFinishSetHintBits(buf, false, true);
```
Nit: just curious why set the third parameter as “true”? When the second (mark_dirty) is false, the third parameter is not used at all.
10 - 0005 - freespace.c
```
- * Reset the next slot pointer. This encourages the use of low-numbered
- * pages, increasing the chances that a later vacuum can truncate the
- * relation. We don't bother with marking the page dirty if it wasn't
- * already, since this is just a hint.
+ * Try to reset the next slot pointer. This encourages the use of
+ * low-numbered pages, increasing the chances that a later vacuum can
+ * truncate the relation. We don't bother with marking the page dirty if
+ * it wasn't already, since this is just a hint.
*/
LockBuffer(buf, BUFFER_LOCK_SHARE);
- ((FSMPage) PageGetContents(page))->fp_next_slot = 0;
+ if (BufferBeginSetHintBits(buf))
+ {
+ ((FSMPage) PageGetContents(page))->fp_next_slot = 0;
+ BufferFinishSetHintBits(buf, false, false);
+ }
```
Before this patch, we unconditionally set fp_next_slot, now the setting might be skipped. You have add “Try to” in the comment that has explained the possibility of skipping setting fp_next_slot. Would it be better to add a brief statement for what will result in when skipping setting fp_next_slot? Something like “Skipping the update only affects reuse, not correctness”.
Lately, I saw you have done this in nbtinsert.c:
```
* mark the index entry killed. It's ok if we're not
* allowed to, this isn't required for correctness.
```
which, I think, confirmed my comment.
11 - 0005 Maybe not a problem. In nbtree.h:
```
/*
* We need to be able to tell the difference between read and write
* requests for pages, in order to do locking correctly.
*/
#define BT_READ BUFFER_LOCK_SHARE
#define BT_WRITE BUFFER_LOCK_EXCLUSIVE
```
Can the new lock type BUFFER_LOCK_SHARE_EXCLUSIVE be used by nbt? Maybe implicitly upgrading from BT_READ to BUFFER_LOCK_SHARE_EXCLUSIVE is good enough?
12 - 0005 - heapam_visibility.c
After this commit, tuple hint bits may remain unset if we can’t obtain share-exclusive permission. That’s fine because hint bits are advisory and optional, but this is a behavior change. Would it make sense to mention this explicitly and briefly in the SetHintBitsExt() comment?
13 - 0005 - nbtutils.c
```
+ /*
+ * If we're not able to set hint bits, there's no point
+ * continuing.
+ */
+ if (!killedsomething &&
+ !BufferBeginSetHintBits(buf))
+ goto unlock_page;
```
I like this code, because it ensures to only call BufferBeginSetHintBits once. But lately, I saw the same logic in gistget.c:
```
+ if (!killedsomething)
+ {
+ /*
+ * Use hint bit infrastructure to be allowed to modify the page
+ * without holding an exclusive lock.
+ */
+ if (!BufferBeginSetHintBits(buffer))
+ goto unlock;
+ }
```
I just feel the gist version is easier to read, so maybe change the nbt one to be the same as the gist one. But I think the nbt one’s comment is better.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-01-15 06:22 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-03-28 10:30 [PATCH v15 6/8] Row pattern recognition patch (docs). Tatsuo Ishii <[email protected]>
2026-01-15 06:22 Re: Buffer locking is special (hints, checksums, AIO writes) Chao Li <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox