Re: [HACKERS] Deprecation
On Sat, Oct 17, 2009 at 03:01:27PM -0400, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Sounds like a good reason to remove add_missing_from in 8.5. Seems like the general consensus is that it's okay to do that. I will go make it happen unless somebody squawks pretty soon... regards, tom lane +1 -dg -- David Gould da...@sonic.net 510 536 1443510 282 0869 If simplicity worked, the world would be overrun with insects. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecation
Greg Stark gsst...@mit.edu writes: On Fri, Oct 16, 2009 at 12:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: However, if the columnref looks like x.y where x happens to match some table in the database (and not in the query) that doesn't have a column y, the implicit-RTE code will have already modified the querytree before finding out that column y doesn't exist. Hm, so if you do nothing then really the only thing that doesn't work is if you have add_missing_from then plpgsql record variables wouldn't work when you tried to reference their columns? Do nothing isn't the right phrase here --- it would take a great deal of work and ugly, hard-to-maintain code to get it to work even that badly. The code paths in transformColumnRef are fairly messy already :-(. Getting rid of add_missing_from would definitely make it easier to refactor to support hooks for external variable sources. The approach I had been thinking about proposing, before David piped up with his modest proposal, was to have external variables take precedence over implicit RTEs --- ie, we'd call the hook *before* trying the add_missing_from case. But that seems pretty weird, and it'd still be messy to program. What it would mainly accomplish is to avoid the extra lock hazard. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecation
Tom Lane wrote: Greg Stark gsst...@mit.edu writes: On Fri, Oct 16, 2009 at 12:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: However, if the columnref looks like x.y where x happens to match some table in the database (and not in the query) that doesn't have a column y, the implicit-RTE code will have already modified the querytree before finding out that column y doesn't exist. Hm, so if you do nothing then really the only thing that doesn't work is if you have add_missing_from then plpgsql record variables wouldn't work when you tried to reference their columns? Do nothing isn't the right phrase here --- it would take a great deal of work and ugly, hard-to-maintain code to get it to work even that badly. The code paths in transformColumnRef are fairly messy already :-(. Getting rid of add_missing_from would definitely make it easier to refactor to support hooks for external variable sources. The approach I had been thinking about proposing, before David piped up with his modest proposal, was to have external variables take precedence over implicit RTEs --- ie, we'd call the hook *before* trying the add_missing_from case. But that seems pretty weird, and it'd still be messy to program. What it would mainly accomplish is to avoid the extra lock hazard. Sounds like a good reason to remove add_missing_from in 8.5. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecation
Tom Lane t...@sss.pgh.pa.us writes: Do nothing isn't the right phrase here --- it would take a great deal of work and ugly, hard-to-maintain code to get it to work even that badly. The code paths in transformColumnRef are fairly messy already :-(. Getting rid of add_missing_from would definitely make it easier to refactor to support hooks for external variable sources. A little voice from the field, +1 for deprecating add_missing_from. Regards, -- dim -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecation
2009/10/17 Dimitri Fontaine dfonta...@hi-media.com: Tom Lane t...@sss.pgh.pa.us writes: Do nothing isn't the right phrase here --- it would take a great deal of work and ugly, hard-to-maintain code to get it to work even that badly. The code paths in transformColumnRef are fairly messy already :-(. Getting rid of add_missing_from would definitely make it easier to refactor to support hooks for external variable sources. A little voice from the field, +1 for deprecating add_missing_from. +1 Pavel Regards, -- dim -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecation
Bruce Momjian br...@momjian.us writes: Sounds like a good reason to remove add_missing_from in 8.5. Seems like the general consensus is that it's okay to do that. I will go make it happen unless somebody squawks pretty soon... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecation
Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Sounds like a good reason to remove add_missing_from in 8.5. Seems like the general consensus is that it's okay to do that. I will go make it happen unless somebody squawks pretty soon... Squawk. I am currently travelling. Please give me until early next week to research and react. thanks andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecation
Andrew Dunstan and...@dunslane.net writes: Squawk. I am currently travelling. Please give me until early next week to research and react. Okay, I'll hold off for a bit. For reference, attached is the patch I was about to apply. This doesn't do any of the refactoring I had in mind, it just removes the implicit-RTE logic. regards, tom lane Index: doc/src/sgml/config.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/config.sgml,v retrieving revision 1.230 diff -c -r1.230 config.sgml *** doc/src/sgml/config.sgml 3 Oct 2009 23:10:47 - 1.230 --- doc/src/sgml/config.sgml 17 Oct 2009 20:37:00 - *** *** 4659,4695 variablelist - varlistentry id=guc-add-missing-from xreflabel=add_missing_from - termvarnameadd_missing_from/varname (typeboolean/type)/term - indextermprimaryFROM/secondarymissing// - indexterm -primaryvarnameadd_missing_from/ configuration parameter/primary - /indexterm - listitem -para - When on, tables that are referenced by a query will be - automatically added to the literalFROM/ clause if not - already present. This behavior does not comply with the SQL - standard and many people dislike it because it can mask mistakes - (such as referencing a table where you should have referenced - its alias). The default is literaloff/. This variable can be - enabled for compatibility with releases of - productnamePostgreSQL/ prior to 8.1, where this behavior was - allowed by default. -/para - -para - Note that even when this variable is enabled, a warning - message will be emitted for each implicit literalFROM/ - entry referenced by a query. Users are encouraged to update - their applications to not rely on this behavior, by adding all - tables referenced by a query to the query's literalFROM/ - clause (or its literalUSING/ clause in the case of - commandDELETE/). -/para - /listitem - /varlistentry - varlistentry id=guc-array-nulls xreflabel=array_nulls termvarnamearray_nulls/varname (typeboolean/type)/term indexterm --- 4659,4664 Index: doc/src/sgml/queries.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/queries.sgml,v retrieving revision 1.55 diff -c -r1.55 queries.sgml *** doc/src/sgml/queries.sgml 17 Jun 2009 21:58:49 - 1.55 --- doc/src/sgml/queries.sgml 17 Oct 2009 20:37:00 - *** *** 521,543 /para para ! The alias becomes the new name of the table reference for the ! current query mdash; it is no longer possible to refer to the table ! by the original name. Thus: ! programlisting ! SELECT * FROM my_table AS m WHERE my_table.a gt; 5; ! /programlisting ! is not valid according to the SQL standard. In ! productnamePostgreSQL/productname this will draw an error, assuming the ! xref linkend=guc-add-missing-from configuration variable is ! literaloff/ (as it is by default). If it is literalon/, ! an implicit table reference will be added to the ! literalFROM/literal clause, so the query is processed as if ! it were written as: programlisting ! SELECT * FROM my_table AS m, my_table AS my_table WHERE my_table.a gt; 5; /programlisting - That will result in a cross join, which is usually not what you want. /para para --- 521,533 /para para ! The alias becomes the new name of the table reference so far as the ! current query is concerned mdash; it is not allowed to refer to the ! table by the original name elsewhere in the query. Thus, this is not ! valid: programlisting ! SELECT * FROM my_table AS m WHERE my_table.a gt; 5;-- wrong /programlisting /para para Index: doc/src/sgml/ref/select.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ref/select.sgml,v retrieving revision 1.125 diff -c -r1.125 select.sgml *** doc/src/sgml/ref/select.sgml 18 Sep 2009 05:00:42 - 1.125 --- doc/src/sgml/ref/select.sgml 17 Oct 2009 20:37:01 - *** *** 1451,1462 productnamePostgreSQL/productname releases prior to 8.1 would accept queries of this form, and add an implicit entry to the query's literalFROM/literal clause for each table ! referenced by the query. This is no longer the default behavior, ! because it does not comply with the SQL standard, and is ! considered by many to be error-prone. For compatibility with ! applications that rely on this behavior the xref ! linkend=guc-add-missing-from configuration variable can be ! enabled. /para /refsect2 --- 1451,1457
Re: [HACKERS] Deprecation
David Fetter da...@fetter.org writes: We have some really silly legacy stuff in PostgreSQL, the silliest of which, as far as I've found, is the add_missing_from GUC. Considering that we just had a discussion about a significant application that's still using it, I'm not sure what's your hurry. Is your intent specifically to break OpenACS in hopes of getting their attention? If so, you need to be a bit more up-front about that. (I would actually not mind getting rid of it, because that would greatly simplify a problem I'm wrestling with right now, namely how to put hooks into the parser for resolution of plpgsql variables. But we need to be honest about what it's going to do to users.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecation
On Fri, Oct 16, 2009 at 01:23:16PM -0400, Tom Lane wrote: David Fetter da...@fetter.org writes: We have some really silly legacy stuff in PostgreSQL, the silliest of which, as far as I've found, is the add_missing_from GUC. Considering that we just had a discussion about a significant application that's still using it, I'm not sure what's your hurry. Is your intent specifically to break OpenACS in hopes of getting their attention? If so, you need to be a bit more up-front about that. (I would actually not mind getting rid of it, because that would greatly simplify a problem I'm wrestling with right now, namely how to put hooks into the parser for resolution of plpgsql variables. But we need to be honest about what it's going to do to users.) Breaking legacy applications is a side effect, one we should probably publish far and wide, of the code cleanup. My hidden agenda, such as it is, is to make sure people don't get the misapprehension that they can just fire and forget with PostgreSQL, or any other software. Interfaces change; ugly kludges get removed, and they need to build their processes around this fact rather than around wishful thinking. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecation
On Fri, Oct 16, 2009 at 10:23 AM, Tom Lane t...@sss.pgh.pa.us wrote: (I would actually not mind getting rid of it, because that would greatly simplify a problem I'm wrestling with right now, namely how to put hooks into the parser for resolution of plpgsql variables. But we need to be honest about what it's going to do to users.) I was actually expecting you to come down on the side of keep it until it gets in the way. But if it's getting in the way already then it seems reasonable to start talking about getting rid of it. It only affects OpenACS if they want to upgrade to 8.5 which will presumably mean other application changes as well. Though probaby none requiring as much major code changes as this. -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecation
Greg Stark gsst...@mit.edu writes: On Fri, Oct 16, 2009 at 10:23 AM, Tom Lane t...@sss.pgh.pa.us wrote: (I would actually not mind getting rid of it, because that would greatly simplify a problem I'm wrestling with right now, namely how to put hooks into the parser for resolution of plpgsql variables. But we need to be honest about what it's going to do to users.) I was actually expecting you to come down on the side of keep it until it gets in the way. But if it's getting in the way already then it seems reasonable to start talking about getting rid of it. Yeah. The problem is that I'd like to have plpgsql install a hook that runs at the end of transformColumnRef and has an opportunity to provide a reference to a plpgsql var if nothing was found in the normal SQL lookups. However, if the columnref looks like x.y where x happens to match some table in the database (and not in the query) that doesn't have a column y, the implicit-RTE code will have already modified the querytree before finding out that column y doesn't exist. While that can probably be unwound somehow, it's going to be a major PITA, and there would be no way to hide the fact that a lock got taken on x before we changed our minds about including it in the query. Considering that this is all legacy behavior anyway it would be nice to not have to fix it. So, while I do think it's something we should leave alone until it gets in the way, this is a sufficiently large value of in the way that I'm willing to talk about removing add_missing_from. I'm just concerned about the impact of that, considering that an app that still depends on it came up as recently as yesterday. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecation
On Fri, Oct 16, 2009 at 9:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: So, while I do think it's something we should leave alone until it gets in the way, this is a sufficiently large value of in the way that I'm willing to talk about removing add_missing_from. I'm just concerned about the impact of that, considering that an app that still depends on it came up as recently as yesterday. 8.4 is going to be supported for several years so they can document it's the last version supported by the current version of OpenACS. If they want the benefits from 8.5 and higher (for themselves or their users), they'll do the work needed to remove the various bad practices from their code. Note that the removal of the implicit casts to text was far more problematic for a lot of apps than add_missing_from=off - which is the de facto standard for several years now. And as you can see in http://openacs.org/forums/message-view?message_id=2471518 , they did the ground work to support the tsearch2 changes from contrib to core. A friendly email to them explaining why it needs to be fixed in their code should be sufficient (and it might be already fixed, btw). -- Guillaume -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecation
On Fri, Oct 16, 2009 at 12:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: However, if the columnref looks like x.y where x happens to match some table in the database (and not in the query) that doesn't have a column y, the implicit-RTE code will have already modified the querytree before finding out that column y doesn't exist. Hm, so if you do nothing then really the only thing that doesn't work is if you have add_missing_from then plpgsql record variables wouldn't work when you tried to reference their columns? Perhaps we could just document that add_missing_from breaks that case? The worst thing about that is that someone could switch that option on globally on their server and break code that's part of some third-party module. -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecation
On Fri, 16 Oct 2009, Greg Stark wrote: It only affects OpenACS if they want to upgrade to 8.5 which will presumably mean other application changes as well. Though probaby none requiring as much major code changes as this. Being one that hosts alot of OACS sites, and has a fair experience with it, I agree ... as long as this isn't something that is going to be backpatched, there should be no reason why this can't go in for 8.5 ... the OACS guys will either fix the code, or stick to 8.4 ... Marc G. FournierHub.Org Hosting Solutions S.A. scra...@hub.org http://www.hub.org Yahoo:yscrappySkype: hub.orgICQ:7615664MSN:scra...@hub.org -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Deprecation
On Fri, 16 Oct 2009, Tom Lane wrote: So, while I do think it's something we should leave alone until it gets in the way, this is a sufficiently large value of in the way that I'm willing to talk about removing add_missing_from. I'm just concerned about the impact of that, considering that an app that still depends on it came up as recently as yesterday. As this should / would only affect 8.5+, just means that the app in question has to be stuck at 8.4 or fix the code. Since, as David points out, this 'hack' has been in there since 8.1, I think we've given the app in question more then sufficient time to fix their code already, no? 3 years, or so? Marc G. FournierHub.Org Hosting Solutions S.A. scra...@hub.org http://www.hub.org Yahoo:yscrappySkype: hub.orgICQ:7615664MSN:scra...@hub.org -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
