CREATE CAST code review
От | Tom Lane |
---|---|
Тема | CREATE CAST code review |
Дата | |
Msg-id | 15458.1027210293@sss.pgh.pa.us обсуждение исходный текст |
Ответы |
Re: CREATE CAST code review
|
Список | pgsql-hackers |
I looked through your CREATE CAST commit a little. Looks pretty good but I had a few suggestions/concerns. * The biggie is that I'm not satisfied with the permissions checking. You have "To be able to create a cast, you must own the underlying function. To be able to create a binary compatible cast, you must own both the source and the target data type." (Same to drop a cast.) It seems to me that this is quite dangerous, as any random luser who can access both datatypes can create a function and then a cast, thereby changing the behavior of *both* types in rather subtle ways, and perhaps insinuating unexpected, untrusted code into queries issued by people who weren't expecting any cast to be applied. (Not to mention causing a denial-of-service for the legitimate definer of that cast, if he hadn't gotten around to making it quite yet.) The problem would be slightly less bad if function definition required USAGE privilege on the arg/result types ... but not much. I think I would prefer this definition: to create/drop a cast, you must own both datatypes, plus the underlying function if any. What's the rationale for having such a weak permissions scheme? * I see that you are worried in pg_dump about which schema to associate a cast with, if it's binary-compatible. I'm confused too; would it work to dump the cast iff either of the source/dest datatypes are to be dumped? (This might be a better rule even in the not-binary-compatible case.) * Various more-or-less minor coding gripes: * pg_cast table not documented in catalogs.sgml. * shoddy implementation of getObjectDescription() for cast. (I have some to-do items in that routine myself, though, and will be happy to fix this while at it.) * since pg_cast depends on having a unique OID, it *must* have an OID index to enforce that uniqueness; otherwise the system can fail after OID wraparound. * since you must define the OID index anyway, you may as well use it in a systable scan in DropCastById, instead of using heapscan. * in CreateCast, there's no need to use the relatively expensive get_system_catalog_relid() lookup; you've got pg_cast open and so you can just do RelationGetRelid on it. regards, tom lane PS: I also want to raise a flag that we still haven't resolved the issues we discussed a few months ago, about exactly *which* implicit casts should be provided. I think that's a different thread though.
В списке pgsql-hackers по дате отправления: