TidScan needs handling of a corner cases

Started by Gurjeet Singhover 17 years ago2 messages
#1Gurjeet Singh
singh.gurjeet@gmail.com

Hi All,

I noticed that the TidScan fails to identify when the requested block is
not in the relation. Consider this (pg_class has 6 blocks):

postgres=# explain analyze select ctid, * from pg_class where ctid in (
'(6,1)' );
ERROR: could not read block 6 of relation 1663/11511/1259: read only 0 of
8192 bytes

Also, it is known that 0 is not a valid row-offset in the block, but the
tid input function accepts this value (it rejects -ve values). For the same
setup:

postgres=# explain analyze select ctid, * from pg_class where ctid in (
'(6,0)' );
QUERY
PLAN
----------------------------------------------------------------------------------------------------
Tid Scan on pg_class (cost=0.00..4.01 rows=1 width=211) (actual
time=0.009..0.009 rows=0 loops=1)
TID Cond: (ctid = '(6,0)'::tid)
Total runtime: 0.130 ms
(3 rows)

Can we safely fix these? First one by ignoring the request if
requested_block >= RelationGetNumberOfBlocks(), and second one by accepting
only non-zero positive values in the tid input function.

Best regards

--
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | indiatimes | yahoo }.com

EnterpriseDB http://www.enterprisedb.com

Mail sent from my BlackLaptop device

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Gurjeet Singh (#1)
Re: TidScan needs handling of a corner cases

"Gurjeet Singh" <singh.gurjeet@gmail.com> writes:

postgres=# explain analyze select ctid, * from pg_class where ctid in (
'(6,1)' );
ERROR: could not read block 6 of relation 1663/11511/1259: read only 0 of
8192 bytes

I was about to say that throwing an error for this is just fine, but on
closer investigation we didn't throw an error before 8.3; the behavioral
change is because mdread() now throws an error for out-of-bounds read.
So for consistency with historical behavior this probably needs to be
fixed.

Also, it is known that 0 is not a valid row-offset in the block, but the
tid input function accepts this value (it rejects -ve values). For the same
setup:

Not sure what you think needs to be fixed in this case? Again,
backwards compatibility seems a reason not to change it; and I sure
don't see why you'd want to throw an error for this case but not the
other one.

regards, tom lane