| From: | jian he <jian(dot)universality(at)gmail(dot)com> |
|---|---|
| To: | Ajin Cherian <itsajin(at)gmail(dot)com> |
| Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, Garrett Thornburg <film42(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: PATCH: Add REINDEX tag to event triggers |
| Date: | 2023-11-27 16:28:50 |
| Message-ID: | CACJufxFPTWsdNj=XLnuBuf=J68NAHPNn7C0uQodzPHYQ=h=Lnw@mail.gmail.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Nov 27, 2023 at 6:58 PM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
>
> I just started reviewing the patch. Some minor comments:
> In patch 0001:
> In standard_ProcessUtility(), since you are unconditionally calling
> ProcessUtilitySlow() in case of T_ReindexStmt, you really don't need
> the case statement for T_ReindexStmt just like all the other commands
> which have event trigger support. It will call ProcessUtilitySlow() as
> default.
>
> In patch 0004:
> No need to duplicate reindex_event_trigger_collect in indexcmds.c
> since it is already present in index.c. Add it to index.h and make the
> function extern so that it can be accessed in both index.c and
> indexcmds.c
>
> regards,
> Ajin Cherian
> Fujitsu Australia
Thanks for reviewing it!
The above 2 suggestions are good, now the code is more neat.
| Attachment | Content-Type | Size |
|---|---|---|
| v6-0001-make-event-triggers-facility-react-to-ReindexStmt.patch | text/x-patch | 2.3 KB |
| v6-0002-tag-ReindexStmt-as-ddl-in-GetCommandLogLevel.patch | text/x-patch | 867 bytes |
| v6-0003-refactor-multiple-functions-to-support-tracking-r.patch | text/x-patch | 13.5 KB |
| v6-0004-make-Event-trigger-support-reindex-command.patch | text/x-patch | 14.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Alvaro Herrera | 2023-11-27 16:35:25 | Re: Missing docs on AT TIME ZONE precedence? |
| Previous Message | vignesh C | 2023-11-27 16:28:00 | Re: Improve tab completion for ALTER DEFAULT PRIVILEGE and ALTER TABLE |