citus Inaccurate comments about LookupStorageId()

The LookupStorageId() claims it returns false if the relation doesn't have a meta page yet. Does it means zero is false here? If so, maybe change it to it returns zero if the relation doesn't have a meta page yet is better. If not, we should update the comment, right? https://github.com/citusdata/citus/blob/c3579eef0626563f62c42f12629af8a4dfcb000f/src/backend/columnar/columnar_metadata.c#L1935-L1951

Jan 03 '24 08:01 japinli

I'm not sure if this can ever return 0 because storage id sequence starts from 10000000000, due to:

CREATE SEQUENCE IF NOT EXISTS storageid_seq MINVALUE 10000000000 NO CYCLE;

Someone needs to have a closer look into call-sites of this function to see if this function can ever return 0, e.g., during citus / postgres upgrades?, when the relation is empty? when the storage is corrupted for some reason?

Jan 03 '24 14:01 onurctirtir

@onurctirtir Thank you for your explanation. I dig in deeper and find the storage id comes from the first block, which is stored after PageHeader, so if this data corrupted, it might be error, however, there is no way to check this, right?

Jan 04 '24 01:01 japinli