Thread: Small fix for inv_getsize
Hello, Just realized that inv_getsize is a little bit wrong :-)). It just get the first page, not last Here is the patch which will fix the behavior. -- Sincerely Yours, Denis Perchine ---------------------------------- E-Mail: dyp@perchine.com HomePage: http://www.perchine.com/dyp/ FidoNet: 2:5000/120.5 ----------------------------------
Attachment
Denis Perchine <dyp@perchine.com> writes:
> Just realized that inv_getsize is a little bit wrong :-)). It just get the
> first page, not last
> Here is the patch which will fix the behavior.
No it doesn't; it's a loop, and your patch will change nothing. Your
original version tried to stop after fetching one tuple, which was
wrong because of visibility considerations.
Now that I think about it, this code could do a two-key scan backwards
and stop after finding the first (last) valid tuple, but that's more
than a one-line change.
regards, tom lane
> > Just realized that inv_getsize is a little bit wrong :-)). It just get > > the first page, not last > > Here is the patch which will fix the behavior. > > No it doesn't; it's a loop, and your patch will change nothing. Your > original version tried to stop after fetching one tuple, which was > wrong because of visibility considerations. > > Now that I think about it, this code could do a two-key scan backwards > and stop after finding the first (last) valid tuple, but that's more > than a one-line change. Actual logic is to find the maximum of pageno, for specified oid. I do index scan on 2-keys index, specifying only one key as constraint... If I do index scan forward I will get the smallest pageno first... Otherwise I get the highest pageno... And this is what I want... Or I get something wrong? Isn't this how order by on index is done? -- Sincerely Yours, Denis Perchine ---------------------------------- E-Mail: dyp@perchine.com HomePage: http://www.perchine.com/dyp/ FidoNet: 2:5000/120.5 ----------------------------------
Denis Perchine <dyp@perchine.com> writes:
>> Now that I think about it, this code could do a two-key scan backwards
>> and stop after finding the first (last) valid tuple, but that's more
>> than a one-line change.
> Actual logic is to find the maximum of pageno, for specified oid.
> I do index scan on 2-keys index, specifying only one key as constraint...
> If I do index scan forward I will get the smallest pageno first...
> Otherwise I get the highest pageno... And this is what I want...
Hmm ... probably right, but the loop logic doesn't behave that way
right now.
regards, tom lane
> >> Now that I think about it, this code could do a two-key scan backwards > >> and stop after finding the first (last) valid tuple, but that's more > >> than a one-line change. > > > > Actual logic is to find the maximum of pageno, for specified oid. > > I do index scan on 2-keys index, specifying only one key as constraint... > > If I do index scan forward I will get the smallest pageno first... > > Otherwise I get the highest pageno... And this is what I want... > > Hmm ... probably right, but the loop logic doesn't behave that way > right now. Why not... Did not I explained what happend there? obj_desc->index_r is 2 key index... I do index scan backward. Get first valid tuple... What I miss... Can you please give me the correct one... I am totally confused... Sorry for asking this... -- Sincerely Yours, Denis Perchine ---------------------------------- E-Mail: dyp@perchine.com HomePage: http://www.perchine.com/dyp/ FidoNet: 2:5000/120.5 ----------------------------------
Denis Perchine <dyp@perchine.com> writes:
>> Hmm ... probably right, but the loop logic doesn't behave that way
>> right now.
> Why not... Did not I explained what happend there?
You didn't read the way I changed it: right now it scans all the tuples
and takes the maximum endpoint, rather than assuming they will be read
in any particular order.
regards, tom lane
> >> Hmm ... probably right, but the loop logic doesn't behave that way > >> right now. > > > > Why not... Did not I explained what happend there? > > You didn't read the way I changed it: right now it scans all the tuples > and takes the maximum endpoint, rather than assuming they will be read > in any particular order. OK. I see... I just wondering whether it is possible to use index to get the maximum... -- Sincerely Yours, Denis Perchine ---------------------------------- E-Mail: dyp@perchine.com HomePage: http://www.perchine.com/dyp/ FidoNet: 2:5000/120.5 ----------------------------------
Denis Perchine <dyp@perchine.com> writes:
> OK. I see... I just wondering whether it is possible to use index to get the
> maximum...
I think you are right, it's possible to do it that way. The original
code was broken because it didn't check tuple visibility, so I just
copied-and-pasted some other code without thinking very hard. Feel
free to improve it.
regards, tom lane
> > OK. I see... I just wondering whether it is possible to use index to get > > the maximum... > > I think you are right, it's possible to do it that way. The original > code was broken because it didn't check tuple visibility, so I just > copied-and-pasted some other code without thinking very hard. Feel > free to improve it. Sorry, but I did not find any significant difference between my code and code you wrote. Except VARATT_IS_EXTENDED check (is it neccessary, can I store data and be sure that it is not toasted? I do not like this for BLOBs). All other seems the same... Please give me an example of this check... -- Sincerely Yours, Denis Perchine ---------------------------------- E-Mail: dyp@perchine.com HomePage: http://www.perchine.com/dyp/ FidoNet: 2:5000/120.5 ----------------------------------
Denis Perchine <dyp@perchine.com> writes:
> you wrote. Except VARATT_IS_EXTENDED check (is it neccessary, can I store
> data and be sure that it is not toasted? I do not like this for BLOBs).
Yes, it's necessary *and* appropriate. LO data won't be moved off,
because pg_largeobject doesn't have a toast table (unless the user makes
one), but it can be compressed.
> All other seems the same... Please give me an example of this check...
The loop only has to loop till it finds a valid tuple.
regards, tom lane
3 Ноябрь 2000 01:19, Tom Lane написал:
> Denis Perchine <dyp@perchine.com> writes:
> > you wrote. Except VARATT_IS_EXTENDED check (is it neccessary, can I store
> > data and be sure that it is not toasted? I do not like this for BLOBs).
>
> Yes, it's necessary *and* appropriate. LO data won't be moved off,
> because pg_largeobject doesn't have a toast table (unless the user makes
> one), but it can be compressed.
>
> > All other seems the same... Please give me an example of this check...
>
> The loop only has to loop till it finds a valid tuple.
But my code do exactly this...
The only difference between your code and mine (except you search through all
tuples) is in toast handling... Isn't (tuple.t_data == NULL) a validity check?
My code:
while ((indexRes = index_getnext(sd, ForwardScanDirection))) {
tuple.t_self = indexRes->heap_iptr;
heap_fetch(obj_desc->heap_r, SnapshotNow, &tuple, &buffer);
pfree(indexRes);
if (tuple.t_data == NULL)
continue;
found++;
data = (Form_pg_largeobject) GETSTRUCT(&tuple);
lastbyte = data->pageno * IBLKSIZE +
getbytealen(&(data->data));
ReleaseBuffer(buffer);
break;
}
Your code:
while ((indexRes = index_getnext(sd, ForwardScanDirection)))
{
tuple.t_self = indexRes->heap_iptr;
heap_fetch(obj_desc->heap_r, SnapshotNow, &tuple, &buffer);
pfree(indexRes);
if (tuple.t_data == NULL)
continue;
found = true;
data = (Form_pg_largeobject) GETSTRUCT(&tuple);
datafield = &(data->data);
pfreeit = false;
if (VARATT_IS_EXTENDED(datafield))
{
datafield = (bytea *)
heap_tuple_untoast_attr((varattrib *)
datafield);
pfreeit = true;
}
thislastbyte = data->pageno * LOBLKSIZE +
getbytealen(datafield);
if (thislastbyte > lastbyte)
lastbyte = thislastbyte;
if (pfreeit)
pfree(datafield);
ReleaseBuffer(buffer);
}
--
Sincerely Yours,
Denis Perchine
----------------------------------
E-Mail: dyp@perchine.com
HomePage: http://www.perchine.com/dyp/
FidoNet: 2:5000/120.5
----------------------------------
Denis Perchine <dyp@perchine.com> writes:
> But my code do exactly this...
The differences you cite below are considerably greater than the
one-line patch I was responding to.
regards, tom lane