Re: [HACKERS] pageinspect: Hash index support

On Tue, Feb 21, 2017 at 3:14 PM, Ashutosh Sharma wrote: > Thanks for reporting it. This is because of incorrect data typecasting. > Attached is the patch that fixes this issue. Oops, that's probably my fault. Thanks for the patch; committed. -- Robert Haas EnterpriseDB:

Re: [HACKERS] pageinspect: Hash index support

Thanks for reporting it. This is because of incorrect data typecasting. Attached is the patch that fixes this issue. On Tue, Feb 21, 2017 at 2:58 PM, Mithun Cy wrote: > On Fri, Feb 10, 2017 at 1:06 AM, Robert Haas > wrote: > > > Alright,

Re: [HACKERS] pageinspect: Hash index support

On Fri, Feb 10, 2017 at 1:06 AM, Robert Haas wrote: > Alright, committed with a little further hacking. I did pull the latest code, and tried Test: create table t1(t int); create index i1 on t1 using hash(t); insert into t1 select generate_series(1, 1000);

Re: [HACKERS] pageinspect: Hash index support

On Thu, Feb 9, 2017 at 1:11 PM, Ashutosh Sharma wrote: >> I think you should just tighten up the sanity checking in the existing >> function _hash_ovflblkno_to_bitno rather than duplicating the code. I >> don't think it's called often enough for one extra (cheap) test to

Re: [HACKERS] pageinspect: Hash index support

> I think you should just tighten up the sanity checking in the existing > function _hash_ovflblkno_to_bitno rather than duplicating the code. I > don't think it's called often enough for one extra (cheap) test to be > an issue. (You should change the elog in that function to an ereport, > too,

Re: [HACKERS] pageinspect: Hash index support

On Thu, Feb 9, 2017 at 8:56 AM, Ashutosh Sharma wrote: >> If you want to verify that the supplied page number is an overflow >> page before returning the bit, I think you should do that calculation >> based on the metapage. There's enough information in hashm_spares to >>

Re: [HACKERS] pageinspect: Hash index support

On Wed, Feb 8, 2017 at 11:26 PM, Robert Haas wrote: > On Wed, Feb 8, 2017 at 11:58 AM, Ashutosh Sharma > wrote: >>> And then, instead, you need to add some code to set bit based on the >>> bitmap page, like what you have: >>> >>> +mapbuf =

Re: [HACKERS] pageinspect: Hash index support

On Wed, Feb 8, 2017 at 9:25 AM, Ashutosh Sharma wrote: >>> 1) Check if an overflow page is a new page. If so, read a bitmap page >>> to confirm if a bit corresponding to this overflow page is clear or >>> not. For this, I would first add Assert statement to ensure that the

Re: [HACKERS] pageinspect: Hash index support

>> 1) Check if an overflow page is a new page. If so, read a bitmap page >> to confirm if a bit corresponding to this overflow page is clear or >> not. For this, I would first add Assert statement to ensure that the >> bit is clear and if it is, then set the statusbit as false indicating >> that

Re: [HACKERS] pageinspect: Hash index support

On Sat, Feb 4, 2017 at 7:06 AM, Ashutosh Sharma wrote: >> As far as I can tell, the hash_bitmap_info() function is doing >> something completely ridiculous. One would expect that the purpose of >> this function was to tell you about the status of pages in the bitmap. >>

Re: [HACKERS] pageinspect: Hash index support

> As far as I can tell, the hash_bitmap_info() function is doing > something completely ridiculous. One would expect that the purpose of > this function was to tell you about the status of pages in the bitmap. > The documentation claims that this is what the function does: it > claims that this

Re: [HACKERS] pageinspect: Hash index support

On Sat, Jan 28, 2017 at 10:25 PM, Ashutosh Sharma wrote: > Hi, > > Please find my reply inline. > >> In hash_bimap_info(), we go to the trouble of creating a raw page but >> never do anything with it. I guess the idea here is just to error out >> if the supplied page

Re: [HACKERS] pageinspect: Hash index support

On Fri, Feb 3, 2017 at 11:49 AM, Jesper Pedersen wrote: > Disregard, as Tom has committed a fix. So we're six commits into this mess now and I'm hopeful that we've got most of the problems with type selection and crashing fixed now. However, I discovered another thing

