| From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
|---|---|
| To: | Nikolay Shaplov <dhyan(at)nataraj(dot)su> |
| Cc: | Dent John <denty(at)qqdd(dot)eu>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Iwata, Aya" <iwata(dot)aya(at)jp(dot)fujitsu(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz> |
| Subject: | Re: [PATCH] Do not use StdRdOptions in Access Methods |
| Date: | 2019-10-10 08:17:30 |
| Message-ID: | CA+HiwqF5Mx55bGkpQPzdre9FvSsHFSF8OHDyvDc+gCArsGhBOg@mail.gmail.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hello Nikolay,
I read comments that Tomas left at:
https://www.postgresql.org/message-id/20190727173841.7ypzo4xuzizvijge%40development
I'd like to join Michael in reiterating one point from Tomas' review.
I think the patch can go further in trying to make the code in this
area more maintainable.
For example, even without this patch, the following stanza is repeated
in many places:
options = parseRelOptions(reloptions, validate, foo_relopt_kind,
&numoptions);
rdopts = allocateReloptStruct(sizeof(FooOptions), options, numoptions);
fillRelOptions((void *) rdopts, sizeof(FooOptions), options, numoptions,
validate, foo_relopt_tab, lengthof(foo_relopt_tab));
return (bytea *) rdopts;
and this patch adds few more instances as it's adding more Options structs.
I think it wouldn't be hard to encapsulate the above stanza in a new
public function in reloptions.c and call it from the various places
that now have the above code.
Thanks,
Amit
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Langote | 2019-10-10 08:28:02 | Re: dropping column prevented due to inherited index |
| Previous Message | Michael Paquier | 2019-10-10 08:03:16 | Re: use of the term "verifier" with SCRAM |