On Thu, Dec 19, 2024 at 8:29 AM Robert Haas <robertmh...@gmail.com> wrote: > Cool. I don't want to commit this right before my vacation but I'll > look into it in the new year if nobody beats me to it.
Nobody has beaten me to it, but I thought of one potential issue. Suppose that somebody has a foreign table that exists today with one of these properties set to the empty string. Such a foreign table is not usable for queries, because the queries won't make it past scan.l on the remote side, but it's allowed to exist. With this patch, it's not allowed to exist any more. That would be fine if no such objects exist in the wild, but if they do, people will get a pg_dump or pg_upgrade failure when they try to upgrade to a release that includes this change. Does that mean that we shouldn't commit this patch? I'm inclined to think that it's probably still OK, because (1) few users are likely to have such objects, (2) the workaround is easy, just drop the object or fix it to do something useful, just as users already need to do in plenty of other, similar cases and (3) unclear error messages are not great and future users might benefit from this error message being better. However, one could take the opposite position and say that (C1) it is unlikely that anyone will try to do this in the first place and (C2) the error message that is currently generated when you try to do this isn't really all that unclear, and therefore it's not worth creating even a minor dump-and-reload hazard; and that position also seems pretty defensible to me. Other opinions? -- Robert Haas EDB: http://www.enterprisedb.com