Re: [HACKERS] pageinspect: Hash index support

On 02/03/2017 11:41 AM, Jesper Pedersen wrote: contrib/pageinspect actually seems to lack *any* infrastructure for sharing functions across modules. It's time to remedy that. I propose inventing "pageinspect.h" to hold commonly visible declarations, and moving get_page_from_raw() into

Re: [HACKERS] pageinspect: Hash index support

Hi, On 02/03/2017 11:36 AM, Robert Haas wrote: On Fri, Feb 3, 2017 at 10:11 AM, Tom Lane wrote: BTW, the buildfarm is still crashing on ia64 and sparc64 members. I believe this is the same problem addressed in 84ad68d64 for pageinspect's GIN functions, to wit, the payload

Re: [HACKERS] pageinspect: Hash index support

On Fri, Feb 3, 2017 at 10:11 AM, Tom Lane wrote: > BTW, the buildfarm is still crashing on ia64 and sparc64 members. > I believe this is the same problem addressed in 84ad68d64 for > pageinspect's GIN functions, to wit, the payload of a "bytea" > is not maxaligned, so machines

Re: [HACKERS] pageinspect: Hash index support

BTW, the buildfarm is still crashing on ia64 and sparc64 members. I believe this is the same problem addressed in 84ad68d64 for pageinspect's GIN functions, to wit, the payload of a "bytea" is not maxaligned, so machines that care about alignment won't be happy when trying to fetch 64-bit values

Re: [HACKERS] pageinspect: Hash index support

On 02/02/2017 02:28 PM, Jesper Pedersen wrote: On 02/02/2017 02:24 PM, Robert Haas wrote: So, committed. Wow, I wish every patch had this many reviewers. Thanks Robert ! This message should have included a thank you to everybody who provided valuable feedback for this feature, and for

Re: [HACKERS] pageinspect: Hash index support

>> I think it's OK to check hash_bitmap_info() or any other functions >> with different page types at least once. >> >> [1]- >> https://www.postgresql.org/message-id/CA%2BTgmoZUjrVy52TUU3b_Q5L6jcrt7w5R4qFwMXdeCuKQBmYWqQ%40mail.gmail.com > > Sure, I just don't know if we need to test them 4 or 5

Re: [HACKERS] pageinspect: Hash index support

On Fri, Feb 3, 2017 at 4:28 AM, Jesper Pedersen wrote: > On 02/02/2017 02:24 PM, Robert Haas wrote: >> >> So, committed. Wow, I wish every patch had this many reviewers. >> > > Thanks Robert ! 9 people in total per the commit message. Yes that's rare, and thanks for

Re: [HACKERS] pageinspect: Hash index support

On 02/02/2017 02:24 PM, Robert Haas wrote: So, committed. Wow, I wish every patch had this many reviewers. Thanks Robert ! Best regards, Jesper -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] pageinspect: Hash index support

On Thu, Feb 2, 2017 at 6:29 AM, Ashutosh Sharma wrote: >> >> + ptr = (char *) itup + IndexInfoFindDataOffset(itup->t_info); >> + Assert(ptr <= uargs->page + BLCKSZ); >> >> I think this should be promoted to an ereport(); these functions can >>

Re: [HACKERS] pageinspect: Hash index support

> > + ptr = (char *) itup + IndexInfoFindDataOffset(itup->t_info); > + Assert(ptr <= uargs->page + BLCKSZ); > > I think this should be promoted to an ereport(); these functions can > accept an arbitrary bytea. I think we had added 'ptr' variable initially just to dump

Re: [HACKERS] pageinspect: Hash index support

On Sat, Jan 28, 2017 at 9:09 PM, Ashutosh Sharma wrote: > okay. Thanks. I have done changes on top of this patch. + ptr = (char *) itup + IndexInfoFindDataOffset(itup->t_info); + Assert(ptr <= uargs->page + BLCKSZ); I think this should be

Re: [HACKERS] pageinspect: Hash index support

On Sun, Jan 29, 2017 at 11:09 AM, Ashutosh Sharma wrote: > okay. Thanks. I have done changes on top of this patch. Moved to CF 2017-03 as there is a new patch, no reviews. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes

Re: [HACKERS] pageinspect: Hash index support

