Small fix for inv_getsize

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
----------------------------------

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:

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

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:

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

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