Thread: Documenting removal of nonnullvalue() and friends
Hi, It seems that this commit: commit 2cf8afe5d11377faaf5721b2c2f0089436abf704 Author: Peter Eisentraut <peter_e@gmx.net> Date: Sun Oct 5 17:33:17 2008 +0000 Remove obsolete internal functions istrue, isfalse, isnottrue, isnotfalse, nullvalue, nonvalue. A long time ago, these were used to implement the SQL constructs IS TRUE, etc. removed several undocumented functions which probably weren't intended for wide use. The commit message claims these functions are "internal", though they were accessible as SQL-language functions. At least one person had been using them on 8.3, and was surprised at seeing them gone with no explanation: http://archives.postgresql.org/pgsql-novice/2010-10/msg00084.php Perhaps a mention of this change could be made in the 8.4 release notes? Josh
On Tue, Oct 12, 2010 at 1:03 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote: > Perhaps a mention of this change could be made in the 8.4 release notes? And here's a patch for the 8.4 release notes. Josh
Attachment
On Tue, Oct 12, 2010 at 9:05 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote: > On Tue, Oct 12, 2010 at 1:03 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote: >> Perhaps a mention of this change could be made in the 8.4 release notes? > > And here's a patch for the 8.4 release notes. It seems odd to change our release notes after the fact. I don't think we usually do that. I'm also not really sure that this is important enough to be worth documenting. It looks fairly self-explanatory. *ducks* -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Wed, Oct 13, 2010 at 9:56 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Oct 12, 2010 at 9:05 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote: >> On Tue, Oct 12, 2010 at 1:03 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote: >>> Perhaps a mention of this change could be made in the 8.4 release notes? >> >> And here's a patch for the 8.4 release notes. > > It seems odd to change our release notes after the fact. I don't > think we usually do that. Well, I do count 10 or so updates to release-8.4.sgml since its release date (2009-07-01). Granted, most of these are minor cleanups or to document further point releases. > I'm also not really sure that this is important enough to be worth > documenting. It looks fairly self-explanatory. Oh well. Would anyone favor instead back-patching the documentation for the 8.3, 8.2, and 8.1 branches to include mentions of these previously-undocumented functions, instead? In <http://archives.postgresql.org/pgsql-docs/2004-08/msg00015.php>, Tom opined that they should be left undocumented, but I really don't agree with that. I almost concluded the original -novice poster was simply missing a user-defined function from her old database, since googling for "postgresql nonnullvalue" brought up nothing useful, and there was no mention of it in the (current) source code. It seems pretty harsh to me to intentionally not document these functions, even if they are obscure and not terribly useful. Josh
On Wed, Oct 13, 2010 at 11:25 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote: > On Wed, Oct 13, 2010 at 9:56 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> On Tue, Oct 12, 2010 at 9:05 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote: >>> On Tue, Oct 12, 2010 at 1:03 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote: >>>> Perhaps a mention of this change could be made in the 8.4 release notes? >>> >>> And here's a patch for the 8.4 release notes. >> >> It seems odd to change our release notes after the fact. I don't >> think we usually do that. > > Well, I do count 10 or so updates to release-8.4.sgml since its > release date (2009-07-01). Granted, most of these are minor cleanups > or to document further point releases. As far as I'm aware they're all for further point releases or markup adjustments. I'm not aware of a case where we've changed the *content* of the release notes after the fact. I might be wrong, of course. >> I'm also not really sure that this is important enough to be worth >> documenting. It looks fairly self-explanatory. > > Oh well. > > Would anyone favor instead back-patching the documentation for the > 8.3, 8.2, and 8.1 branches to include mentions of these > previously-undocumented functions, instead? In > <http://archives.postgresql.org/pgsql-docs/2004-08/msg00015.php>, Tom > opined that they should be left undocumented, but I really don't agree > with that. > > I almost concluded the original -novice poster was simply missing a > user-defined function from her old database, since googling for > "postgresql nonnullvalue" brought up nothing useful, and there was no > mention of it in the (current) source code. It seems pretty harsh to > me to intentionally not document these functions, even if they are > obscure and not terribly useful. Well, I certainly would have supported documenting them when they existed, and release-noting them when they were removed, but I'm not too sure how much value there is in doing it years after the fact. A future Google search will probably find this thread, or that one. Still, I don't think it would be unreasonable to add a note to the prior-release documentation saying - hey, these functions exist. They are for internal use only, and do not exist in later versions. Don't use 'em. Now, how many people will actually see that note? Probably not many. But it wouldn't bother me to have it there. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On tor, 2010-10-14 at 08:50 -0400, Robert Haas wrote: > Still, I don't think it would be unreasonable to add a note to the > prior-release documentation saying - hey, these functions exist. They > are for internal use only, and do not exist in later versions. Don't > use 'em. Now, how many people will actually see that note? Probably > not many. But it wouldn't bother me to have it there. Just like there is a philosophical argument against changing the release notes after the release, there is an argument against changing a past release to "retroactively predict" how future releases behave. We have made exceptions to both points on occasion, but usually only for critical upgrade-related issues.
On Thu, Oct 14, 2010 at 10:00 AM, Peter Eisentraut <peter_e@gmx.net> wrote: > The sets of intentionally documented and undocumented functions is part > of the API specification of a release, and we're not changing that after > the release, especially not when a future release ends up reverting the > change. Fair enough. I still think that we shouldn't be intentionally leaving features undocumented, as it just leads to confusion down the road (e.g. trying to figure out what some legacy code is doing, without documentation). I'm curious now, though: are there any other functions which we're leaving intentionally undocumented? The only similar one I've run across are the deprecated "-a" and "-A" arguments to the createuser command. (There's another argument for documenting deprecated features/arguments: it's easier to know what needs to be ripped out in future releases). Josh
Peter Eisentraut <peter_e@gmx.net> writes: > On tor, 2010-10-14 at 08:50 -0400, Robert Haas wrote: >> Still, I don't think it would be unreasonable to add a note to the >> prior-release documentation saying - hey, these functions exist. They >> are for internal use only, and do not exist in later versions. Don't >> use 'em. Now, how many people will actually see that note? Probably >> not many. But it wouldn't bother me to have it there. > Just like there is a philosophical argument against changing the release > notes after the release, there is an argument against changing a past > release to "retroactively predict" how future releases behave. We have > made exceptions to both points on occasion, but usually only for > critical upgrade-related issues. I would put this whole issue in the category of "if you use undocumented features, you should not be surprised when they undocumentedly break". There are many many places where it's possible to see undocumented implementation details in Postgres. If we put in release-note warnings every time we changed one of those details, the release notes would be twice as long and half as useful as they are now. regards, tom lane
Josh Kupershmidt <schmiddy@gmail.com> writes: > I'm curious now, though: are there any other functions which we're > leaving intentionally undocumented? Practically all the functions underlying operators are undocumented, eg int4pl(int, int). (If you consider IS NOT NULL to be an operator, then nonnullvalue is exactly one of those.) It's possible for people to use those directly but it's discouraged. I do *not* want to add them to the docs, because (a) I do *not* want people to use them directly and (b) there are a hell of a lot of them, so the docs would get much much longer with no actual added value. Perhaps what we should really add to the docs is a statement that use of undocumented functions is deprecated and we will not be responsible when your code breaks because one of those is changed or removed. regards, tom lane
On ons, 2010-10-13 at 23:25 -0400, Josh Kupershmidt wrote: > Would anyone favor instead back-patching the documentation for the > 8.3, 8.2, and 8.1 branches to include mentions of these > previously-undocumented functions, instead? In > <http://archives.postgresql.org/pgsql-docs/2004-08/msg00015.php>, Tom > opined that they should be left undocumented, but I really don't agree > with that. The sets of intentionally documented and undocumented functions is part of the API specification of a release, and we're not changing that after the release, especially not when a future release ends up reverting the change.
On Thu, Oct 14, 2010 at 10:00 AM, Peter Eisentraut <peter_e@gmx.net> wrote: > On ons, 2010-10-13 at 23:25 -0400, Josh Kupershmidt wrote: >> Would anyone favor instead back-patching the documentation for the >> 8.3, 8.2, and 8.1 branches to include mentions of these >> previously-undocumented functions, instead? In >> <http://archives.postgresql.org/pgsql-docs/2004-08/msg00015.php>, Tom >> opined that they should be left undocumented, but I really don't agree >> with that. > > The sets of intentionally documented and undocumented functions is part > of the API specification of a release, and we're not changing that after > the release, especially not when a future release ends up reverting the > change. I find the idea of things being intentionally undocumented quite difficult. How is someone coming along supposed to know which things are intentionally undocumented and which things are unintentionally undocumented? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes: > I find the idea of things being intentionally undocumented quite > difficult. How is someone coming along supposed to know which things > are intentionally undocumented and which things are unintentionally > undocumented? They should assume the lack of documentation is intentional. Six or seven years ago your argument might have held some water, but we've been quite rigorous about requiring externally-visible features to be documented from the get-go. regards, tom lane
On Thu, Oct 14, 2010 at 6:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> I find the idea of things being intentionally undocumented quite >> difficult. How is someone coming along supposed to know which things >> are intentionally undocumented and which things are unintentionally >> undocumented? > > They should assume the lack of documentation is intentional. Six or > seven years ago your argument might have held some water, but we've > been quite rigorous about requiring externally-visible features to be > documented from the get-go. That just doesn't match my experience. We goof. People find it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes: > On Thu, Oct 14, 2010 at 6:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> They should assume the lack of documentation is intentional. �Six or >> seven years ago your argument might have held some water, but we've >> been quite rigorous about requiring externally-visible features to be >> documented from the get-go. > That just doesn't match my experience. We goof. People find it. Sure, there are bugs of that sort, just like we have bugs of many other sorts. When people find them, we fix them. But the general principle is that functions and operators that are meant to be used directly from SQL are shown in the SGML documentation; and those that aren't, aren't. There are probably upwards of a thousand entries in pg_proc that are not, and never were, meant to be directly invoked from SQL. (In particular, with 706 entries in pg_operator, there are presumably about 700 functions underlying operators; and then you've got functions underlying aggregates, index AM support functions, selectivity estimation functions, text search support functions, foreign data wrapper functions, yadda yadda.) It is not sane to propose documenting those individually, not only from the standpoint of docs bloat but because documenting them would encourage people to call them, which is exactly not the result we want. regards, tom lane
On Thu, Oct 14, 2010 at 7:15 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Thu, Oct 14, 2010 at 6:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> They should assume the lack of documentation is intentional. Six or >>> seven years ago your argument might have held some water, but we've >>> been quite rigorous about requiring externally-visible features to be >>> documented from the get-go. > >> That just doesn't match my experience. We goof. People find it. > > Sure, there are bugs of that sort, just like we have bugs of many other > sorts. When people find them, we fix them. But the general principle > is that functions and operators that are meant to be used directly from > SQL are shown in the SGML documentation; and those that aren't, aren't. > > There are probably upwards of a thousand entries in pg_proc that are > not, and never were, meant to be directly invoked from SQL. (In > particular, with 706 entries in pg_operator, there are presumably about > 700 functions underlying operators; and then you've got functions > underlying aggregates, index AM support functions, selectivity > estimation functions, text search support functions, foreign data > wrapper functions, yadda yadda.) It is not sane to propose documenting > those individually, not only from the standpoint of docs bloat but > because documenting them would encourage people to call them, which is > exactly not the result we want. Part of the problem, I think, is that people don't necessarily find this stuff via the documentation. They fire up psql or pgAdmin and start typing backslash commands. They see something good, so they use it. How are they to know it's undocumented? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On tor, 2010-10-14 at 19:17 -0400, Robert Haas wrote: > Part of the problem, I think, is that people don't necessarily find > this stuff via the documentation. They fire up psql or pgAdmin and > start typing backslash commands. They see something good, so they use > it. How are they to know it's undocumented? This could possibly be addressed if we more diligently maintained the system catalogs comments, and then possibly default the comments of undocumented objects to "internal object, don't use".
Peter Eisentraut <peter_e@gmx.net> writes: > On tor, 2010-10-14 at 19:17 -0400, Robert Haas wrote: >> Part of the problem, I think, is that people don't necessarily find >> this stuff via the documentation. They fire up psql or pgAdmin and >> start typing backslash commands. They see something good, so they use >> it. How are they to know it's undocumented? > This could possibly be addressed if we more diligently maintained the > system catalogs comments, and then possibly default the comments of > undocumented objects to "internal object, don't use". I thought about this a bit more last night. It's certainly true that a lot of "internal" functions have comments that don't suggest that they're not meant to be used directly. What I think would be a good plan for functions that underlie operators is that we move any useful comments from pg_proc to pg_operator, and then install a comment in pg_proc that says "implementation of operator +" (or whatever the operator name is). This will not only let people know that they should use an operator instead, but which one to use, when they find the function via \df. I believe that there are a few cases where we document both the operator and the equivalent function, so in those cases both should have the regular comment. The same sort of approach could be used for functions that are meant as aggregate support, if they don't have any real stand-alone use. I think most of the other categories of support functions are already pretty obviously internal, if there even are any that don't have "internal" arguments. If that sounds like a reasonable plan, I'm willing to have a go at it after the commitfest closes. regards, tom lane
On Fri, Oct 15, 2010 at 11:40 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> On tor, 2010-10-14 at 19:17 -0400, Robert Haas wrote: >>> Part of the problem, I think, is that people don't necessarily find >>> this stuff via the documentation. They fire up psql or pgAdmin and >>> start typing backslash commands. They see something good, so they use >>> it. How are they to know it's undocumented? > >> This could possibly be addressed if we more diligently maintained the >> system catalogs comments, and then possibly default the comments of >> undocumented objects to "internal object, don't use". > > I thought about this a bit more last night. It's certainly true that > a lot of "internal" functions have comments that don't suggest that > they're not meant to be used directly. What I think would be a good > plan for functions that underlie operators is that we move any useful > comments from pg_proc to pg_operator, and then install a comment in > pg_proc that says "implementation of operator +" (or whatever the > operator name is). This will not only let people know that they should > use an operator instead, but which one to use, when they find the > function via \df. > > I believe that there are a few cases where we document both the operator > and the equivalent function, so in those cases both should have the > regular comment. > > The same sort of approach could be used for functions that are meant as > aggregate support, if they don't have any real stand-alone use. I think > most of the other categories of support functions are already pretty > obviously internal, if there even are any that don't have "internal" > arguments. > > If that sounds like a reasonable plan, I'm willing to have a go at it > after the commitfest closes. It's a reasonable plan, but I'm not sure it's going to do a whole lot of good in practice. I use \df all the time but \df+ not too often. I'm halfway tempted to propose that we add a prosupersekret column that can be set on things we don't intend users to make use of directly, and then hide them even from \dfS and \df <pattern>. But I suspect you'll all just laugh at me. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes: > On Fri, Oct 15, 2010 at 11:40 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I thought about this a bit more last night. �It's certainly true that >> a lot of "internal" functions have comments that don't suggest that >> they're not meant to be used directly. �What I think would be a good >> plan for functions that underlie operators is that we move any useful >> comments from pg_proc to pg_operator, and then install a comment in >> pg_proc that says "implementation of operator +" (or whatever the >> operator name is). > It's a reasonable plan, but I'm not sure it's going to do a whole lot > of good in practice. I use \df all the time but \df+ not too often. I don't think that argument holds a lot of water. If you found an interesting-looking function via \df, wouldn't you make at least some effort to verify what it does before putting it into a production query? I should think you'd at least look into \df+ or the SGML docs. > I'm halfway tempted to propose that we add a prosupersekret column > that can be set on things we don't intend users to make use of > directly, and then hide them even from \dfS and \df <pattern>. But I > suspect you'll all just laugh at me. I've occasionally toyed with the idea of moving all non-public functions into a separate schema, say "pg_internal", which wouldn't be in the standard search path (and thus ordinary \df wouldn't see it). That would be nice from a cleanliness standpoint, but there are downsides too. The big problem is that it moves this endeavor out of the realm of "let's improve the documentation" and into the realm of "let's intentionally break every app that dared to use a non-public function". It seems clear to me that that's a huge overreaction. 99% of the time it's no big deal if somebody uses one of these directly; as portability problems go, this was a tiny and easily solved issue. regards, tom lane
On Sat, Oct 16, 2010 at 10:28 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Fri, Oct 15, 2010 at 11:40 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> I thought about this a bit more last night. It's certainly true that >>> a lot of "internal" functions have comments that don't suggest that >>> they're not meant to be used directly. What I think would be a good >>> plan for functions that underlie operators is that we move any useful >>> comments from pg_proc to pg_operator, and then install a comment in >>> pg_proc that says "implementation of operator +" (or whatever the >>> operator name is). > >> It's a reasonable plan, but I'm not sure it's going to do a whole lot >> of good in practice. I use \df all the time but \df+ not too often. > > I don't think that argument holds a lot of water. If you found an > interesting-looking function via \df, wouldn't you make at least some > effort to verify what it does before putting it into a production query? > I should think you'd at least look into \df+ or the SGML docs. Maybe. But even if I did, I am not known for being the least cautious person among my set of acquaintances. >> I'm halfway tempted to propose that we add a prosupersekret column >> that can be set on things we don't intend users to make use of >> directly, and then hide them even from \dfS and \df <pattern>. But I >> suspect you'll all just laugh at me. > > I've occasionally toyed with the idea of moving all non-public functions > into a separate schema, say "pg_internal", which wouldn't be in the > standard search path (and thus ordinary \df wouldn't see it). That > would be nice from a cleanliness standpoint, but there are downsides > too. The big problem is that it moves this endeavor out of the realm of > "let's improve the documentation" and into the realm of "let's > intentionally break every app that dared to use a non-public function". > It seems clear to me that that's a huge overreaction. 99% of the time > it's no big deal if somebody uses one of these directly; as portability > problems go, this was a tiny and easily solved issue. It probably would have been a good idea to design it that way originally, but I think it's too late to do that now. That would break a lot more stuff than what I proposed, because you'd really be moving it, not just sweeping it under the rug. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On lör, 2010-10-16 at 10:28 -0400, Tom Lane wrote: > I've occasionally toyed with the idea of moving all non-public > functions > into a separate schema, say "pg_internal", which wouldn't be in the > standard search path (and thus ordinary \df wouldn't see it). That > would be nice from a cleanliness standpoint, but there are downsides > too. Another thing we could do at least for new functions is to name them starting with an underscore (or two).
Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > On tor, 2010-10-14 at 19:17 -0400, Robert Haas wrote: > >> Part of the problem, I think, is that people don't necessarily find > >> this stuff via the documentation. They fire up psql or pgAdmin and > >> start typing backslash commands. They see something good, so they use > >> it. How are they to know it's undocumented? > > > This could possibly be addressed if we more diligently maintained the > > system catalogs comments, and then possibly default the comments of > > undocumented objects to "internal object, don't use". > > I thought about this a bit more last night. It's certainly true that > a lot of "internal" functions have comments that don't suggest that > they're not meant to be used directly. What I think would be a good > plan for functions that underlie operators is that we move any useful > comments from pg_proc to pg_operator, and then install a comment in > pg_proc that says "implementation of operator +" (or whatever the > operator name is). This will not only let people know that they should > use an operator instead, but which one to use, when they find the > function via \df. > > I believe that there are a few cases where we document both the operator > and the equivalent function, so in those cases both should have the > regular comment. > > The same sort of approach could be used for functions that are meant as > aggregate support, if they don't have any real stand-alone use. I think > most of the other categories of support functions are already pretty > obviously internal, if there even are any that don't have "internal" > arguments. > > If that sounds like a reasonable plan, I'm willing to have a go at it > after the commitfest closes. Tom, any work on this? A TODO? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian <bruce@momjian.us> writes: > Tom Lane wrote: >> I thought about this a bit more last night. It's certainly true that >> a lot of "internal" functions have comments that don't suggest that >> they're not meant to be used directly. What I think would be a good >> plan for functions that underlie operators is that we move any useful >> comments from pg_proc to pg_operator, and then install a comment in >> pg_proc that says "implementation of operator +" (or whatever the >> operator name is). >> ... >> If that sounds like a reasonable plan, I'm willing to have a go at it >> after the commitfest closes. > Tom, any work on this? A TODO? I haven't done anything about this yet, but it's still on my to-do list. Right now (or at least after I finish the patches I'm working on) would probably be about the best possible time to do it, since hopefully we have the minimum number of unapplied patches that would need to be revised to follow the new convention. regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Tom Lane wrote: > >> I thought about this a bit more last night. It's certainly true that > >> a lot of "internal" functions have comments that don't suggest that > >> they're not meant to be used directly. What I think would be a good > >> plan for functions that underlie operators is that we move any useful > >> comments from pg_proc to pg_operator, and then install a comment in > >> pg_proc that says "implementation of operator +" (or whatever the > >> operator name is). > >> ... > >> If that sounds like a reasonable plan, I'm willing to have a go at it > >> after the commitfest closes. > > > Tom, any work on this? A TODO? > > I haven't done anything about this yet, but it's still on my to-do list. > Right now (or at least after I finish the patches I'm working on) would > probably be about the best possible time to do it, since hopefully we > have the minimum number of unapplied patches that would need to be > revised to follow the new convention. Good point. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
I wrote: > Bruce Momjian <bruce@momjian.us> writes: >> Tom Lane wrote: >>> I thought about this a bit more last night. It's certainly true that >>> a lot of "internal" functions have comments that don't suggest that >>> they're not meant to be used directly. What I think would be a good >>> plan for functions that underlie operators is that we move any useful >>> comments from pg_proc to pg_operator, and then install a comment in >>> pg_proc that says "implementation of operator +" (or whatever the >>> operator name is). >>> ... >>> If that sounds like a reasonable plan, I'm willing to have a go at it >>> after the commitfest closes. >> Tom, any work on this? A TODO? > I haven't done anything about this yet, but it's still on my to-do list. Done now. regards, tom lane