Hi, > Please include tests in your patch. I have posted a sample test suite > in > , > which you could use. > > Also, as mentioned before, hash_metapage_info result fields spares and > mapp should be

Re: [HACKERS] pageinspect: Hash index support

> The heap tuple is on page 3407872 at line pointer offset 12584? > That's an awfully but not implausibly big page number (about 26GB into > the relation) and an awfully and implausibly large line pointer offset > (how do we fit 12584 index tuples into an 8192-byte page?). Also, the > fact that

Re: [HACKERS] pageinspect: Hash index support

Hi, Please find my reply inline. > In hash_bimap_info(), we go to the trouble of creating a raw page but > never do anything with it. I guess the idea here is just to error out > if the supplied page number is not an overflow page, but there are no > comments explaining that. Anyway, I'm not

Re: [HACKERS] pageinspect: Hash index support

On 1/18/17 10:45 AM, Jesper Pedersen wrote: > Fixed in this version: > > * verify_hash_page: Display magic in hex, like hash_metapage_info > * Update header for hash_page_type > > Moving the patch back to 'Needs Review'. Please include tests in your patch. I have posted a sample test suite in

Re: [HACKERS] pageinspect: Hash index support

On Tue, Jan 24, 2017 at 9:59 PM, Mithun Cy wrote: > On Thu, Jan 19, 2017 at 5:08 PM, Ashutosh Sharma > wrote: > > Thanks, Ashutosh and Jesper. I have tested the patch I do not have any > more comments so making it ready for committer. I took a

Re: [HACKERS] pageinspect: Hash index support

On Thu, Jan 19, 2017 at 5:08 PM, Ashutosh Sharma wrote: Thanks, Ashutosh and Jesper. I have tested the patch I do not have any more comments so making it ready for committer. -- Thanks and Regards Mithun C Y EnterpriseDB: http://www.enterprisedb.com -- Sent via

Re: [HACKERS] pageinspect: Hash index support

On Tue, Jan 24, 2017 at 11:41 AM, Ashutosh Sharma wrote: >>> Secondly, we will have to input overflow block number as an input to >>> this function so as to determine the overflow bit number which can be >>> used further to identify the bitmap page. >>> >> >> I think you

Re: [HACKERS] pageinspect: Hash index support

>> Secondly, we will have to input overflow block number as an input to >> this function so as to determine the overflow bit number which can be >> used further to identify the bitmap page. >> > > I think you can get that from bucket number by using BUCKET_TO_BLKNO. > You can get bucket number

Re: [HACKERS] pageinspect: Hash index support

