Re: [HACKERS] Deprecation

2009-10-19 Thread daveg
On Sat, Oct 17, 2009 at 03:01:27PM -0400, Tom Lane wrote:
> Bruce Momjian  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

2009-10-17 Thread Tom Lane
Andrew Dunstan  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 
  
   
  
-  
-   add_missing_from (boolean)
-   FROMmissing
-   
-add_missing_from configuration parameter
-   
-   
-
- When on, tables that are referenced by a query will be
- automatically added to the FROM 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 off. This variable can be
- enabled for compatibility with releases of
- PostgreSQL prior to 8.1, where this behavior was
- allowed by default.
-
- 
-
- Note that even when this variable is enabled, a warning
- message will be emitted for each implicit FROM
- 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 FROM
- clause (or its USING clause in the case of
- DELETE).
-
-   
-  
- 
   
array_nulls (boolean)

--- 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 
  
  
  
!  The alias becomes the new name of the table reference for the
!  current query — it is no longer possible to refer to the table
!  by the original name.  Thus:
! 
! SELECT * FROM my_table AS m WHERE my_table.a > 5;
! 
!  is not valid according to the SQL standard.  In
!  PostgreSQL this will draw an error, assuming the
!   configuration variable is
!  off (as it is by default).  If it is on,
!  an implicit table reference will be added to the
!  FROM clause, so the query is processed as if
!  it were written as:
  
! SELECT * FROM my_table AS m, my_table AS my_table WHERE my_table.a > 5;
  
-  That will result in a cross join, which is usually not what you want.
  
  
  
--- 521,533 
  
  
  
!  The alias becomes the new name of the table reference so far as the
!  current query is concerned — it is not allowed to refer to the
!  table by the original name elsewhere in the query.  Thus, this is not
!  valid:
  
! SELECT * FROM my_table AS m WHERE my_table.a > 5;-- wrong
  
  
  
  
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 
  PostgreSQL releases prior to
  8.1 would accept queries of this form, and add an implicit entry
  to the query's FROM 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  configuration variable can be
! enabled.
 

  
--- 1451,1457 
  PostgreSQL releases prior to
  8.1 would accept queries of this form, and add an implicit entry
  to the query's FROM clause for each table
! referenced by the query. This is no longer allowed.
 

  
Index: doc/src/sgml/ref/show.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/show.sgml,v
retrieving revision 1.47
diff -c -r1.47 show.sgml
*** doc/src/sgml/ref/show.sgml	14 Nov 2008 10:22:47 -	1.47
--- doc/src/sgml/ref/show.sgml	17 Oct 2009 20:37:01 -
***
*** 168,183 
 Show all settings:
  
  SHOW ALL;
!   name  |setting | de

Re: [HACKERS] Deprecation

2009-10-17 Thread Andrew Dunstan



Tom Lane wrote:

Bruce Momjian  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

2009-10-17 Thread Tom Lane
Bruce Momjian  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

2009-10-17 Thread Pavel Stehule
2009/10/17 Dimitri Fontaine :
> Tom Lane  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

2009-10-17 Thread Dimitri Fontaine
Tom Lane  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 Thread Bruce Momjian
Tom Lane wrote:
> Greg Stark  writes:
> > On Fri, Oct 16, 2009 at 12:26 PM, Tom Lane  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  http://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

2009-10-17 Thread Tom Lane
Greg Stark  writes:
> On Fri, Oct 16, 2009 at 12:26 PM, Tom Lane  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

2009-10-16 Thread Marc G. Fournier

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


Re: [HACKERS] Deprecation

2009-10-16 Thread Marc G. Fournier

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

2009-10-16 Thread Greg Stark
On Fri, Oct 16, 2009 at 12:26 PM, Tom Lane  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

2009-10-16 Thread Guillaume Smet
On Fri, Oct 16, 2009 at 9:26 PM, 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.

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

2009-10-16 Thread Tom Lane
Greg Stark  writes:
> On Fri, Oct 16, 2009 at 10:23 AM, Tom Lane  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

2009-10-16 Thread Greg Stark
On Fri, Oct 16, 2009 at 10:23 AM, Tom Lane  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

2009-10-16 Thread David Fetter
On Fri, Oct 16, 2009 at 01:23:16PM -0400, Tom Lane wrote:
> David Fetter  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  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

2009-10-16 Thread Tom Lane
David Fetter  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


[HACKERS] Deprecation

2009-10-16 Thread David Fetter
Folks,

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.

Since it's been deprecated, as in off by default, since 8.1, I'd like
to suggest that we remove it from both docs and code and throw an
error if someone tries to set it, just as if they'd set
add_flying_spaghetti_monster.

What say?

Cheers,
David.
-- 
David Fetter  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