public inbox for [email protected]help / color / mirror / Atom feed
Re: Control your disk usage in PG: Introduction to Disk Quota Extension 11+ messages / 7 participants [nested] [flat]
* Re: Control your disk usage in PG: Introduction to Disk Quota Extension @ 2018-12-26 05:56 Michael Paquier <[email protected]> 0 siblings, 1 reply; 11+ messages in thread From: Michael Paquier @ 2018-12-26 05:56 UTC (permalink / raw) To: Robert Haas <[email protected]>; +Cc: [email protected]; pgsql-hackers; Tomas Vondra <[email protected]> On Wed, Nov 21, 2018 at 09:47:44AM -0500, Robert Haas wrote: > +1 for adding some hooks to support this kind of thing, but I think > the names you've chosen are not very good. The hook name should > describe the place from which it is called, not the purpose for which > one imagines that it will be used, because somebody else might imagine > another use. Both BufferExtendCheckPerms_hook_type and > SmgrStat_hook_type are imagining that they know what the hook does - > CheckPerms in the first case and Stat in the second case. I personally don't mind making Postgres more pluggable, but I don't think that we actually need the extra ones proposed here at the layer of smgr, as smgr is already a layer designed to call an underlying set of APIs able to extend, unlink, etc. depending on the storage type. > For this particular purpose, I don't immediately see why you need a > hook in both places. If ReadBuffer is called with P_NEW, aren't we > guaranteed to end up in smgrextend()? Yes, that's a bit awkward. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Control your disk usage in PG: Introduction to Disk Quota Extension @ 2019-01-07 10:56 Haozhou Wang <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 1 reply; 11+ messages in thread From: Haozhou Wang @ 2019-01-07 10:56 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers; Tomas Vondra <[email protected]>; Hubert Zhang <[email protected]> Thanks very much for your comments. To the best of my knowledge, smgr is a layer that abstract the storage operations. Therefore, it is a good place to control or collect information the storage operations without touching the physical storage layer. Moreover, smgr is coming with actual disk IO operation (not consider the OS cache) for postgres. So we do not need to worry about the buffer management in postgres. It will make the purpose of hook is pure: a hook for actual disk IO. Regards, Haozhou On Wed, Dec 26, 2018 at 1:56 PM Michael Paquier <[email protected]> wrote: > On Wed, Nov 21, 2018 at 09:47:44AM -0500, Robert Haas wrote: > > +1 for adding some hooks to support this kind of thing, but I think > > the names you've chosen are not very good. The hook name should > > describe the place from which it is called, not the purpose for which > > one imagines that it will be used, because somebody else might imagine > > another use. Both BufferExtendCheckPerms_hook_type and > > SmgrStat_hook_type are imagining that they know what the hook does - > > CheckPerms in the first case and Stat in the second case. > > I personally don't mind making Postgres more pluggable, but I don't > think that we actually need the extra ones proposed here at the layer > of smgr, as smgr is already a layer designed to call an underlying set > of APIs able to extend, unlink, etc. depending on the storage type. > > > For this particular purpose, I don't immediately see why you need a > > hook in both places. If ReadBuffer is called with P_NEW, aren't we > > guaranteed to end up in smgrextend()? > > Yes, that's a bit awkward. > -- > Michael ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Control your disk usage in PG: Introduction to Disk Quota Extension @ 2019-01-22 04:08 Hubert Zhang <[email protected]> parent: Haozhou Wang <[email protected]> 0 siblings, 1 reply; 11+ messages in thread From: Hubert Zhang @ 2019-01-22 04:08 UTC (permalink / raw) To: Haozhou Wang <[email protected]>; +Cc: Michael Paquier <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers; Tomas Vondra <[email protected]> > > > For this particular purpose, I don't immediately see why you need a > > hook in both places. If ReadBuffer is called with P_NEW, aren't we > > guaranteed to end up in smgrextend()? > Yes, that's a bit awkward. Hi Michael, we revisit the ReadBuffer hook and remove it in the latest patch. ReadBuffer hook is original used to do enforcement(e.g. out of diskquota limit) when query is loading data. We plan to put the enforcement work of running query to separate diskquota worker process. Let worker process to detect the backends to be cancelled and send SIGINT to these backends. So there is no need for ReadBuffer hook anymore. Our patch currently only contains smgr related hooks to catch the file change and get the Active Table list for diskquota extension. Thanks Hubert. On Mon, Jan 7, 2019 at 6:56 PM Haozhou Wang <[email protected]> wrote: > Thanks very much for your comments. > > To the best of my knowledge, smgr is a layer that abstract the storage > operations. Therefore, it is a good place to control or collect information > the storage operations without touching the physical storage layer. > Moreover, smgr is coming with actual disk IO operation (not consider the > OS cache) for postgres. So we do not need to worry about the buffer > management in postgres. > It will make the purpose of hook is pure: a hook for actual disk IO. > > Regards, > Haozhou > > On Wed, Dec 26, 2018 at 1:56 PM Michael Paquier <[email protected]> > wrote: > >> On Wed, Nov 21, 2018 at 09:47:44AM -0500, Robert Haas wrote: >> > +1 for adding some hooks to support this kind of thing, but I think >> > the names you've chosen are not very good. The hook name should >> > describe the place from which it is called, not the purpose for which >> > one imagines that it will be used, because somebody else might imagine >> > another use. Both BufferExtendCheckPerms_hook_type and >> > SmgrStat_hook_type are imagining that they know what the hook does - >> > CheckPerms in the first case and Stat in the second case. >> >> I personally don't mind making Postgres more pluggable, but I don't >> think that we actually need the extra ones proposed here at the layer >> of smgr, as smgr is already a layer designed to call an underlying set >> of APIs able to extend, unlink, etc. depending on the storage type. >> >> > For this particular purpose, I don't immediately see why you need a >> > hook in both places. If ReadBuffer is called with P_NEW, aren't we >> > guaranteed to end up in smgrextend()? >> >> Yes, that's a bit awkward. >> -- >> Michael > > -- Thanks Hubert Zhang Attachments: [application/octet-stream] disk_quota_hooks_v3.patch (4.0K, ../../CAB0yrek4pbh-x7vYmKh70YRfbXhd7k1Ceqwhyy35jUYdYRrXAg@mail.gmail.com/3-disk_quota_hooks_v3.patch) download | inline diff: From 4aac2c69cbb0001be5e95b6d482fc3b032208f81 Mon Sep 17 00:00:00 2001 From: Hubert Zhang <[email protected]> Date: Tue, 22 Jan 2019 11:46:29 +0800 Subject: [PATCH] Add smgr*_hook() hook points to extend the logic of storage management. Co-authored-by: Haozhou Wang <[email protected]> Co-authored-by: Hubert Zhang <[email protected]> Co-authored-by: Hao Wu <[email protected]> --- src/backend/storage/smgr/smgr.c | 30 ++++++++++++++++++++++++++++++ src/include/storage/smgr.h | 18 ++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c index 0c0bba4ab3..b4ba9d9a4d 100644 --- a/src/backend/storage/smgr/smgr.c +++ b/src/backend/storage/smgr/smgr.c @@ -90,6 +90,16 @@ static const f_smgr smgrsw[] = { static const int NSmgr = lengthof(smgrsw); +/* + * Hook for plugins to extend smgr functions. + * for example, collect statistics from smgr functions + * via recording the active relfilenode information. + */ +smgrcreate_hook_type smgrcreate_hook = NULL; +smgrextend_hook_type smgrextend_hook = NULL; +smgrtruncate_hook_type smgrtruncate_hook = NULL; +smgrdounlinkall_hook_type smgrdounlinkall_hook = NULL; + /* * Each backend has a hashtable that stores all extant SMgrRelation objects. @@ -397,6 +407,11 @@ smgrcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo) if (isRedo && reln->md_num_open_segs[forknum] > 0) return; + if (smgrcreate_hook) + { + (*smgrcreate_hook)(reln, forknum, isRedo); + } + /* * We may be using the target table space for the first time in this * database, so create a per-database subdirectory if needed. @@ -492,6 +507,11 @@ smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo) if (nrels == 0) return; + if (smgrdounlinkall_hook) + { + (*smgrdounlinkall_hook)(rels, nrels, isRedo); + } + /* * create an array which contains all relations to be dropped, and close * each relation's forks at the smgr level while at it @@ -615,6 +635,11 @@ void smgrextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, char *buffer, bool skipFsync) { + if (smgrextend_hook) + { + (*smgrextend_hook)(reln, forknum, blocknum, buffer, skipFsync); + } + smgrsw[reln->smgr_which].smgr_extend(reln, forknum, blocknum, buffer, skipFsync); } @@ -698,6 +723,11 @@ smgrnblocks(SMgrRelation reln, ForkNumber forknum) void smgrtruncate(SMgrRelation reln, ForkNumber forknum, BlockNumber nblocks) { + if (smgrtruncate_hook) + { + (*smgrtruncate_hook)(reln, forknum, nblocks); + } + /* * Get rid of any buffers for the about-to-be-deleted blocks. bufmgr will * just drop them without bothering to write the contents. diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h index 820d08ed4e..5426543b5c 100644 --- a/src/include/storage/smgr.h +++ b/src/include/storage/smgr.h @@ -144,5 +144,23 @@ extern void RememberFsyncRequest(RelFileNode rnode, ForkNumber forknum, extern void ForgetRelationFsyncRequests(RelFileNode rnode, ForkNumber forknum); extern void ForgetDatabaseFsyncRequests(Oid dbid); extern void DropRelationFiles(RelFileNode *delrels, int ndelrels, bool isRedo); +/* + * Hook for plugins to extend smgr functions. + * for example, collect statistics from smgr functions + * via recording the active relfilenode information. + */ +typedef void (*smgrcreate_hook_type)(SMgrRelation reln, ForkNumber forknum, + bool isRedo); +extern PGDLLIMPORT smgrcreate_hook_type smgrcreate_hook; +typedef void (*smgrextend_hook_type)(SMgrRelation reln, ForkNumber forknum, + BlockNumber blocknum, + char *buffer, bool skipFsync); +extern PGDLLIMPORT smgrextend_hook_type smgrextend_hook; +typedef void (*smgrtruncate_hook_type)(SMgrRelation reln, ForkNumber forknum, + BlockNumber nblocks); +extern PGDLLIMPORT smgrtruncate_hook_type smgrtruncate_hook; +typedef void (*smgrdounlinkall_hook_type)(SMgrRelation *rels, int nrels, + bool isRedo); +extern PGDLLIMPORT smgrdounlinkall_hook_type smgrdounlinkall_hook; #endif /* SMGR_H */ -- 2.11.0 (Apple Git-81) ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Control your disk usage in PG: Introduction to Disk Quota Extension @ 2019-01-30 02:26 Hubert Zhang <[email protected]> parent: Hubert Zhang <[email protected]> 0 siblings, 1 reply; 11+ messages in thread From: Hubert Zhang @ 2019-01-30 02:26 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers; Tomas Vondra <[email protected]>; Heikki Linnakangas <[email protected]> Hi Michael, Robert For you question about the hook position, I want to explain more about the background why we want to introduce these hooks. We wrote a diskquota extension <https://github.com/greenplum-db/diskquota; for Postgresql(which is inspired by Heikki's pg_quota <https://github.com/hlinnaka/pg_quota;). Diskquota extension is used to control the disk usage in Postgresql in a fine-grained way, which means: 1. You could set disk quota limit at schema level or role level. 2. A background worker will gather the current disk usage for each schema/role in realtime. 3. A background worker will generate the blacklist for schema/role whose quota limit is exceeded. 4. New transaction want to insert data into the schema/role in the blacklist will be cancelled. In step 2, gathering the current disk usage for each schema needs to sum disk size of all the tables in this schema. This is a time consuming operation. We want to use hooks in SMGR to detect the Active Table, and only recalculate the disk size of all the Active Tables. For example, the smgrextend hook indicates that you allocate a new block and the table need to be treated as Active Table. Do you have some better hook positions recommend to solve the above user case? Thanks in advance. Hubert On Tue, Jan 22, 2019 at 12:08 PM Hubert Zhang <[email protected]> wrote: > > For this particular purpose, I don't immediately see why you need a >> > hook in both places. If ReadBuffer is called with P_NEW, aren't we >> > guaranteed to end up in smgrextend()? >> Yes, that's a bit awkward. > > > Hi Michael, we revisit the ReadBuffer hook and remove it in the latest > patch. > ReadBuffer hook is original used to do enforcement(e.g. out of diskquota > limit) when query is loading data. > We plan to put the enforcement work of running query to separate diskquota > worker process. > Let worker process to detect the backends to be cancelled and send SIGINT > to these backends. > So there is no need for ReadBuffer hook anymore. > > Our patch currently only contains smgr related hooks to catch the file > change and get the Active Table list for diskquota extension. > > Thanks Hubert. > > > On Mon, Jan 7, 2019 at 6:56 PM Haozhou Wang <[email protected]> wrote: > >> Thanks very much for your comments. >> >> To the best of my knowledge, smgr is a layer that abstract the storage >> operations. Therefore, it is a good place to control or collect information >> the storage operations without touching the physical storage layer. >> Moreover, smgr is coming with actual disk IO operation (not consider the >> OS cache) for postgres. So we do not need to worry about the buffer >> management in postgres. >> It will make the purpose of hook is pure: a hook for actual disk IO. >> >> Regards, >> Haozhou >> >> On Wed, Dec 26, 2018 at 1:56 PM Michael Paquier <[email protected]> >> wrote: >> >>> On Wed, Nov 21, 2018 at 09:47:44AM -0500, Robert Haas wrote: >>> > +1 for adding some hooks to support this kind of thing, but I think >>> > the names you've chosen are not very good. The hook name should >>> > describe the place from which it is called, not the purpose for which >>> > one imagines that it will be used, because somebody else might imagine >>> > another use. Both BufferExtendCheckPerms_hook_type and >>> > SmgrStat_hook_type are imagining that they know what the hook does - >>> > CheckPerms in the first case and Stat in the second case. >>> >>> I personally don't mind making Postgres more pluggable, but I don't >>> think that we actually need the extra ones proposed here at the layer >>> of smgr, as smgr is already a layer designed to call an underlying set >>> of APIs able to extend, unlink, etc. depending on the storage type. >>> >>> > For this particular purpose, I don't immediately see why you need a >>> > hook in both places. If ReadBuffer is called with P_NEW, aren't we >>> > guaranteed to end up in smgrextend()? >>> >>> Yes, that's a bit awkward. >>> -- >>> Michael >> >> > > -- > Thanks > > Hubert Zhang > -- Thanks Hubert Zhang ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Control your disk usage in PG: Introduction to Disk Quota Extension @ 2019-01-30 12:34 Hubert Zhang <[email protected]> parent: Hubert Zhang <[email protected]> 0 siblings, 0 replies; 11+ messages in thread From: Hubert Zhang @ 2019-01-30 12:34 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers; Tomas Vondra <[email protected]>; Heikki Linnakangas <[email protected]> Hi all, Currently we add hooks in SMGR to detect which table is being modified(disk size change). Inserting rows into existing page with room will not introduce new block, and thus should not be treated as active table. smgrextend is a good position to meet this behavior. We welcome suggestions on other better hook positions! Besides, suppose we use smgr as hook position, I want to discuss the API passed to the hook function. Take smgrextend as example. The function interface of smgrextend is like that: ``` void smgrextend (SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, char *buffer, bool skipFsync); ``` So the hook api of smgrextend could have two main options. Hook API Option1 ``` typedef void (*smgrextend_hook_type) (RelFileNode smgr_rnode,ForkNumber forknum); ``` Hook API Option 2 ``` typedef void (*smgrextend_hook_type) (SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,char *buffer, bool skipFsync); ``` As for Option1. Since diskquota extension only needs the relfilenode information to detect the active tables, Option1 just pass the RelFileNode to the hook function. It's more clear and has semantic meaning. While Option 2 is to pass the original parameters to the hook functions without any filter. This is more general and let the different hook implementations to decide how to use these parameters. Option 1 also needs some additional work to handle smgrdounlinkall case, whose input parameter is the SMgrRelation list. We may need to palloc Relfilenode list and pfree it manually. smgrdounlinkall function interface: ``` smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo, char *relstorages) ``` Based on the assumption we use smgr as hook position, hook API option1 or option2 which is better? Or we could find some balanced API between option1 and option2? Again comments on other better hook positions are appreciated! Thanks Hubert On Wed, Jan 30, 2019 at 10:26 AM Hubert Zhang <[email protected]> wrote: > Hi Michael, Robert > For you question about the hook position, I want to explain more about the > background why we want to introduce these hooks. > We wrote a diskquota extension <https://github.com/greenplum-db/diskquota; > for Postgresql(which is inspired by Heikki's pg_quota > <https://github.com/hlinnaka/pg_quota;). Diskquota extension is used to > control the disk usage in Postgresql in a fine-grained way, which means: > 1. You could set disk quota limit at schema level or role level. > 2. A background worker will gather the current disk usage for each > schema/role in realtime. > 3. A background worker will generate the blacklist for schema/role whose > quota limit is exceeded. > 4. New transaction want to insert data into the schema/role in the > blacklist will be cancelled. > > In step 2, gathering the current disk usage for each schema needs to sum > disk size of all the tables in this schema. This is a time consuming > operation. We want to use hooks in SMGR to detect the Active Table, and > only recalculate the disk size of all the Active Tables. > For example, the smgrextend hook indicates that you allocate a new block > and the table need to be treated as Active Table. > > Do you have some better hook positions recommend to solve the above user > case? > Thanks in advance. > > Hubert > > > > > > On Tue, Jan 22, 2019 at 12:08 PM Hubert Zhang <[email protected]> wrote: > >> > For this particular purpose, I don't immediately see why you need a >>> > hook in both places. If ReadBuffer is called with P_NEW, aren't we >>> > guaranteed to end up in smgrextend()? >>> Yes, that's a bit awkward. >> >> >> Hi Michael, we revisit the ReadBuffer hook and remove it in the latest >> patch. >> ReadBuffer hook is original used to do enforcement(e.g. out of diskquota >> limit) when query is loading data. >> We plan to put the enforcement work of running query to separate >> diskquota worker process. >> Let worker process to detect the backends to be cancelled and send SIGINT >> to these backends. >> So there is no need for ReadBuffer hook anymore. >> >> Our patch currently only contains smgr related hooks to catch the file >> change and get the Active Table list for diskquota extension. >> >> Thanks Hubert. >> >> >> On Mon, Jan 7, 2019 at 6:56 PM Haozhou Wang <[email protected]> wrote: >> >>> Thanks very much for your comments. >>> >>> To the best of my knowledge, smgr is a layer that abstract the storage >>> operations. Therefore, it is a good place to control or collect information >>> the storage operations without touching the physical storage layer. >>> Moreover, smgr is coming with actual disk IO operation (not consider the >>> OS cache) for postgres. So we do not need to worry about the buffer >>> management in postgres. >>> It will make the purpose of hook is pure: a hook for actual disk IO. >>> >>> Regards, >>> Haozhou >>> >>> On Wed, Dec 26, 2018 at 1:56 PM Michael Paquier <[email protected]> >>> wrote: >>> >>>> On Wed, Nov 21, 2018 at 09:47:44AM -0500, Robert Haas wrote: >>>> > +1 for adding some hooks to support this kind of thing, but I think >>>> > the names you've chosen are not very good. The hook name should >>>> > describe the place from which it is called, not the purpose for which >>>> > one imagines that it will be used, because somebody else might imagine >>>> > another use. Both BufferExtendCheckPerms_hook_type and >>>> > SmgrStat_hook_type are imagining that they know what the hook does - >>>> > CheckPerms in the first case and Stat in the second case. >>>> >>>> I personally don't mind making Postgres more pluggable, but I don't >>>> think that we actually need the extra ones proposed here at the layer >>>> of smgr, as smgr is already a layer designed to call an underlying set >>>> of APIs able to extend, unlink, etc. depending on the storage type. >>>> >>>> > For this particular purpose, I don't immediately see why you need a >>>> > hook in both places. If ReadBuffer is called with P_NEW, aren't we >>>> > guaranteed to end up in smgrextend()? >>>> >>>> Yes, that's a bit awkward. >>>> -- >>>> Michael >>> >>> >> >> -- >> Thanks >> >> Hubert Zhang >> > > > -- > Thanks > > Hubert Zhang > -- Thanks Hubert Zhang ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: typos @ 2023-01-03 07:33 Amit Kapila <[email protected]> 0 siblings, 1 reply; 11+ messages in thread From: Amit Kapila @ 2023-01-03 07:33 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Justin Pryzby <[email protected]>; [email protected] On Tue, Jan 3, 2023 at 12:58 PM Michael Paquier <[email protected]> wrote: > > On Fri, Dec 30, 2022 at 05:12:57PM -0600, Justin Pryzby wrote: > > # Use larger ccache cache, as this task compiles with multiple compilers / > # flag combinations > - CCACHE_MAXSIZE: "1GB" > + CCACHE_MAXSIZE: "1G" > > In 0006, I am not sure how much this matters. > The other places in that file use M, so maybe, this is more consistent. One minor comment: - spoken in Belgium (BE), with a <acronym>UTF-8</acronym> character set + spoken in Belgium (BE), with a <acronym>UTF</acronym>-8 character set Shouldn't this be <acronym>UTF8</acronym> as we are using in func.sgml? -- With Regards, Amit Kapila. ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: typos @ 2023-01-03 08:41 Michael Paquier <[email protected]> parent: Amit Kapila <[email protected]> 0 siblings, 1 reply; 11+ messages in thread From: Michael Paquier @ 2023-01-03 08:41 UTC (permalink / raw) To: Amit Kapila <[email protected]>; +Cc: Justin Pryzby <[email protected]>; [email protected] On Tue, Jan 03, 2023 at 01:03:01PM +0530, Amit Kapila wrote: > One minor comment: > - spoken in Belgium (BE), with a <acronym>UTF-8</acronym> character set > + spoken in Belgium (BE), with a <acronym>UTF</acronym>-8 character set > > Shouldn't this be <acronym>UTF8</acronym> as we are using in func.sgml? Yeah, I was wondering as well why this change is not worse, which is why I left it out of 33ab0a2. There is an acronym for UTF in acronym.sgml, which makes sense to me, but that's the only place where this is used. To add more on top of that, the docs basically need only UTF8, and we have three references to UTF-16, none of them using the <acronym> markup. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: typos @ 2023-01-03 16:16 Peter Eisentraut <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 0 replies; 11+ messages in thread From: Peter Eisentraut @ 2023-01-03 16:16 UTC (permalink / raw) To: Michael Paquier <[email protected]>; Amit Kapila <[email protected]>; +Cc: Justin Pryzby <[email protected]>; [email protected] On 03.01.23 09:41, Michael Paquier wrote: > On Tue, Jan 03, 2023 at 01:03:01PM +0530, Amit Kapila wrote: >> One minor comment: >> - spoken in Belgium (BE), with a <acronym>UTF-8</acronym> character set >> + spoken in Belgium (BE), with a <acronym>UTF</acronym>-8 character set >> >> Shouldn't this be <acronym>UTF8</acronym> as we are using in func.sgml? > > Yeah, I was wondering as well why this change is not worse, which is > why I left it out of 33ab0a2. There is an acronym for UTF in > acronym.sgml, which makes sense to me, but that's the only place where > this is used. To add more on top of that, the docs basically need > only UTF8, and we have three references to UTF-16, none of them using > the <acronym> markup. The thing is called "UTF-8". Here, we are not talking about the PostgreSQL identifier. ^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH v2 1/4] misc binaryheap fixes @ 2023-07-19 22:54 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 11+ messages in thread From: Nathan Bossart @ 2023-07-19 22:54 UTC (permalink / raw) --- src/backend/postmaster/pgarch.c | 12 ++++++------ src/backend/replication/logical/reorderbuffer.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index 46af349564..c28e6f2070 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -108,7 +108,7 @@ static ArchiveModuleState *archive_module_state; * archive_status. Minimizing the number of directory scans when there are * many files to archive can significantly improve archival rate. * - * arch_heap is a max-heap that is used during the directory scan to track + * arch_heap is a min-heap that is used during the directory scan to track * the highest-priority files to archive. After the directory scan * completes, the file names are stored in ascending order of priority in * arch_files. pgarch_readyXlog() returns files from arch_files until it @@ -248,7 +248,7 @@ PgArchiverMain(void) arch_files = palloc(sizeof(struct arch_files_state)); arch_files->arch_files_size = 0; - /* Initialize our max-heap for prioritizing files to archive. */ + /* Initialize our min-heap for prioritizing files to archive. */ arch_files->arch_heap = binaryheap_allocate(NUM_FILES_PER_DIRECTORY_SCAN, ready_file_comparator, NULL); @@ -624,7 +624,7 @@ pgarch_readyXlog(char *xlog) basename[basenamelen] = '\0'; /* - * Store the file in our max-heap if it has a high enough priority. + * Store the file in our min-heap if it has a high enough priority. */ if (arch_files->arch_heap->bh_size < NUM_FILES_PER_DIRECTORY_SCAN) { @@ -644,15 +644,15 @@ pgarch_readyXlog(char *xlog) * Remove the lowest priority file and add the current one to the * heap. */ - arch_file = DatumGetCString(binaryheap_remove_first(arch_files->arch_heap)); + arch_file = DatumGetCString(binaryheap_first(arch_files->arch_heap)); strcpy(arch_file, basename); - binaryheap_add(arch_files->arch_heap, CStringGetDatum(arch_file)); + binaryheap_replace_first(arch_files->arch_heap, CStringGetDatum(arch_file)); } } FreeDir(rldir); /* If no files were found, simply return. */ - if (arch_files->arch_heap->bh_size == 0) + if (binaryheap_empty(arch_files->arch_heap)) return false; /* diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 26d252bd87..05bbcecd29 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -1381,7 +1381,7 @@ ReorderBufferIterTXNNext(ReorderBuffer *rb, ReorderBufferIterTXNState *state) int32 off; /* nothing there anymore */ - if (state->heap->bh_size == 0) + if (binaryheap_empty(state->heap)) return NULL; off = DatumGetInt32(binaryheap_first(state->heap)); -- 2.25.1 --qDbXVdCdHGoSgWSk Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0002-make-binaryheap-available-to-frontend.patch" ^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH v2 1/4] misc binaryheap fixes @ 2023-07-19 22:54 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 11+ messages in thread From: Nathan Bossart @ 2023-07-19 22:54 UTC (permalink / raw) --- src/backend/postmaster/pgarch.c | 12 ++++++------ src/backend/replication/logical/reorderbuffer.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index 46af349564..c28e6f2070 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -108,7 +108,7 @@ static ArchiveModuleState *archive_module_state; * archive_status. Minimizing the number of directory scans when there are * many files to archive can significantly improve archival rate. * - * arch_heap is a max-heap that is used during the directory scan to track + * arch_heap is a min-heap that is used during the directory scan to track * the highest-priority files to archive. After the directory scan * completes, the file names are stored in ascending order of priority in * arch_files. pgarch_readyXlog() returns files from arch_files until it @@ -248,7 +248,7 @@ PgArchiverMain(void) arch_files = palloc(sizeof(struct arch_files_state)); arch_files->arch_files_size = 0; - /* Initialize our max-heap for prioritizing files to archive. */ + /* Initialize our min-heap for prioritizing files to archive. */ arch_files->arch_heap = binaryheap_allocate(NUM_FILES_PER_DIRECTORY_SCAN, ready_file_comparator, NULL); @@ -624,7 +624,7 @@ pgarch_readyXlog(char *xlog) basename[basenamelen] = '\0'; /* - * Store the file in our max-heap if it has a high enough priority. + * Store the file in our min-heap if it has a high enough priority. */ if (arch_files->arch_heap->bh_size < NUM_FILES_PER_DIRECTORY_SCAN) { @@ -644,15 +644,15 @@ pgarch_readyXlog(char *xlog) * Remove the lowest priority file and add the current one to the * heap. */ - arch_file = DatumGetCString(binaryheap_remove_first(arch_files->arch_heap)); + arch_file = DatumGetCString(binaryheap_first(arch_files->arch_heap)); strcpy(arch_file, basename); - binaryheap_add(arch_files->arch_heap, CStringGetDatum(arch_file)); + binaryheap_replace_first(arch_files->arch_heap, CStringGetDatum(arch_file)); } } FreeDir(rldir); /* If no files were found, simply return. */ - if (arch_files->arch_heap->bh_size == 0) + if (binaryheap_empty(arch_files->arch_heap)) return false; /* diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 26d252bd87..05bbcecd29 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -1381,7 +1381,7 @@ ReorderBufferIterTXNNext(ReorderBuffer *rb, ReorderBufferIterTXNState *state) int32 off; /* nothing there anymore */ - if (state->heap->bh_size == 0) + if (binaryheap_empty(state->heap)) return NULL; off = DatumGetInt32(binaryheap_first(state->heap)); -- 2.25.1 --qDbXVdCdHGoSgWSk Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0002-make-binaryheap-available-to-frontend.patch" ^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH v8 2/7] Row pattern recognition patch (parse/analysis). @ 2023-09-25 05:01 Tatsuo Ishii <[email protected]> 0 siblings, 0 replies; 11+ messages in thread From: Tatsuo Ishii @ 2023-09-25 05:01 UTC (permalink / raw) --- src/backend/parser/parse_agg.c | 7 + src/backend/parser/parse_clause.c | 293 +++++++++++++++++++++++++++++- src/backend/parser/parse_expr.c | 4 + src/backend/parser/parse_func.c | 3 + 4 files changed, 306 insertions(+), 1 deletion(-) diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c index 85cd47b7ae..aa7a1cee80 100644 --- a/src/backend/parser/parse_agg.c +++ b/src/backend/parser/parse_agg.c @@ -564,6 +564,10 @@ check_agglevels_and_constraints(ParseState *pstate, Node *expr) errkind = true; break; + case EXPR_KIND_RPR_DEFINE: + errkind = true; + break; + /* * There is intentionally no default: case here, so that the * compiler will warn if we add a new ParseExprKind without @@ -953,6 +957,9 @@ transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc, case EXPR_KIND_CYCLE_MARK: errkind = true; break; + case EXPR_KIND_RPR_DEFINE: + errkind = true; + break; /* * There is intentionally no default: case here, so that the diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index 334b9b42bd..293d4b1680 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -100,7 +100,10 @@ static WindowClause *findWindowClause(List *wclist, const char *name); static Node *transformFrameOffset(ParseState *pstate, int frameOptions, Oid rangeopfamily, Oid rangeopcintype, Oid *inRangeFunc, Node *clause); - +static void transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef, List **targetlist); +static List *transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef, List **targetlist); +static void transformPatternClause(ParseState *pstate, WindowClause *wc, WindowDef *windef); +static List *transformMeasureClause(ParseState *pstate, WindowClause *wc, WindowDef *windef); /* * transformFromClause - @@ -2950,6 +2953,10 @@ transformWindowDefinitions(ParseState *pstate, rangeopfamily, rangeopcintype, &wc->endInRangeFunc, windef->endOffset); + + /* Process Row Pattern Recognition related clauses */ + transformRPR(pstate, wc, windef, targetlist); + wc->runCondition = NIL; wc->winref = winref; @@ -3815,3 +3822,287 @@ transformFrameOffset(ParseState *pstate, int frameOptions, return node; } + +/* + * transformRPR + * Process Row Pattern Recognition related clauses + */ +static void +transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef, List **targetlist) +{ + /* + * Window definition exists? + */ + if (windef == NULL) + return; + + /* + * Row Pattern Common Syntax clause exists? + */ + if (windef->rpCommonSyntax == NULL) + return; + + /* Check Frame option. Frame must start at current row */ + if ((wc->frameOptions & FRAMEOPTION_START_CURRENT_ROW) == 0) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("FRAME must start at current row when row patttern recognition is used"))); + + /* Transform AFTER MACH SKIP TO clause */ + wc->rpSkipTo = windef->rpCommonSyntax->rpSkipTo; + + /* Transform SEEK or INITIAL clause */ + wc->initial = windef->rpCommonSyntax->initial; + + /* Transform DEFINE clause into list of TargetEntry's */ + wc->defineClause = transformDefineClause(pstate, wc, windef, targetlist); + + /* Check PATTERN clause and copy to patternClause */ + transformPatternClause(pstate, wc, windef); + + /* Transform MEASURE clause */ + transformMeasureClause(pstate, wc, windef); +} + +/* + * transformDefineClause Process DEFINE clause and transform ResTarget into + * list of TargetEntry. + * + * XXX we only support column reference in row pattern definition search + * condition, e.g. "price". <row pattern definition variable name>.<column + * reference> is not supported, e.g. "A.price". + */ +static List * +transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef, List **targetlist) +{ + /* DEFINE variable name initials */ + static char *defineVariableInitials = "abcdefghijklmnopqrstuvwxyz"; + + ListCell *lc, *l; + ResTarget *restarget, *r; + List *restargets; + char *name; + int initialLen; + int i; + + /* + * If Row Definition Common Syntax exists, DEFINE clause must exist. + * (the raw parser should have already checked it.) + */ + Assert(windef->rpCommonSyntax->rpDefs != NULL); + + /* + * Check and add "A AS A IS TRUE" if pattern variable is missing in DEFINE + * per the SQL standard. + */ + restargets = NIL; + foreach(lc, windef->rpCommonSyntax->rpPatterns) + { + A_Expr *a; + bool found = false; + + if (!IsA(lfirst(lc), A_Expr)) + ereport(ERROR, + errmsg("node type is not A_Expr")); + + a = (A_Expr *)lfirst(lc); + name = strVal(a->lexpr); + + foreach(l, windef->rpCommonSyntax->rpDefs) + { + restarget = (ResTarget *)lfirst(l); + + if (!strcmp(restarget->name, name)) + { + found = true; + break; + } + } + + if (!found) + { + /* + * "name" is missing. So create "name AS name IS TRUE" ResTarget + * node and add it to the temporary list. + */ + A_Const *n; + + restarget = makeNode(ResTarget); + n = makeNode(A_Const); + n->val.boolval.type = T_Boolean; + n->val.boolval.boolval = true; + n->location = -1; + restarget->name = pstrdup(name); + restarget->indirection = NIL; + restarget->val = (Node *)n; + restarget->location = -1; + restargets = lappend((List *)restargets, restarget); + } + } + + if (list_length(restargets) >= 1) + { + /* add missing DEFINEs */ + windef->rpCommonSyntax->rpDefs = list_concat(windef->rpCommonSyntax->rpDefs, + restargets); + list_free(restargets); + } + + /* + * Check for duplicate row pattern definition variables. The standard + * requires that no two row pattern definition variable names shall be + * equivalent. + */ + restargets = NIL; + foreach(lc, windef->rpCommonSyntax->rpDefs) + { + restarget = (ResTarget *)lfirst(lc); + name = restarget->name; + + /* + * Add DEFINE expression (Restarget->val) to the targetlist as a + * TargetEntry if it does not exist yet. Planner will add the column + * ref var node to the outer plan's target list later on. This makes + * DEFINE expression could access the outer tuple while evaluating + * PATTERN. + * + * XXX: adding whole expressions of DEFINE to the plan.targetlist is + * not so good, because it's not necessary to evalute the expression + * in the target list while running the plan. We should extract the + * var nodes only then add them to the plan.targetlist. + */ + findTargetlistEntrySQL99(pstate, (Node *)restarget->val, targetlist, EXPR_KIND_RPR_DEFINE); + + /* + * Make sure that the row pattern definition search condition is a + * boolean expression. + */ + transformWhereClause(pstate, restarget->val, + EXPR_KIND_RPR_DEFINE, "DEFINE"); + + foreach(l, restargets) + { + char *n; + + r = (ResTarget *) lfirst(l); + n = r->name; + + if (!strcmp(n, name)) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("row pattern definition variable name \"%s\" appears more than once in DEFINE clause", + name), + parser_errposition(pstate, exprLocation((Node *)r)))); + } + restargets = lappend(restargets, restarget); + } + list_free(restargets); + + /* + * Create list of row pattern DEFINE variable name's initial. + * We assign [a-z] to them (up to 26 variable names are allowed). + */ + restargets = NIL; + i = 0; + initialLen = strlen(defineVariableInitials); + + foreach(lc, windef->rpCommonSyntax->rpDefs) + { + char initial[2]; + + restarget = (ResTarget *)lfirst(lc); + name = restarget->name; + + if (i >= initialLen) + { + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("number of row pattern definition variable names exceeds %d", initialLen), + parser_errposition(pstate, exprLocation((Node *)restarget)))); + } + initial[0] = defineVariableInitials[i++]; + initial[1] = '\0'; + wc->defineInitial = lappend(wc->defineInitial, makeString(pstrdup(initial))); + } + + return transformTargetList(pstate, windef->rpCommonSyntax->rpDefs, + EXPR_KIND_RPR_DEFINE); +} + +/* + * transformPatternClause + * Process PATTERN clause and return PATTERN clause in the raw parse tree + */ +static void +transformPatternClause(ParseState *pstate, WindowClause *wc, WindowDef *windef) +{ + ListCell *lc, *l; + + /* + * Row Pattern Common Syntax clause exists? + */ + if (windef->rpCommonSyntax == NULL) + return; + + /* + * Primary row pattern variable names in PATTERN clause must appear in + * DEFINE clause as row pattern definition variable names. + */ + wc->patternVariable = NIL; + wc->patternRegexp = NIL; + foreach(lc, windef->rpCommonSyntax->rpPatterns) + { + A_Expr *a; + char *name; + char *regexp; + bool found = false; + + if (!IsA(lfirst(lc), A_Expr)) + ereport(ERROR, + errmsg("node type is not A_Expr")); + + a = (A_Expr *)lfirst(lc); + name = strVal(a->lexpr); + + foreach(l, windef->rpCommonSyntax->rpDefs) + { + ResTarget *restarget = (ResTarget *)lfirst(l); + + if (!strcmp(restarget->name, name)) + { + found = true; + break; + } + } + + if (!found) + { + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("primary row pattern variable name \"%s\" does not appear in DEFINE clause", + name), + parser_errposition(pstate, exprLocation((Node *)a)))); + } + wc->patternVariable = lappend(wc->patternVariable, makeString(pstrdup(name))); + regexp = strVal(lfirst(list_head(a->name))); + wc->patternRegexp = lappend(wc->patternRegexp, makeString(pstrdup(regexp))); + } +} + +/* + * transformMeasureClause + * Process MEASURE clause + * XXX MEASURE clause is not supported yet + */ +static List * +transformMeasureClause(ParseState *pstate, WindowClause *wc, WindowDef *windef) +{ + if (windef->rowPatternMeasures == NIL) + return NIL; + + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("%s","MEASURE clause is not supported yet"), + parser_errposition(pstate, exprLocation((Node *)windef->rowPatternMeasures)))); + return NIL; +} diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 64c582c344..18b58ac263 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -557,6 +557,7 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) case EXPR_KIND_COPY_WHERE: case EXPR_KIND_GENERATED_COLUMN: case EXPR_KIND_CYCLE_MARK: + case EXPR_KIND_RPR_DEFINE: /* okay */ break; @@ -1770,6 +1771,7 @@ transformSubLink(ParseState *pstate, SubLink *sublink) case EXPR_KIND_VALUES: case EXPR_KIND_VALUES_SINGLE: case EXPR_KIND_CYCLE_MARK: + case EXPR_KIND_RPR_DEFINE: /* okay */ break; case EXPR_KIND_CHECK_CONSTRAINT: @@ -3149,6 +3151,8 @@ ParseExprKindName(ParseExprKind exprKind) return "GENERATED AS"; case EXPR_KIND_CYCLE_MARK: return "CYCLE"; + case EXPR_KIND_RPR_DEFINE: + return "DEFINE"; /* * There is intentionally no default: case here, so that the diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index b3f0b6a137..2ff3699538 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -2656,6 +2656,9 @@ check_srf_call_placement(ParseState *pstate, Node *last_srf, int location) case EXPR_KIND_CYCLE_MARK: errkind = true; break; + case EXPR_KIND_RPR_DEFINE: + errkind = true; + break; /* * There is intentionally no default: case here, so that the -- 2.25.1 ----Next_Part(Mon_Sep_25_14_26_30_2023_752)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v8-0003-Row-pattern-recognition-patch-planner.patch" ^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2023-09-25 05:01 UTC | newest] Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2018-12-26 05:56 Re: Control your disk usage in PG: Introduction to Disk Quota Extension Michael Paquier <[email protected]> 2019-01-07 10:56 ` Haozhou Wang <[email protected]> 2019-01-22 04:08 ` Hubert Zhang <[email protected]> 2019-01-30 02:26 ` Hubert Zhang <[email protected]> 2019-01-30 12:34 ` Hubert Zhang <[email protected]> 2023-01-03 07:33 Re: typos Amit Kapila <[email protected]> 2023-01-03 08:41 ` Re: typos Michael Paquier <[email protected]> 2023-01-03 16:16 ` Re: typos Peter Eisentraut <[email protected]> 2023-07-19 22:54 [PATCH v2 1/4] misc binaryheap fixes Nathan Bossart <[email protected]> 2023-07-19 22:54 [PATCH v2 1/4] misc binaryheap fixes Nathan Bossart <[email protected]> 2023-09-25 05:01 [PATCH v8 2/7] Row pattern recognition patch (parse/analysis). Tatsuo Ishii <[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