The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: not tested
Spec compliant: not tested
Documentation:tested, passed
Hello,
I've reviewed your patch and it applies correctly and the doc
On Wed, Nov 15, 2023 at 8:29 PM Roberto Mello
wrote:
>
> Suggestion:
>
> In the C example you added you mention in the comment:
>
> + /* Methods from TableAmRoutine omitted from example, but all
> + non-optional ones must be provided here. */
>
> Perhaps you could provide a "see " to point th
Suggestion:
In the C example you added you mention in the comment:
+ /* Methods from TableAmRoutine omitted from example, but all
+ non-optional ones must be provided here. */
Perhaps you could provide a "see " to point the reader finding your
example where he could find these non-optional
Thanks Robert for mentioning this! I indeed did not notice the switch.
> Nitpicking a little: your patch appears to change more lines than it does,
> because it added line breaks earlier in the lines. I would generally avoid
> that unless there's good reason to do so.
Thanks! I'm not sure why t
On Fri, May 3, 2024 at 1:35 PM Phil Eaton wrote:
> Happy for feedback. Updated patch is attached.
I took a look at this patch and I don't think this is a very good
idea, for two reasons:
1. We change the table access method interface definitions not all
that infrequently, so I think this will be
> I took a look at this patch and I don't think this is a very good
> idea,
No problem! I've dropped the v2 code additions and stuck with the v1
attempt plus feedback.
Thank you!
Phil
v3-0001-Add-minimal-C-example-and-SQL-registration-exampl.patch
Description: Binary data
On Tue, May 14, 2024 at 3:02 PM Phil Eaton wrote:
> > I took a look at this patch and I don't think this is a very good
> > idea,
>
> No problem! I've dropped the v2 code additions and stuck with the v1
> attempt plus feedback.
That looks more reasonable. I'd like to quibble with this text:
+. H
> I think this should say something more like "Here is how an extension
> SQL script might create a table access method handler".
Fair point. It is referred to elsewhere [0] in docs as a "script
file", so I've done that.
> Shouldn't "mem_tableam_handler" be "my_tableam_handler"?
Sorry about that
On Fri, May 24, 2024 at 3:11 PM Phil Eaton wrote:
> > I think this should say something more like "Here is how an extension
> > SQL script might create a table access method handler".
>
> Fair point. It is referred to elsewhere [0] in docs as a "script
> file", so I've done that.
>
> > Shouldn't
I noticed that there were two CF entries pointing at this thread:
https://commitfest.postgresql.org/48/4655/
https://commitfest.postgresql.org/48/4973/
That doesn't seem helpful, so I've marked the second one "Withdrawn".
regards, tom lane
On Fri, Jan 26, 2024 at 3:03 PM Fabrízio de Royes Mello
wrote:
> On Wed, Nov 15, 2023 at 8:29 PM Roberto Mello wrote:
> > Suggestion:
> >
> > In the C example you added you mention in the comment:
> >
> > + /* Methods from TableAmRoutine omitted from example, but all
> > + non-optional ones
11 matches
Mail list logo
