Thread: [HACKERS] Minor typo in partition.c
Attached patch gets rid of a "is has". Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
On 14 April 2017 at 08:13, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: > Attached patch gets rid of a "is has". Yes, its a typo, but doesn't add missing information or change the meaning. It should be fixed, but could I suggest we include that in your next lot of doc changes, rather than keep making single minor changes? (I had understood that we were going to add more docs together, but I was awaiting your restructuring patch) -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2017/04/14 16:25, Simon Riggs wrote: > On 14 April 2017 at 08:13, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: > >> Attached patch gets rid of a "is has". > > Yes, its a typo, but doesn't add missing information or change the meaning. > > It should be fixed, but could I suggest we include that in your next > lot of doc changes, rather than keep making single minor changes? Sorry, I'm not working on any new doc changes at the moment, because... > (I had understood that we were going to add more docs together, but I > was awaiting your restructuring patch) The restructuring patch was committed on Apr 1st, after I posted the first version of the patch on March 3rd here: https://www.postgresql.org/message-id/4eb8cfe4-b6c9-903d-4ce8-2a31fcee27e1%40lab.ntt.co.jp The committed version: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=8f18a880a5f138d4da94173d15514142331f8de6 Thanks, Amit
On 14 April 2017 at 08:39, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: > On 2017/04/14 16:25, Simon Riggs wrote: >> On 14 April 2017 at 08:13, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: >> >>> Attached patch gets rid of a "is has". >> >> Yes, its a typo, but doesn't add missing information or change the meaning. >> >> It should be fixed, but could I suggest we include that in your next >> lot of doc changes, rather than keep making single minor changes? > > Sorry, I'm not working on any new doc changes at the moment, because... > >> (I had understood that we were going to add more docs together, but I >> was awaiting your restructuring patch) > > The restructuring patch was committed on Apr 1st, after I posted the first > version of the patch on March 3rd here: > > https://www.postgresql.org/message-id/4eb8cfe4-b6c9-903d-4ce8-2a31fcee27e1%40lab.ntt.co.jp > > The committed version: > > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=8f18a880a5f138d4da94173d15514142331f8de6 Oh, just looks very different to what we discussed, so I presumed more changes were coming. Where shall I mention BRIN in that chapter? -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2017/04/14 16:43, Simon Riggs wrote: > On 14 April 2017 at 08:39, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: >> On 2017/04/14 16:25, Simon Riggs wrote: >>> On 14 April 2017 at 08:13, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: >>> >>>> Attached patch gets rid of a "is has". >>> >>> Yes, its a typo, but doesn't add missing information or change the meaning. >>> >>> It should be fixed, but could I suggest we include that in your next >>> lot of doc changes, rather than keep making single minor changes? >> >> Sorry, I'm not working on any new doc changes at the moment, because... >> >>> (I had understood that we were going to add more docs together, but I >>> was awaiting your restructuring patch) >> >> The restructuring patch was committed on Apr 1st, after I posted the first >> version of the patch on March 3rd here: >> >> https://www.postgresql.org/message-id/4eb8cfe4-b6c9-903d-4ce8-2a31fcee27e1%40lab.ntt.co.jp >> >> The committed version: >> >> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=8f18a880a5f138d4da94173d15514142331f8de6 > > Oh, just looks very different to what we discussed, so I presumed more > changes were coming. It looks different perhaps as the result of working through the review comments: https://www.postgresql.org/message-id/CA%2BTgmoawNTaXw2yq01U9c0FxhUnty7avz7V0di0iyiWGkR0tfg%40mail.gmail.com > Where shall I mention BRIN in that chapter? Maybe just append a new <sect2> right below where <sect2 id="ddl-partitioning-constraint-exclusion"> ends? I had included both BRIN and UNION ALL views under a sub-section titled "Alternative Methods" in my original patch (March 3rd one), but removed the whole sub-section per review comments. Perhaps, my patch didn't do a good enough job of describing either BRIN or UNION ALL views, that it ended up being not convincing enough to be made part of the section. By the way, will you also be adding details on UNION ALL views (I remember you said you would along with BRIN)? Actually, my patch copied verbatim what older docs had about them that you said is too short a description. Thanks, Amit
On Fri, Apr 14, 2017 at 3:43 AM, Simon Riggs <simon@2ndquadrant.com> wrote: > Oh, just looks very different to what we discussed, so I presumed more > changes were coming. I waited quite a while for you to review Amit's patches on that thread, but as you never did, I eventually picked it up. > Where shall I mention BRIN in that chapter? You never answered my email about why BRIN belongs in that chapter. I maintain that it doesn't, because BRIN is not a partitioning method. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 14 April 2017 at 14:02, Robert Haas <robertmhaas@gmail.com> wrote: > On Fri, Apr 14, 2017 at 3:43 AM, Simon Riggs <simon@2ndquadrant.com> wrote: >> Oh, just looks very different to what we discussed, so I presumed more >> changes were coming. > > I waited quite a while for you to review Amit's patches on that > thread, but as you never did, I eventually picked it up. Sorry about that, I guess I was concentrating on things listed on the CF app at that time. >> Where shall I mention BRIN in that chapter? > > You never answered my email about why BRIN belongs in that chapter. I > maintain that it doesn't, because BRIN is not a partitioning method. BRIN is a method of speeding up queries by skipping large chunks of tables, just the same as declarative partitioning. There are similarities and differences, which makes sense to highlight. Some DBMS/data access methods support both features, as we do, others only one or the other. Since we support both it makes sense to mention them in the same section of docs. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 14 April 2017 at 09:18, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: >> Where shall I mention BRIN in that chapter? > > Maybe just append a new <sect2> right below where <sect2 > id="ddl-partitioning-constraint-exclusion"> ends? > > I had included both BRIN and UNION ALL views under a sub-section titled > "Alternative Methods" in my original patch (March 3rd one), but removed > the whole sub-section per review comments. Perhaps, my patch didn't do a > good enough job of describing either BRIN or UNION ALL views, that it > ended up being not convincing enough to be made part of the section. > > By the way, will you also be adding details on UNION ALL views (I remember > you said you would along with BRIN)? Actually, my patch copied verbatim > what older docs had about them that you said is too short a description. It makes sense to describe all similar techniques in one section of the docs. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Sat, Apr 15, 2017 at 7:38 AM, Simon Riggs <simon@2ndquadrant.com> wrote: >> I waited quite a while for you to review Amit's patches on that >> thread, but as you never did, I eventually picked it up. > > Sorry about that, I guess I was concentrating on things listed on the > CF app at that time. No problem from my end, just explaining that I did try to give you an opportunity to have a crack at it. >>> Where shall I mention BRIN in that chapter? >> >> You never answered my email about why BRIN belongs in that chapter. I >> maintain that it doesn't, because BRIN is not a partitioning method. > > BRIN is a method of speeding up queries by skipping large chunks of > tables, just the same as declarative partitioning. Well, *any* index speeds up queries by skipping large chunks of a table; that's not unique to BRIN. I suppose if you tilt your head right, you could view partitioning as a very coarse-grained kind of indexing; there are echoes of that in the code. In my opinion, the relationship between BRIN and partitioning is that both features are concerned with locality, but partitioning (and inheritance, and UNION ALL views) create locality, whereas BRIN takes advantage of locality created by some other means (or locality that exists by chance). However, I consider those things different enough that I wouldn't document them together. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company