Re: [HACKERS] pageinspect and hash indexes

2017-04-05 Thread Robert Haas
On Fri, Mar 24, 2017 at 3:44 PM, Ashutosh Sharma wrote: > Agreed. Moreover, previous approach might even change the current > behaviour of functions like hash_page_items() and hash_page_stats() > basically when we pass UNUSED pages to these functions. Attached is > the

Re: [HACKERS] pageinspect and hash indexes

2017-03-24 Thread Ashutosh Sharma
Thanks for reviewing my patch. I have removed the extra white space. Attached are both the patches. >>> >>> Sorry, I have mistakenly attached wrong patch. Here are the correct >>> set of patches. >> >> The latest version of patches looks fine to me. > > I don't really like 0002. What

Re: [HACKERS] pageinspect and hash indexes

2017-03-24 Thread Robert Haas
On Fri, Mar 24, 2017 at 3:54 AM, Amit Kapila wrote: > On Fri, Mar 24, 2017 at 9:50 AM, Ashutosh Sharma > wrote: >> On Fri, Mar 24, 2017 at 9:46 AM, Ashutosh Sharma >> wrote: >>> >>> Thanks for reviewing my patch. I have

Re: [HACKERS] pageinspect and hash indexes

2017-03-24 Thread Amit Kapila
On Fri, Mar 24, 2017 at 9:50 AM, Ashutosh Sharma wrote: > On Fri, Mar 24, 2017 at 9:46 AM, Ashutosh Sharma > wrote: >> >> Thanks for reviewing my patch. I have removed the extra white space. >> Attached are both the patches. > > Sorry, I have

Re: [HACKERS] pageinspect and hash indexes

2017-03-23 Thread Ashutosh Sharma
On Fri, Mar 24, 2017 at 9:46 AM, Ashutosh Sharma wrote: > On Fri, Mar 24, 2017 at 9:21 AM, Amit Kapila wrote: >> On Thu, Mar 23, 2017 at 6:46 PM, Ashutosh Sharma >> wrote: Oh, okay, but my main objection was that

Re: [HACKERS] pageinspect and hash indexes

2017-03-23 Thread Ashutosh Sharma
On Fri, Mar 24, 2017 at 9:21 AM, Amit Kapila wrote: > On Thu, Mar 23, 2017 at 6:46 PM, Ashutosh Sharma > wrote: >>> >>> Oh, okay, but my main objection was that we should not check hash page >>> type (hasho_flag) without ensuring whether it is a

Re: [HACKERS] pageinspect and hash indexes

2017-03-23 Thread Amit Kapila
On Thu, Mar 23, 2017 at 6:46 PM, Ashutosh Sharma wrote: >> >> Oh, okay, but my main objection was that we should not check hash page >> type (hasho_flag) without ensuring whether it is a hash page. Can you >> try to adjust the above code so that this check can be moved

Re: [HACKERS] pageinspect and hash indexes

