GetNewObjectId question
While browsing through varsup.c, I noticed this comment on GetNewObjectId:
* Hence, this routine should generally not be used directly. The only direct
* callers should be GetNewOidWithIndex() and GetNewRelFileNumber() in
* catalog/catalog.c.
But AddRoleMems in user.c appears to also call the function directly:
/* get an OID for the new row and insert it */
objectId = GetNewObjectId();
new_record[Anum_pg_auth_members_oid - 1] = objectId;
tuple = heap_form_tuple(pg_authmem_dsc,
new_record, new_record_nulls);
CatalogTupleInsert(pg_authmem_rel, tuple);
I'm not sure if that call is right, but this seems inconsistent.
Should that caller be using GetNewOidWithIndex instead? Or should the
comment be updated?
Thanks,
Maciek
Maciek Sakrejda <m.sakrejda@gmail.com> writes:
While browsing through varsup.c, I noticed this comment on GetNewObjectId:
* Hence, this routine should generally not be used directly. The only direct
* callers should be GetNewOidWithIndex() and GetNewRelFileNumber() in
* catalog/catalog.c.
But AddRoleMems in user.c appears to also call the function directly:
/* get an OID for the new row and insert it */
objectId = GetNewObjectId();
Yeah, that looks like somebody didn't read the memo.
Want to submit a patch?
regards, tom lane
On Sat, Dec 10, 2022 at 07:11:13PM -0500, Tom Lane wrote:
Yeah, that looks like somebody didn't read the memo.
Want to submit a patch?
The comment has been added in e3ce2de but the call originates from
6566133, so that's a HEAD-only issue.
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
On Sat, Dec 10, 2022 at 07:11:13PM -0500, Tom Lane wrote:
Yeah, that looks like somebody didn't read the memo.
Want to submit a patch?
The comment has been added in e3ce2de but the call originates from
6566133, so that's a HEAD-only issue.
Ah, good that the bug hasn't made it to a released version yet.
But that comment is *way* older than e3ce2de; it's been touched
a couple of times, but it dates to 721e53785 AFAICS.
regards, tom lane
Sure. My C is pretty limited, but I think it's just the attached? I
patterned the usage on the way this is done in CreateRole. It passes
check-world here.
Attachments:
0001-Replace-GetNewObjectId-call-with-GetNewOidWithIndex.patchtext/x-patch; charset=US-ASCII; name=0001-Replace-GetNewObjectId-call-with-GetNewOidWithIndex.patchDownload+2-2
On Sat, Dec 10, 2022 at 11:03:38PM -0800, Maciek Sakrejda wrote:
Sure. My C is pretty limited, but I think it's just the attached? I
patterned the usage on the way this is done in CreateRole. It passes
check-world here.
Looks OK seen from here. Thanks for the patch! I don't have much
freshness and time today, but I can get back to this thread tomorrow.
--
Michael