On Wed, Jan 18, 2017 at 3:24 PM, Ashutosh Sharma wrote: > >> 4. >> +Datum >> +hash_page_items(PG_FUNCTION_ARGS) >> +{ >> + bytea *raw_page = PG_GETARG_BYTEA_P(0); >> >> >> +Datum >> +hash_bitmap_info(PG_FUNCTION_ARGS) >> +{ >> + Oid indexRelid = PG_GETARG_OID(0); >> +

Re: [HACKERS] pageinspect: Hash index support

Hi, I got some 'trailing whitespace' error (shown below) when git applying v7 patch attached upthread. I have corrected the same and also ran pgindent on a new file 'hashfuncs.c' added as a part of this project. Attached is the updated v8 patch.

Re: [HACKERS] pageinspect: Hash index support

Hi, On 01/18/2017 04:54 AM, Ashutosh Sharma wrote: Is there a reason for keeping the input arguments for hash_bitmap_info() different from hash_page_items()? Yes, there are two reasons behind it. Firstly, we need metapage to identify the bitmap page that holds the information about the

Re: [HACKERS] pageinspect: Hash index support

Hi, > 1. > +static Page > +verify_hash_page(bytea *raw_page, int flags) > > Few checks for meta page are missing, refer _hash_checkpage. okay, I have added the checks for meta page as well. Please refer to attached patch. > > 2. > + if (!superuser()) > + ereport(ERROR, > +

Re: [HACKERS] pageinspect: Hash index support

On Fri, Jan 13, 2017 at 12:49 AM, Jesper Pedersen wrote: > Hi, > > On 01/11/2017 03:16 PM, Ashutosh Sharma wrote: >> >> >> I have rephrased it to make it more clear. >> > > Rebased, and removed the compile warn in hashfuncs.c > Review comments: 1. +static Page

Re: [HACKERS] pageinspect: Hash index support

On Tue, Jan 17, 2017 at 3:06 PM, Amit Kapila wrote: > > I think your calculation is not right. 66 indicates LH_BUCKET_PAGE | > LH_BUCKET_NEEDS_SPLIT_CLEANUP which is a valid state after the split. > This flag will be cleared either during next split or when vacuum >

Re: [HACKERS] pageinspect: Hash index support

On Tue, Jan 17, 2017 at 1:22 PM, Mithun Cy wrote: >> >> 7. I think it is not your bug, but probably a bug in Hash index >> itself; page flag is set to 66 (for below test); So the page is both >> LH_BUCKET_PAGE and LH_BITMAP_PAGE. Is not this a bug in hash index? >> >>

Re: [HACKERS] pageinspect: Hash index support

> > 7. I think it is not your bug, but probably a bug in Hash index > itself; page flag is set to 66 (for below test); So the page is both > LH_BUCKET_PAGE and LH_BITMAP_PAGE. Is not this a bug in hash index? > > I have inserted 3000 records. Hash index is on integer column. > select hasho_flag

Re: [HACKERS] pageinspect: Hash index support

On Fri, Jan 13, 2017 at 12:49 AM, Jesper Pedersen wrote: > Rebased, and removed the compile warn in hashfuncs.c I did some testing and review for the patch. I did not see any major issue, but there are few minor cases for which I have few suggestions. 1. Source File

Re: [HACKERS] pageinspect: Hash index support

Hi, On 01/11/2017 03:16 PM, Ashutosh Sharma wrote: I have rephrased it to make it more clear. Rebased, and removed the compile warn in hashfuncs.c Best regards, Jesper >From 8a07230b89b97280f0f1d645145da1fd140969c6 Mon Sep 17 00:00:00 2001 From: jesperpedersen

Re: [HACKERS] pageinspect: Hash index support

Hi, > >> + /* >> + * We copy the page into local storage to avoid holding pin on >> the >> + * buffer longer than we must, and possibly failing to release >> it at >> + * all if the calling query doesn't fetch all rows. >> + */ >>

Re: [HACKERS] pageinspect: Hash index support

Hi, > +values[j++] = UInt16GetDatum(uargs->offset); > +values[j++] = CStringGetTextDatum(psprintf("(%u,%u)", > + > BlockIdGetBlockNumber(&(itup->t_tid.ip_blkid)), > +itup->t_tid.ip_posid)); > + > +ptr = (char *) itup +

Re: [HACKERS] pageinspect: Hash index support

Jesper Pedersen wrote: > + /* > + * We copy the page into local storage to avoid holding pin on > the > + * buffer longer than we must, and possibly failing to release > it at > + * all if the calling query doesn't fetch all rows. > +

Re: [HACKERS] pageinspect: Hash index support

> +itup = (IndexTuple) PageGetItem(uargs->page, id); > + > +MemSet(nulls, 0, sizeof(nulls)); > + > +j = 0; > +values[j++] = UInt16GetDatum(uargs->offset); > +values[j++] = CStringGetTextDatum(psprintf("(%u,%u)", > + >

Re: [HACKERS] pageinspect: Hash index support

On Tue, Jan 10, 2017 at 12:19 PM, Jesper Pedersen wrote: > Revised patched attached. +itup = (IndexTuple) PageGetItem(uargs->page, id); + +MemSet(nulls, 0, sizeof(nulls)); + +j = 0; +values[j++] = UInt16GetDatum(uargs->offset); +

Re: [HACKERS] pageinspect: Hash index support

On 01/10/2017 10:39 AM, Tom Lane wrote: Robert Haas writes: No, you're missing the point. The patch doesn't need to add pageinspect--1.6.sql at all, nor does it remove pageinspect--1.5.sql. It only needs to add pageinspect--1.5--1.6.sql and change the default version to

Re: [HACKERS] pageinspect: Hash index support

Robert Haas writes: > No, you're missing the point. The patch doesn't need to add > pageinspect--1.6.sql at all, nor does it remove pageinspect--1.5.sql. > It only needs to add pageinspect--1.5--1.6.sql and change the default > version to 1.6. No other changes are

Re: [HACKERS] pageinspect: Hash index support

On Fri, Jan 6, 2017 at 1:14 AM, Ashutosh Sharma wrote: >> The previous patch was using pageinspect--1.5.sql as a base, and then uses >> pageinspect--1.5-1.6.sql to upgrade to version 1.6. >> >> Removing pageinspect--1.5.sql, and adding pageinspect--1.6.sql with the >>

Re: [HACKERS] pageinspect: Hash index support

> The previous patch was using pageinspect--1.5.sql as a base, and then uses > pageinspect--1.5-1.6.sql to upgrade to version 1.6. > > Removing pageinspect--1.5.sql, and adding pageinspect--1.6.sql with the > current interface will use pageinspect--1.6.sql directly where as existing >

Re: [HACKERS] pageinspect: Hash index support

Hi Ashutosh, On 01/05/2017 07:13 AM, Ashutosh Sharma wrote: * Readded pageinspect--1.6.sql In order to have the latest pageinspect interface in 1 file, as we need something to install from. I think there should be no problem even if we simply add pageinspect--1.5--1.6.sql file instead of

Re: [HACKERS] pageinspect: Hash index support

Hi Jesper, > * Rename convert_ovflblkno_to_bitno to _hash_ovflblkno_to_bitno > > Such that there is only 1 method, which is exposed Okay, Thanks. It makes sense. > > * Readded pageinspect--1.6.sql > > In order to have the latest pageinspect interface in 1 file, as we need > something to install

Re: [HACKERS] pageinspect: Hash index support

Hi Ashutosh, On 12/20/2016 05:55 AM, Ashutosh Sharma wrote: 1) It introduces two new functions hash_page_type() and hash_bitmap_info(). hash_page_type basically displays the type of hash page whereas hash_bitmap_info() shows the status of a bit for a particular overflow page in bitmap page of

Re: [HACKERS] pageinspect: Hash index support

Hi Jesper, > I was planning to submit a new version next week for CF/January, so I'll > review your changes with the previous feedback in mind. > > Thanks for working on this ! As i was not seeing any updates from you for last 1 month, I thought of working on it. I have created a commit-fest

Re: [HACKERS] pageinspect: Hash index support

Hi, On 12/20/2016 05:55 AM, Ashutosh Sharma wrote: Attached is the revised patch. Please have a look and let me know your feedback. I have also changed the status for this patch in the upcoming commitfest to "needs review". Thanks. I was planning to submit a new version next week for

Re: [HACKERS] pageinspect: Hash index support

Hi All, I have spent some time in reviewing the latest v8 patch from Jesper and could find some issues which i would like to list down, 1) Firstly, the DATA section in the Makefile is referring to "pageinspect--1.6.sql" file and currently this file is missing. +DATA = pageinspect--1.6.sql

Re: [HACKERS] pageinspect: Hash index support

Peter Eisentraut writes: > On 11/2/16 1:54 PM, Tom Lane wrote: >> I think the right thing is likely to be to copy the presented bytea >> into a palloc'd (and therefore properly aligned) buffer. And not >> just in this one function. > Does the attached look

Re: [HACKERS] pageinspect: Hash index support

On 11/2/16 1:54 PM, Tom Lane wrote: > I think the right thing is likely to be to copy the presented bytea > into a palloc'd (and therefore properly aligned) buffer. And not > just in this one function. Does the attached look reasonable? -- Peter Eisentraut

Re: [HACKERS] pageinspect: Hash index support

Peter Eisentraut writes: > So, we're down to crashes in gin_metapage_info() on ia64 and sparc64. > My guess is that the raw page data that is passed into the function > needs to be 8-byte aligned before being accessed as GinMetaPageData. That's what it looks

Re: [HACKERS] pageinspect: Hash index support

On 11/1/16 3:28 PM, Peter Eisentraut wrote: > I have also committed the tests > that I proposed and will work through the failures. So, we're down to crashes in gin_metapage_info() on ia64 and sparc64. My guess is that the raw page data that is passed into the function needs to be 8-byte aligned

Re: [HACKERS] pageinspect: Hash index support

On 11/01/2016 03:28 PM, Peter Eisentraut wrote: Ok, thanks for your feedback. Maybe "Returned with Feedback" is more appropriate, as there is still development left. I have applied the documentation change that introduced subsections, which seems quite useful independently. I have also

Re: [HACKERS] pageinspect: Hash index support

On 10/3/16 8:52 AM, Jesper Pedersen wrote: > On 09/29/2016 04:02 PM, Peter Eisentraut wrote: >> On 9/29/16 4:00 PM, Peter Eisentraut wrote: >>> Since the commit fest is drawing to a close, I'll set this patch as >>> returned with feedback. >> >> Actually, the CF app informs me that moving to the

Re: [HACKERS] pageinspect: Hash index support

On Mon, Oct 3, 2016 at 9:52 PM, Jesper Pedersen wrote: > Maybe "Returned with Feedback" is more appropriate, as there is still > development left. I have switched it to "waiting on author". -- Michael -- Sent via pgsql-hackers mailing list

Re: [HACKERS] pageinspect: Hash index support

On 09/29/2016 04:02 PM, Peter Eisentraut wrote: On 9/29/16 4:00 PM, Peter Eisentraut wrote: Since the commit fest is drawing to a close, I'll set this patch as returned with feedback. Actually, the CF app informs me that moving to the next CF is more appropriate, so I have done that. Ok,

Re: [HACKERS] pageinspect: Hash index support

On 9/29/16 4:00 PM, Peter Eisentraut wrote: > Since the commit fest is drawing to a close, I'll set this patch as > returned with feedback. Actually, the CF app informs me that moving to the next CF is more appropriate, so I have done that. -- Peter Eisentraut

Re: [HACKERS] pageinspect: Hash index support

I think we should look into handling the different page types better. The hash_page_stats function was copied from btree, which only has one type. It's not clear whether all the values apply to each page type. At least they should be null if they don't apply. BRIN has a separate function for

Re: [HACKERS] pageinspect: Hash index support

I wrote some tests for pageinspect, attached here (hash stuff in a separate patch). I think all the output numbers ought to be deterministic (I excluded everything that might contain xids). Please test. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7

Re: [HACKERS] pageinspect: Hash index support

On 09/29/2016 11:58 AM, Peter Eisentraut wrote: On 9/27/16 10:10 AM, Jesper Pedersen wrote: contrib/pageinspect/pageinspect--1.5--1.6.sql | 59 contrib/pageinspect/pageinspect--1.5.sql | 279 -- contrib/pageinspect/pageinspect--1.6.sql | 346

Re: [HACKERS] pageinspect: Hash index support

On 9/27/16 10:10 AM, Jesper Pedersen wrote: > contrib/pageinspect/pageinspect--1.5--1.6.sql | 59 > contrib/pageinspect/pageinspect--1.5.sql | 279 -- > contrib/pageinspect/pageinspect--1.6.sql | 346 ++ I think there is still a misunderstanding

Re: [HACKERS] pageinspect: Hash index support

On 09/26/2016 10:45 PM, Peter Eisentraut wrote: On 9/26/16 1:39 PM, Jesper Pedersen wrote: Left as is, since BuildTupleFromCStrings() vs. xyzGetDatum() are equally readable in this case. But, I can change the patch if needed. The point is that to use BuildTupleFromCStrings() you need to

Re: [HACKERS] pageinspect: Hash index support

Peter Eisentraut wrote: > On 9/26/16 1:39 PM, Jesper Pedersen wrote: > >> - hash_metap result fields spares and mapp should be arrays of integer. > > > > B-tree and BRIN uses a 'text' field as output, so left as is. > > These fields are specific to hash, so the precedent doesn't necessarily >

Re: [HACKERS] pageinspect: Hash index support

On 9/26/16 1:39 PM, Jesper Pedersen wrote: > Left as is, since BuildTupleFromCStrings() vs. xyzGetDatum() are equally > readable in this case. But, I can change the patch if needed. The point is that to use BuildTupleFromCStrings() you need to convert numbers to strings, and then they are

Re: [HACKERS] pageinspect: Hash index support

Jeff Janes wrote: > On Tue, Sep 20, 2016 at 12:19 AM, Michael Paquier > wrote: > > > > > Note: the patch checks if a superuser is calling the new functions, > > which is a good thing. > > > > If we only have the bytea functions and the user needs to supply the raw >

Re: [HACKERS] pageinspect: Hash index support

On Tue, Sep 20, 2016 at 12:19 AM, Michael Paquier wrote: > > Note: the patch checks if a superuser is calling the new functions, > which is a good thing. > If we only have the bytea functions and the user needs to supply the raw pages themselves, rather than having

Re: [HACKERS] pageinspect: Hash index support

On Mon, Sep 26, 2016 at 10:39 AM, Jesper Pedersen < jesper.peder...@redhat.com> wrote: > Hi, > > On 09/23/2016 12:10 AM, Peter Eisentraut wrote: > >> >> > - As of very recently, we don't need to move pageinspect--1.5.sql to >> pageinspect--1.6.sql anymore. Just add pageinspect--1.5--1.6.sql. >>

Re: [HACKERS] pageinspect: Hash index support

On 09/23/2016 01:56 AM, Amit Kapila wrote: While looking at patch, I noticed below code which seems somewhat problematic: + stat->max_avail = BLCKSZ - (BLCKSZ - phdr->pd_special + SizeOfPageHeaderData); + + /* page type (flags) */ + if (opaque->hasho_flag & LH_META_PAGE) + stat->type = 'm'; +

Re: [HACKERS] pageinspect: Hash index support

Hi, On 09/23/2016 12:10 AM, Peter Eisentraut wrote: On 9/21/16 9:30 AM, Jesper Pedersen wrote: Attached is v5, which add basic page verification. There are still some things that I found that appear to follow the old style (btree) conventions instead the new style (brin, gin) conventions. -

Re: [HACKERS] pageinspect: Hash index support

On Fri, Sep 23, 2016 at 6:11 PM, Peter Eisentraut wrote: > On 9/23/16 1:56 AM, Amit Kapila wrote: >> which comment are you referring here? hashm_mapp contains block >> numbers of bitmap pages. > > The comment I'm referring to says > > The blknos of these

Re: [HACKERS] pageinspect: Hash index support

On 9/23/16 1:56 AM, Amit Kapila wrote: > On Fri, Sep 23, 2016 at 9:40 AM, Peter Eisentraut >> - hash_metap result fields spares and mapp should be arrays of integer. >> > > how would you like to see both those arrays in tuple, right now, I > think Jesper's code is showing it as string. I'm not

Re: [HACKERS] pageinspect: Hash index support

On Fri, Sep 23, 2016 at 9:40 AM, Peter Eisentraut wrote: > On 9/21/16 9:30 AM, Jesper Pedersen wrote: >> Attached is v5, which add basic page verification. > > There are still some things that I found that appear to follow the old > style (btree) conventions

Re: [HACKERS] pageinspect: Hash index support

On 9/21/16 9:30 AM, Jesper Pedersen wrote: > Attached is v5, which add basic page verification. There are still some things that I found that appear to follow the old style (btree) conventions instead the new style (brin, gin) conventions. - Rename hash_metap to hash_metapage_info. - Don't use

Re: [HACKERS] pageinspect: Hash index support

On Tue, Sep 20, 2016 at 11:14 PM, Michael Paquier wrote: > > + > + The type information will be 'm' for a metadata > page, > + 'v' for an overflow page, > 'b' for a bucket page, > + 'i' for a bitmap page, and > 'u' for an unused page. > + >

Re: [HACKERS] pageinspect: Hash index support

On 09/21/2016 08:43 AM, Michael Paquier wrote: page_stats / page_items should not be used on the metadata page. As these functions are marked as superuser only it is expected that people provides the correct input, especially since the raw page structure is used as the input. btree functions

Re: [HACKERS] pageinspect: Hash index support

Hi, > git apply w/ v4 works here, so you will have to investigate what happens on > your side. > Thanks, It works with v4 patch. > As these functions are marked as superuser only it is expected that people > provides the correct input, especially since the raw page structure is used > as the

Re: [HACKERS] pageinspect: Hash index support

On Wed, Sep 21, 2016 at 9:21 PM, Jesper Pedersen wrote: > On 09/21/2016 07:24 AM, Ashutosh Sharma wrote: > git apply w/ v4 works here, so you will have to investigate what happens on > your side. Works here as well. >> I continued reviewing your v4 patch and here are

Re: [HACKERS] pageinspect: Hash index support

On 09/21/2016 02:14 AM, Michael Paquier wrote: Adjusted in v4. Thanks for the new version. Code/doc will need an update once the CHI patch goes in. If you are referring to the WAL support, I guess that this patch or the other patch could just rebase and adjust as needed. It is the main

Re: [HACKERS] pageinspect: Hash index support

Hi, On 09/21/2016 07:24 AM, Ashutosh Sharma wrote: The development branch is @ https://github.com/jesperpedersen/postgres/tree/pageinspect_hash I am working on centOS 7. I am still facing the issue when applying your patch using "git apply" command but if i use 'patch' command it works fine

Re: [HACKERS] pageinspect: Hash index support

Hi, > Which platform are you on ? > > The development branch is @ > https://github.com/jesperpedersen/postgres/tree/pageinspect_hash I am working on centOS 7. I am still facing the issue when applying your patch using "git apply" command but if i use 'patch' command it works fine however i am

Re: [HACKERS] pageinspect: Hash index support

On Wed, Sep 21, 2016 at 3:25 AM, Jesper Pedersen wrote: > On 09/20/2016 12:45 PM, Jeff Janes wrote: >> Is the 2nd "1" in this call needed? >> >> SELECT * FROM hash_page_stats(get_raw_page('mytab_index', 1), 1) >> >> As far as I can tell, that argument is only used to

Re: [HACKERS] pageinspect: Hash index support

Hi, On 09/20/2016 01:46 PM, Ashutosh Sharma wrote: I am getting a fatal error along with some warnings when trying to apply the v3 patch using "git apply" command. [ashu@localhost postgresql]$ git apply 0001-pageinspect-Hash-index-support_v3.patch

Re: [HACKERS] pageinspect: Hash index support

On 09/20/2016 12:45 PM, Jeff Janes wrote: Is the 2nd "1" in this call needed? SELECT * FROM hash_page_stats(get_raw_page('mytab_index', 1), 1) As far as I can tell, that argument is only used to stuff into the output field "blkno", it is not used to instruct the interpretation of the raw page

Re: [HACKERS] pageinspect: Hash index support

Hi, I am getting a fatal error along with some warnings when trying to apply the v3 patch using "git apply" command. [ashu@localhost postgresql]$ git apply 0001-pageinspect-Hash-index-support_v3.patch 0001-pageinspect-Hash-index-support_v3.patch:29: trailing whitespace. brinfuncs.o

Re: [HACKERS] pageinspect: Hash index support

On Tue, Sep 20, 2016 at 5:40 AM, Jesper Pedersen wrote: > On 09/20/2016 03:19 AM, Michael Paquier wrote: > >> You did not get right the comments from Alvaro upthread. The following >> functions are added with this patch: >> function hash_metap(text) >> function

Re: [HACKERS] pageinspect: Hash index support

On 09/20/2016 03:19 AM, Michael Paquier wrote: You did not get right the comments from Alvaro upthread. The following functions are added with this patch: function hash_metap(text) function hash_metap_bytea(bytea) function hash_page_items(text,integer) function hash_page_items_bytea(bytea)

Re: [HACKERS] pageinspect: Hash index support

On Tue, Sep 20, 2016 at 1:11 AM, Jesper Pedersen wrote: > This version adds the overloaded get_raw_page based methods. However, I'm > not verifying the page other than page_id, as hasho_flag can contain > multiple flags in Amit's patches. And, I don't see having

Re: [HACKERS] pageinspect: Hash index support

On 09/14/2016 04:21 PM, Jeff Janes wrote: I suggest that pageinspect functions are more convenient to use via the get_raw_page interface, that is, instead of reading the buffer themselves, the buffer is handed over from elsewhere and they receive it as bytea. This enables other use cases such

Re: [HACKERS] pageinspect: Hash index support

On Tue, Aug 30, 2016 at 10:06 AM, Alvaro Herrera wrote: > Jesper Pedersen wrote: > > Hi, > > > > Attached is a patch that adds support for hash indexes in pageinspect. > > > > The functionality (hash_metap, hash_page_stats and hash_page_items) > follows > > the B-tree

Re: [HACKERS] pageinspect: Hash index support

On Wed, Aug 31, 2016 at 2:06 AM, Alvaro Herrera wrote: > Jesper Pedersen wrote: >> Attached is a patch that adds support for hash indexes in pageinspect. >> >> The functionality (hash_metap, hash_page_stats and hash_page_items) follows >> the B-tree functions. > > I

  1   2   >