2017-03-23 Thread Ashutosh Sharma
Hi Amit, >>> + >>> + /* Check if it is an unused hash page. */ >>> + if (pageopaque->hasho_flag == LH_UNUSED_PAGE) >>> + return page; >>> >>> >>> I don't see we need to have a separate check for unused page, why >>> can't it be checked with other page types in below check. >>> >>> if (pagetype !=

Re: [HACKERS] pageinspect and hash indexes

2017-03-23 Thread Amit Kapila
On Thu, Mar 23, 2017 at 10:02 AM, Ashutosh Sharma wrote: >>> >> >> 1. >> @@ -70,6 +70,17 @@ verify_hash_page(bytea *raw_page, int flags) >> pageopaque = (HashPageOpaque) PageGetSpecialPointer(page); >> + >> + /* Check if it is an unused hash page. */ >> + if

Re: [HACKERS] pageinspect and hash indexes

2017-03-22 Thread Ashutosh Sharma
>> I think it is not just happening for freed overflow but also for newly >> allocated bucket page. It would be good if we could mark freed >> overflow page as UNUSED page rather than just initialising it's header >> portion and leaving the page type in special area as it is. Attached >> is the

Re: [HACKERS] pageinspect and hash indexes

2017-03-22 Thread Amit Kapila
On Mon, Mar 20, 2017 at 6:53 PM, Ashutosh Sharma wrote: > > I think it is not just happening for freed overflow but also for newly > allocated bucket page. It would be good if we could mark freed > overflow page as UNUSED page rather than just initialising it's header >

Re: [HACKERS] pageinspect and hash indexes

2017-03-21 Thread Amit Kapila
On Tue, Mar 21, 2017 at 10:15 AM, Ashutosh Sharma wrote: >> >> I think it is not just happening for freed overflow but also for newly >> allocated bucket page. It would be good if we could mark freed >> overflow page as UNUSED page rather than just initialising it's header

Re: [HACKERS] pageinspect and hash indexes

2017-03-20 Thread Ashutosh Sharma
On Mon, Mar 20, 2017 at 6:53 PM, Ashutosh Sharma wrote: > On Mon, Mar 20, 2017 at 9:31 AM, Amit Kapila wrote: >> On Sat, Mar 18, 2017 at 5:13 PM, Ashutosh Sharma >> wrote: >>> On Sat, Mar 18, 2017 at 1:34 PM, Amit Kapila

Re: [HACKERS] pageinspect and hash indexes

2017-03-20 Thread Ashutosh Sharma
On Mon, Mar 20, 2017 at 9:31 AM, Amit Kapila wrote: > On Sat, Mar 18, 2017 at 5:13 PM, Ashutosh Sharma > wrote: >> On Sat, Mar 18, 2017 at 1:34 PM, Amit Kapila wrote: >>> On Sat, Mar 18, 2017 at 12:12 AM, Ashutosh Sharma

Re: [HACKERS] pageinspect and hash indexes

2017-03-19 Thread Amit Kapila
On Sat, Mar 18, 2017 at 5:13 PM, Ashutosh Sharma wrote: > On Sat, Mar 18, 2017 at 1:34 PM, Amit Kapila wrote: >> On Sat, Mar 18, 2017 at 12:12 AM, Ashutosh Sharma >> wrote: >>> On Fri, Mar 17, 2017 at 10:54 PM, Jeff Janes

Re: [HACKERS] pageinspect and hash indexes

2017-03-18 Thread Ashutosh Sharma
On Sat, Mar 18, 2017 at 1:34 PM, Amit Kapila wrote: > On Sat, Mar 18, 2017 at 12:12 AM, Ashutosh Sharma > wrote: >> On Fri, Mar 17, 2017 at 10:54 PM, Jeff Janes wrote: >>> While trying to figure out some bloating in the

Re: [HACKERS] pageinspect and hash indexes

2017-03-18 Thread Amit Kapila
On Sat, Mar 18, 2017 at 12:12 AM, Ashutosh Sharma wrote: > On Fri, Mar 17, 2017 at 10:54 PM, Jeff Janes wrote: >> While trying to figure out some bloating in the newly logged hash indexes, >> I'm looking into the type of each page in the index. But I

Re: [HACKERS] pageinspect and hash indexes

2017-03-18 Thread Amit Kapila
On Sat, Mar 18, 2017 at 1:42 AM, Tom Lane wrote: > Ashutosh Sharma writes: >> Basically, when we started working on WAL for hash index, we found >> that WAL routine 'XLogReadBufferExtended' does not expect a page to be >> completely zero page else it

Re: [HACKERS] pageinspect and hash indexes

2017-03-17 Thread Tom Lane
Ashutosh Sharma writes: > Basically, when we started working on WAL for hash index, we found > that WAL routine 'XLogReadBufferExtended' does not expect a page to be > completely zero page else it returns Invalid Buffer. To fix this, we > started initializing freed overflow

Re: [HACKERS] pageinspect and hash indexes

2017-03-17 Thread Peter Eisentraut
On 3/17/17 13:24, Jeff Janes wrote: > And isn't it unhelpful to have the pageinspect module throw errors, > rather than returning a dummy value to indicate there was an error? Even if one agreed with that premise, it would be hard to satisfy reliably, because the user-facing pageinspect functions

Re: [HACKERS] pageinspect and hash indexes

2017-03-17 Thread Ashutosh Sharma
On Fri, Mar 17, 2017 at 10:54 PM, Jeff Janes wrote: > While trying to figure out some bloating in the newly logged hash indexes, > I'm looking into the type of each page in the index. But I get an error: > > psql -p 9876 -c "select

[HACKERS] pageinspect and hash indexes

2017-03-17 Thread Jeff Janes
While trying to figure out some bloating in the newly logged hash indexes, I'm looking into the type of each page in the index. But I get an error: psql -p 9876 -c "select hash_page_type(get_raw_page('foo_index_idx',x)) from generate_series(1650,1650) f(x)" ERROR: page is not a hash page