On Fri, Nov 22, 2019 at 04:44:50PM +0900, Michael Paquier wrote:
> Noted, thanks. I'll sit on this thing for a couple of days, and will
> likely look at it again on Monday in order to commit it.
And done.
--
Michael
signature.asc
Description: PGP signature
On Fri, Nov 22, 2019 at 11:01:54AM +0900, Amit Langote wrote:
> The latest patch looks good to me, except, maybe the comment of
> StdRdOptions should be updated:
>
> * StdRdOptions
> * Standard contents of rd_options for heaps and generic indexes.
>
> IIUC, StdRdOptions no longer applies t
On Thu, Nov 21, 2019 at 09:39:53PM +0300, Nikolay Shaplov wrote:
> BRIN_AM_OID and friends are defined in catalog/pg_am_d.h so for core indexes
> we can do relation->rd_rel->relam == BRIN_AM_OID check. But for contrib
> indexes we can't do such a thing.
> Bloom index does not need such check as i
Hello,
On Thu, Nov 21, 2019 at 2:22 PM Michael Paquier wrote:
> Any opinions or objections to share regarding the recent
> progress done?
The latest patch looks good to me, except, maybe the comment of
StdRdOptions should be updated:
* StdRdOptions
* Standard contents of rd_options for h
В письме от среда, 20 ноября 2019 г. 16:44:18 MSK пользователь Michael Paquier
написал:
> > It seems to me that if the plan is to have one option structure for
> > each index AM, which has actually the advantage to reduce the bloat of
> > each relcache entry currently relying on StdRdOptions, the
On Wed, Nov 20, 2019 at 04:44:18PM +0900, Michael Paquier wrote:
> We can do something similar for GIN and BRIN on top of the rest, so
> updated the patch with that. Nikolay, I would be fine to commit this
> patch as-is. Thanks for your patience on this stuff.
So, I have reviewed the full thread
On Fri, Nov 15, 2019 at 10:34:55AM +0900, Michael Paquier wrote:
> It seems to me that if the plan is to have one option structure for
> each index AM, which has actually the advantage to reduce the bloat of
> each relcache entry currently relying on StdRdOptions, then we could
> have those extra a
On Thu, Nov 14, 2019 at 11:20:25AM +0300, Nikolay Shaplov wrote:
> For me there is no mush sense in it, as it does not prevent us from wrong
> type
> casting. Indexes can use all kinds of structures for reloptions, and checking
> that this is index, will not do much.
It seems to me that if the
В письме от четверг, 14 ноября 2019 г. 16:50:18 MSK пользователь Michael
Paquier написал:
> > I've changed the patch to use build_reloptions function and again propose
> > it to the commitfest.
>
> Thanks for the new patch. I have not reviewed the patch in details,
> but I have a small comment.
On Wed, Nov 13, 2019 at 04:26:53PM +0300, Nikolay Shaplov wrote:
> I've changed the patch to use build_reloptions function and again propose it
> to the commitfest.
Thanks for the new patch. I have not reviewed the patch in details,
but I have a small comment.
> +#define SpGistGetFillFactor(rel
On Wed, Nov 13, 2019 at 04:05:20PM +0900, Michael Paquier wrote:
> On Wed, Nov 13, 2019 at 02:29:49PM +0900, Amit Langote wrote:
> > On Wed, Nov 13, 2019 at 2:18 PM Michael Paquier wrote:
> >> There could be an argument for keeping
> >> GET_STRING_RELOPTION actually which is still useful to get a
В письме от среда, 13 ноября 2019 г. 16:05:20 MSK пользователь Michael Paquier
написал:
Guys! Sorry for being away for so long. I did not have much opportunities to
pay my attention to postgres recently.
Thank you for introducing build_reloptions function. It is approximately the
direction I w
On Wed, Nov 13, 2019 at 02:29:49PM +0900, Amit Langote wrote:
> On Wed, Nov 13, 2019 at 2:18 PM Michael Paquier wrote:
>> There could be an argument for keeping
>> GET_STRING_RELOPTION actually which is still useful to get a string
>> value in an option set using the stored offset, and we have
>>
On Wed, Nov 13, 2019 at 2:18 PM Michael Paquier wrote:
> On Wed, Nov 13, 2019 at 10:52:52AM +0900, Amit Langote wrote:
> > Thanks for chiming in about that. I guess that means that we don't
> > need those macros, except GET_STRING_RELOPTION_LEN() that's used in
> > allocateReloptStruct(), which c
On Wed, Nov 13, 2019 at 10:52:52AM +0900, Amit Langote wrote:
> Thanks for chiming in about that. I guess that means that we don't
> need those macros, except GET_STRING_RELOPTION_LEN() that's used in
> allocateReloptStruct(), which can be moved to reloptions.c. Is that
> correct?
I have been lo
Hi Alvaro,
On Wed, Nov 13, 2019 at 6:55 AM Alvaro Herrera wrote:
> On 2019-Nov-07, Amit Langote wrote:
> > On Tue, Nov 5, 2019 at 9:22 AM Michael Paquier wrote:
> > > Please note that I have not switched the old interface
> > > to be static to reloptions.c as if you look closely at reloptions.h
On 2019-Nov-07, Amit Langote wrote:
> On Tue, Nov 5, 2019 at 9:22 AM Michael Paquier wrote:
> > Please note that I have not switched the old interface
> > to be static to reloptions.c as if you look closely at reloptions.h we
> > allow much more flexibility around HANDLE_INT_RELOPTION to fill a
On Thu, Nov 7, 2019 at 10:54 AM Michael Paquier wrote:
> On Thu, Nov 07, 2019 at 10:49:38AM +0900, Amit Langote wrote:
> > I looked around but don't understand why these macros need to be
> > exposed. I read this comment:
> >
> > * Note that this is more or less the same that fillRelOptions doe
On Thu, Nov 07, 2019 at 10:49:38AM +0900, Amit Langote wrote:
> I looked around but don't understand why these macros need to be
> exposed. I read this comment:
>
> * Note that this is more or less the same that fillRelOptions does, so only
> * use this if you need to do something non-standar
On Tue, Nov 5, 2019 at 9:22 AM Michael Paquier wrote:
> On Thu, Oct 31, 2019 at 05:55:55PM +0900, Michael Paquier wrote:
> > Thanks. I exactly did the same thing on my local branch.
>
> Hearing nothing more, I have done some adjustments to the patch and
> committed it.
Thank you.
> Please note
On Thu, Oct 31, 2019 at 05:55:55PM +0900, Michael Paquier wrote:
> Thanks. I exactly did the same thing on my local branch.
Hearing nothing more, I have done some adjustments to the patch and
committed it. Please note that I have not switched the old interface
to be static to reloptions.c as if
On Thu, Oct 31, 2019 at 05:18:46PM +0900, Amit Langote wrote:
> On Thu, Oct 31, 2019 at 4:49 PM Michael Paquier wrote:
>> Let's remove it then.
>
> Removed in the attached.
Thanks. I exactly did the same thing on my local branch.
--
Michael
signature.asc
Description: PGP signature
On Thu, Oct 31, 2019 at 4:49 PM Michael Paquier wrote:
> On Thu, Oct 31, 2019 at 04:38:55PM +0900, Amit Langote wrote:
> > On Wed, Oct 30, 2019 at 12:11 PM Michael Paquier
> > wrote:
> > This sentence sounds wrong, because the result structure doesn't
> > contain values in text-array format. In
On Thu, Oct 31, 2019 at 04:38:55PM +0900, Amit Langote wrote:
> On Wed, Oct 30, 2019 at 12:11 PM Michael Paquier wrote:
> This sentence sounds wrong, because the result structure doesn't
> contain values in text-array format. Individual values in the struct
> would be in their native format (C bo
Hi Michael,
On Wed, Oct 30, 2019 at 12:11 PM Michael Paquier wrote:
> Looks fine. I have done some refinements as per the attached.
Thanks. This stood out to me:
+ * The result is a structure containing all the parsed option values in
+ * text-array format.
This sentence sounds wrong, becaus
On Mon, Oct 28, 2019 at 05:16:54PM +0900, Amit Langote wrote:
> On Sat, Oct 26, 2019 at 11:45 AM Michael Paquier wrote:
>> On Fri, Oct 25, 2019 at 04:42:24PM +0900, Amit Langote wrote:
>> > Hmm, if we're inventing a new API to replace the old one, why not use
>> > that opportunity to be consistent
Hi Alvaro,
On Tue, Oct 29, 2019 at 12:02 AM Alvaro Herrera
wrote:
> On 2019-Oct-23, Michael Paquier wrote:
> > On Wed, Oct 23, 2019 at 11:16:25AM +0900, Amit Langote wrote:
> > > IMO, parts of the patch that only refactors the existing code should
> > > be first in the list as it is easier to rev
On Mon, Oct 28, 2019 at 12:02:20PM -0300, Alvaro Herrera wrote:
> I also think that this has value -- let's go for it. I think I'll be
> back on Wednesday to review it, if you would prefer to wait.
No worries, thanks for looking it.
--
Michael
signature.asc
Description: PGP signature
On 2019-Oct-23, Michael Paquier wrote:
> On Wed, Oct 23, 2019 at 11:16:25AM +0900, Amit Langote wrote:
> > IMO, parts of the patch that only refactors the existing code should
> > be first in the list as it is easier to review, especially if it adds
> > no new concepts. In this case, your patch t
On Sat, Oct 26, 2019 at 11:45 AM Michael Paquier wrote:
> On Fri, Oct 25, 2019 at 04:42:24PM +0900, Amit Langote wrote:
> > Hmm, if we're inventing a new API to replace the old one, why not use
> > that opportunity to be consistent with our general style, which
> > predominantly seems to be either
On Fri, Oct 25, 2019 at 04:42:24PM +0900, Amit Langote wrote:
> Hmm, if we're inventing a new API to replace the old one, why not use
> that opportunity to be consistent with our general style, which
> predominantly seems to be either words_separated_by_underscore() or
> UpperCamelCase(). Thoughts
Hi Michael,
Thanks for taking a look at this.
On Wed, Oct 23, 2019 at 12:51 PM Michael Paquier wrote:
> On Wed, Oct 23, 2019 at 11:16:25AM +0900, Amit Langote wrote:
> > IMO, parts of the patch that only refactors the existing code should
> > be first in the list as it is easier to review, espec
On Wed, Oct 23, 2019 at 11:16:25AM +0900, Amit Langote wrote:
> IMO, parts of the patch that only refactors the existing code should
> be first in the list as it is easier to review, especially if it adds
> no new concepts. In this case, your patch to break StdRdOptions into
> more manageable chun
Hi Nikolay,
Sorry for the late reply.
On Fri, Oct 11, 2019 at 7:54 PM Nikolay Shaplov wrote:
> В письме от четверг, 10 октября 2019 г. 17:17:30 MSK пользователь Amit Langote
> написал:
> > I read comments that Tomas left at:
> > https://www.postgresql.org/message-id/20190727173841.7ypzo4xuzizvij
В письме от четверг, 10 октября 2019 г. 17:17:30 MSK пользователь Amit Langote
написал:
> I read comments that Tomas left at:
> https://www.postgresql.org/message-id/20190727173841.7ypzo4xuzizvijge%40deve
> lopment
>
> I'd like to join Michael in reiterating one point from Tomas' review.
> I thi
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.
В письме от среда, 9 октября 2019 г. 20:26:14 MSK пользователь Dent John
написал:
> I didn’t spot it was an existing pattern.
Sorry, this might be my mistake I should explicitly mention it in the first
place.
> And I agree — making the code uniform will make it easier to evolve in
> future.
>
> On 8 Oct 2019, at 11:33, Nikolay Shaplov wrote:
>
> Whoa-whoa!
>
> I am not inventing something new here. Same code is already used in brin
> (brin.c:820), gin (ginutils.c:602) and gist (gistutils.c:909) indexes. I've
> just copied the idea, to do all index code uniform.
>
> This does not m
В письме от понедельник, 7 октября 2019 г. 18:55:20 MSK пользователь Dent John
написал:
> I like the new approach. And I agree with the ambition — to split out the
> representation from StdRdOptions.
Thanks.
> However, with that change, in the AM’s *options() function, it looks as if
> you coul
Hi Nikolay,
I like the new approach. And I agree with the ambition — to split out the
representation from StdRdOptions.
However, with that change, in the AM’s *options() function, it looks as if you
could simply add new fields to the relopt_parse_elt list. That’s still not
true, because parseR
Hi! I am starting a new thread as commitfest wants new thread for new patch.
This new thread is a follow up to an https://www.postgresql.org/message-id/
2620882.s52SJui4ql%40x200m thread, where I've been trying to get rid of
StdRdOpions structure, and now I've splitted the patch into smaller part
В Fri, 27 Sep 2019 17:24:49 +0900
Michael Paquier пишет:
> > Looks like some good actionable feedback. I've moved this patch to
> > September, and set it to "Waiting on Author".
>
> The patch is in this state for two months now, so I have switched it
> to "returned with feedback". The latest pa
42 matches
Mail list logo
