Обсуждение: Deprecation

Поиск
Список
Период
Сортировка

Deprecation

От
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 <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


Re: Deprecation

От
Tom Lane
Дата:
David Fetter <david@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


Re: Deprecation

От
David Fetter
Дата:
On Fri, Oct 16, 2009 at 01:23:16PM -0400, Tom Lane wrote:
> David Fetter <david@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 <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


Re: Deprecation

От
Greg Stark
Дата:
On Fri, Oct 16, 2009 at 10:23 AM, Tom Lane <tgl@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


Re: Deprecation

От
Tom Lane
Дата:
Greg Stark <gsstark@mit.edu> writes:
> On Fri, Oct 16, 2009 at 10:23 AM, Tom Lane <tgl@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


Re: Deprecation

От
Guillaume Smet
Дата:
On Fri, Oct 16, 2009 at 9:26 PM, Tom Lane <tgl@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


Re: Deprecation

От
Greg Stark
Дата:
On Fri, Oct 16, 2009 at 12:26 PM, Tom Lane <tgl@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


Re: Deprecation

От
"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. Fournier                        Hub.Org Hosting Solutions S.A.
scrappy@hub.org                                     http://www.hub.org

Yahoo:yscrappy    Skype: hub.org    ICQ:7615664    MSN:scrappy@hub.org


Re: Deprecation

От
"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. Fournier                        Hub.Org Hosting Solutions S.A.
scrappy@hub.org                                     http://www.hub.org

Yahoo:yscrappy    Skype: hub.org    ICQ:7615664    MSN:scrappy@hub.org


Re: Deprecation

От
Tom Lane
Дата:
Greg Stark <gsstark@mit.edu> writes:
> On Fri, Oct 16, 2009 at 12:26 PM, Tom Lane <tgl@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


Re: Deprecation

От
Bruce Momjian
Дата:
Tom Lane wrote:
> Greg Stark <gsstark@mit.edu> writes:
> > On Fri, Oct 16, 2009 at 12:26 PM, Tom Lane <tgl@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  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +


Re: Deprecation

От
Dimitri Fontaine
Дата:
Tom Lane <tgl@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


Re: Deprecation

От
Pavel Stehule
Дата:
2009/10/17 Dimitri Fontaine <dfontaine@hi-media.com>:
> Tom Lane <tgl@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
>


Re: Deprecation

От
Tom Lane
Дата:
Bruce Momjian <bruce@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


Re: Deprecation

От
Andrew Dunstan
Дата:

Tom Lane wrote:
> Bruce Momjian <bruce@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



Re: Deprecation

От
Tom Lane
Дата:
Andrew Dunstan <andrew@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 -0000    1.230
--- doc/src/sgml/config.sgml    17 Oct 2009 20:37:00 -0000
***************
*** 4659,4695 ****

       <variablelist>

-      <varlistentry id="guc-add-missing-from" xreflabel="add_missing_from">
-       <term><varname>add_missing_from</varname> (<type>boolean</type>)</term>
-       <indexterm><primary>FROM</><secondary>missing</></>
-       <indexterm>
-        <primary><varname>add_missing_from</> configuration parameter</primary>
-       </indexterm>
-       <listitem>
-        <para>
-         When on, tables that are referenced by a query will be
-         automatically added to the <literal>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 <literal>off</>. This variable can be
-         enabled for compatibility with releases of
-         <productname>PostgreSQL</> 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 <literal>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 <literal>FROM</>
-         clause (or its <literal>USING</> clause in the case of
-         <command>DELETE</>).
-        </para>
-       </listitem>
-      </varlistentry>
-
       <varlistentry id="guc-array-nulls" xreflabel="array_nulls">
        <term><varname>array_nulls</varname> (<type>boolean</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 -0000    1.55
--- doc/src/sgml/queries.sgml    17 Oct 2009 20:37:00 -0000
***************
*** 521,543 ****
      </para>

      <para>
!      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:
! <programlisting>
! SELECT * FROM my_table AS m WHERE my_table.a > 5;
! </programlisting>
!      is not valid according to the SQL standard.  In
!      <productname>PostgreSQL</productname> this will draw an error, assuming the
!      <xref linkend="guc-add-missing-from"> configuration variable is
!      <literal>off</> (as it is by default).  If it is <literal>on</>,
!      an implicit table reference will be added to the
!      <literal>FROM</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 > 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 — 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 > 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 -0000    1.125
--- doc/src/sgml/ref/select.sgml    17 Oct 2009 20:37:01 -0000
***************
*** 1451,1462 ****
      <productname>PostgreSQL</productname> releases prior to
      8.1 would accept queries of this form, and add an implicit entry
      to the query's <literal>FROM</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 ----
      <productname>PostgreSQL</productname> releases prior to
      8.1 would accept queries of this form, and add an implicit entry
      to the query's <literal>FROM</literal> clause for each table
!     referenced by the query. This is no longer allowed.
     </para>
    </refsect2>

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 -0000    1.47
--- doc/src/sgml/ref/show.sgml    17 Oct 2009 20:37:01 -0000
***************
*** 168,183 ****
     Show all settings:
  <programlisting>
  SHOW ALL;
!               name              |            setting             |
description                                          
!
--------------------------------+--------------------------------+----------------------------------------------------------------------------------------------
!  add_missing_from               | off                            | Automatically adds missing table references to
FROMclauses. 
!  allow_system_table_mods        | off                            | Allows modifications of the structure of system
tables.
      .
      .
      .
!  work_mem                       | 1024                           | Sets the maximum memory to be used for query
workspaces.
!  zero_damaged_pages             | off                            | Continues processing past damaged page headers.
! (146 rows)
  </programlisting>
    </para>
   </refsect1>
--- 168,182 ----
     Show all settings:
  <programlisting>
  SHOW ALL;
!               name               |                     setting                     |
                       description                                                           
!
---------------------------------+-------------------------------------------------+-------------------------------------------------------------------------------------------------------------------------------
!  allow_system_table_mods         | off                                             | Allows modifications of the
structureof system tables. 
      .
      .
      .
!  xmloption                       | content                                         | Sets whether XML data in
implicitparsing and serialization operations is to be considered as documents or content fragments. 
!  zero_damaged_pages              | off                                             | Continues processing past
damagedpage headers. 
! (196 rows)
  </programlisting>
    </para>
   </refsect1>
Index: src/backend/parser/parse_expr.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/parse_expr.c,v
retrieving revision 1.244
diff -c -r1.244 parse_expr.c
*** src/backend/parser/parse_expr.c    8 Oct 2009 02:39:23 -0000    1.244
--- src/backend/parser/parse_expr.c    17 Oct 2009 20:37:01 -0000
***************
*** 500,513 ****
                  name2 = strVal(field2);

                  /* Try to identify as a once-qualified column */
!                 node = qualifiedNameToVar(pstate, NULL, name1, name2, true,
                                            cref->location);
                  if (node == NULL)
                  {
                      /*
                       * Not known as a column of any range-table entry, so try
!                      * it as a function call.  Here, we will create an
!                      * implicit RTE for tables not already entered.
                       */
                      node = transformWholeRowRef(pstate, NULL, name1,
                                                  cref->location);
--- 500,512 ----
                  name2 = strVal(field2);

                  /* Try to identify as a once-qualified column */
!                 node = qualifiedNameToVar(pstate, NULL, name1, name2,
                                            cref->location);
                  if (node == NULL)
                  {
                      /*
                       * Not known as a column of any range-table entry, so try
!                      * it as a function call.
                       */
                      node = transformWholeRowRef(pstate, NULL, name1,
                                                  cref->location);
***************
*** 545,551 ****
                  name3 = strVal(field3);

                  /* Try to identify as a twice-qualified column */
!                 node = qualifiedNameToVar(pstate, name1, name2, name3, true,
                                            cref->location);
                  if (node == NULL)
                  {
--- 544,550 ----
                  name3 = strVal(field3);

                  /* Try to identify as a twice-qualified column */
!                 node = qualifiedNameToVar(pstate, name1, name2, name3,
                                            cref->location);
                  if (node == NULL)
                  {
***************
*** 600,606 ****
                  name4 = strVal(field4);

                  /* Try to identify as a twice-qualified column */
!                 node = qualifiedNameToVar(pstate, name2, name3, name4, true,
                                            cref->location);
                  if (node == NULL)
                  {
--- 599,605 ----
                  name4 = strVal(field4);

                  /* Try to identify as a twice-qualified column */
!                 node = qualifiedNameToVar(pstate, name2, name3, name4,
                                            cref->location);
                  if (node == NULL)
                  {
***************
*** 1906,1919 ****
      int            sublevels_up;
      Oid            toid;

!     /* Look up the referenced RTE, creating it if needed */

      rte = refnameRangeTblEntry(pstate, schemaname, relname, location,
                                 &sublevels_up);

      if (rte == NULL)
!         rte = addImplicitRTE(pstate,
!                              makeRangeVar(schemaname, relname, location));

      vnum = RTERangeTablePosn(pstate, rte, &sublevels_up);

--- 1905,1918 ----
      int            sublevels_up;
      Oid            toid;

!     /* Look up the referenced RTE, failing if not present */

      rte = refnameRangeTblEntry(pstate, schemaname, relname, location,
                                 &sublevels_up);

      if (rte == NULL)
!         errorMissingRTE(pstate,
!                         makeRangeVar(schemaname, relname, location));

      vnum = RTERangeTablePosn(pstate, rte, &sublevels_up);

Index: src/backend/parser/parse_relation.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/parse_relation.c,v
retrieving revision 1.143
diff -c -r1.143 parse_relation.c
*** src/backend/parser/parse_relation.c    16 Jul 2009 06:33:43 -0000    1.143
--- src/backend/parser/parse_relation.c    17 Oct 2009 20:37:01 -0000
***************
*** 32,40 ****
  #include "utils/syscache.h"


- /* GUC parameter */
- bool        add_missing_from;
-
  static RangeTblEntry *scanNameSpaceForRefname(ParseState *pstate,
                          const char *refname, int location);
  static RangeTblEntry *scanNameSpaceForRelid(ParseState *pstate, Oid relid,
--- 32,37 ----
***************
*** 51,57 ****
                  int location, bool include_dropped,
                  List **colnames, List **colvars);
  static int    specialAttNum(const char *attname);
- static void warnAutoRange(ParseState *pstate, RangeVar *relation);


  /*
--- 48,53 ----
***************
*** 249,255 ****
   * visible in the p_relnamespace lists.  This behavior is invalid per the SQL
   * spec, and it may give ambiguous results (there might be multiple equally
   * valid matches, but only one will be returned).  This must be used ONLY
!  * as a heuristic in giving suitable error messages.  See warnAutoRange.
   *
   * Notice that we consider both matches on actual relation (or CTE) name
   * and matches on alias.
--- 245,251 ----
   * visible in the p_relnamespace lists.  This behavior is invalid per the SQL
   * spec, and it may give ambiguous results (there might be multiple equally
   * valid matches, but only one will be returned).  This must be used ONLY
!  * as a heuristic in giving suitable error messages.  See errorMissingRTE.
   *
   * Notice that we consider both matches on actual relation (or CTE) name
   * and matches on alias.
***************
*** 573,579 ****
                     char *schemaname,
                     char *refname,
                     char *colname,
-                    bool implicitRTEOK,
                     int location)
  {
      RangeTblEntry *rte;
--- 569,574 ----
***************
*** 581,594 ****

      rte = refnameRangeTblEntry(pstate, schemaname, refname, location,
                                 &sublevels_up);
-
      if (rte == NULL)
!     {
!         if (!implicitRTEOK)
!             return NULL;
!         rte = addImplicitRTE(pstate,
!                              makeRangeVar(schemaname, refname, location));
!     }

      return scanRTEForColumn(pstate, rte, colname, location);
  }
--- 576,583 ----

      rte = refnameRangeTblEntry(pstate, schemaname, refname, location,
                                 &sublevels_up);
      if (rte == NULL)
!         return NULL;

      return scanRTEForColumn(pstate, rte, colname, location);
  }
***************
*** 1528,1569 ****
  }

  /*
-  * Add a POSTQUEL-style implicit RTE.
-  *
-  * We assume caller has already checked that there is no RTE or join with
-  * a conflicting name.
-  */
- RangeTblEntry *
- addImplicitRTE(ParseState *pstate, RangeVar *relation)
- {
-     CommonTableExpr *cte = NULL;
-     Index        levelsup = 0;
-     RangeTblEntry *rte;
-
-     /* issue warning or error as needed */
-     warnAutoRange(pstate, relation);
-
-     /* if it is an unqualified name, it might be a CTE reference */
-     if (!relation->schemaname)
-         cte = scanNameSpaceForCTE(pstate, relation->relname, &levelsup);
-
-     /*
-      * Note that we set inFromCl true, so that the RTE will be listed
-      * explicitly if the parsetree is ever decompiled by ruleutils.c. This
-      * provides a migration path for views/rules that were originally written
-      * with implicit-RTE syntax.
-      */
-     if (cte)
-         rte = addRangeTableEntryForCTE(pstate, cte, levelsup, NULL, true);
-     else
-         rte = addRangeTableEntry(pstate, relation, NULL, false, true);
-     /* Add to joinlist and relnamespace, but not varnamespace */
-     addRTEtoQuery(pstate, rte, true, true, false);
-
-     return rte;
- }
-
- /*
   * expandRTE -- expand the columns of a rangetable entry
   *
   * This creates lists of an RTE's column names (aliases if provided, else
--- 1517,1522 ----
***************
*** 2417,2429 ****
  }

  /*
!  * Generate a warning or error about an implicit RTE, if appropriate.
   *
!  * If ADD_MISSING_FROM is not enabled, raise an error. Otherwise, emit
!  * a warning.
   */
! static void
! warnAutoRange(ParseState *pstate, RangeVar *relation)
  {
      RangeTblEntry *rte;
      int            sublevels_up;
--- 2370,2382 ----
  }

  /*
!  * Generate a suitable error about a missing RTE.
   *
!  * Since this is a very common type of error, we work rather hard to
!  * produce a helpful message.
   */
! void
! errorMissingRTE(ParseState *pstate, RangeVar *relation)
  {
      RangeTblEntry *rte;
      int            sublevels_up;
***************
*** 2431,2437 ****

      /*
       * Check to see if there are any potential matches in the query's
!      * rangetable.    This affects the message we provide.
       */
      rte = searchRangeTable(pstate, relation);

--- 2384,2390 ----

      /*
       * Check to see if there are any potential matches in the query's
!      * rangetable.
       */
      rte = searchRangeTable(pstate, relation);

***************
*** 2452,2490 ****
                               &sublevels_up) == rte)
          badAlias = rte->eref->aliasname;

!     if (!add_missing_from)
!     {
!         if (rte)
!             ereport(ERROR,
!                     (errcode(ERRCODE_UNDEFINED_TABLE),
!             errmsg("invalid reference to FROM-clause entry for table \"%s\"",
!                    relation->relname),
!                      (badAlias ?
!             errhint("Perhaps you meant to reference the table alias \"%s\".",
!                     badAlias) :
!                       errhint("There is an entry for table \"%s\", but it cannot be referenced from this part of the
query.",
!                               rte->eref->aliasname)),
!                      parser_errposition(pstate, relation->location)));
!         else
!             ereport(ERROR,
!                     (errcode(ERRCODE_UNDEFINED_TABLE),
!                      errmsg("missing FROM-clause entry for table \"%s\"",
!                             relation->relname),
!                      parser_errposition(pstate, relation->location)));
!     }
!     else
!     {
!         /* just issue a warning */
!         ereport(NOTICE,
                  (errcode(ERRCODE_UNDEFINED_TABLE),
!                  errmsg("adding missing FROM-clause entry for table \"%s\"",
                          relation->relname),
                   (badAlias ?
!             errhint("Perhaps you meant to reference the table alias \"%s\".",
!                     badAlias) :
!                   (rte ?
!                    errhint("There is an entry for table \"%s\", but it cannot be referenced from this part of the
query.",
!                            rte->eref->aliasname) : 0)),
                   parser_errposition(pstate, relation->location)));
-     }
  }
--- 2405,2425 ----
                               &sublevels_up) == rte)
          badAlias = rte->eref->aliasname;

!     if (rte)
!         ereport(ERROR,
                  (errcode(ERRCODE_UNDEFINED_TABLE),
!                  errmsg("invalid reference to FROM-clause entry for table \"%s\"",
                          relation->relname),
                   (badAlias ?
!                   errhint("Perhaps you meant to reference the table alias \"%s\".",
!                           badAlias) :
!                   errhint("There is an entry for table \"%s\", but it cannot be referenced from this part of the
query.",
!                           rte->eref->aliasname)),
!                  parser_errposition(pstate, relation->location)));
!     else
!         ereport(ERROR,
!                 (errcode(ERRCODE_UNDEFINED_TABLE),
!                  errmsg("missing FROM-clause entry for table \"%s\"",
!                         relation->relname),
                   parser_errposition(pstate, relation->location)));
  }
Index: src/backend/parser/parse_target.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/parse_target.c,v
retrieving revision 1.172
diff -c -r1.172 parse_target.c
*** src/backend/parser/parse_target.c    16 Jul 2009 06:33:43 -0000    1.172
--- src/backend/parser/parse_target.c    17 Oct 2009 20:37:01 -0000
***************
*** 927,935 ****
          rte = refnameRangeTblEntry(pstate, schemaname, relname, cref->location,
                                     &sublevels_up);
          if (rte == NULL)
!             rte = addImplicitRTE(pstate,
!                                  makeRangeVar(schemaname, relname,
!                                               cref->location));

          rtindex = RTERangeTablePosn(pstate, rte, &sublevels_up);

--- 927,934 ----
          rte = refnameRangeTblEntry(pstate, schemaname, relname, cref->location,
                                     &sublevels_up);
          if (rte == NULL)
!             errorMissingRTE(pstate,
!                             makeRangeVar(schemaname, relname, cref->location));

          rtindex = RTERangeTablePosn(pstate, rte, &sublevels_up);

***************
*** 973,980 ****
   *
   * tlist entries are generated for each relation appearing in the query's
   * varnamespace.  We do not consider relnamespace because that would include
!  * input tables of aliasless JOINs, NEW/OLD pseudo-entries, implicit RTEs,
!  * etc.
   *
   * The referenced relations/columns are marked as requiring SELECT access.
   */
--- 972,978 ----
   *
   * tlist entries are generated for each relation appearing in the query's
   * varnamespace.  We do not consider relnamespace because that would include
!  * input tables of aliasless JOINs, NEW/OLD pseudo-entries, etc.
   *
   * The referenced relations/columns are marked as requiring SELECT access.
   */
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.521
diff -c -r1.521 guc.c
*** src/backend/utils/misc/guc.c    13 Oct 2009 14:18:40 -0000    1.521
--- src/backend/utils/misc/guc.c    17 Oct 2009 20:37:01 -0000
***************
*** 45,51 ****
  #include "optimizer/paths.h"
  #include "optimizer/planmain.h"
  #include "parser/parse_expr.h"
- #include "parser/parse_relation.h"
  #include "parser/parse_type.h"
  #include "parser/parser.h"
  #include "parser/scansup.h"
--- 45,50 ----
***************
*** 1058,1071 ****
          false, assign_transaction_read_only, NULL
      },
      {
-         {"add_missing_from", PGC_USERSET, COMPAT_OPTIONS_PREVIOUS,
-             gettext_noop("Automatically adds missing table references to FROM clauses."),
-             NULL
-         },
-         &add_missing_from,
-         false, NULL, NULL
-     },
-     {
          {"check_function_bodies", PGC_USERSET, CLIENT_CONN_STATEMENT,
              gettext_noop("Check function bodies during CREATE FUNCTION."),
              NULL
--- 1057,1062 ----
Index: src/backend/utils/misc/postgresql.conf.sample
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v
retrieving revision 1.267
diff -c -r1.267 postgresql.conf.sample
*** src/backend/utils/misc/postgresql.conf.sample    22 Sep 2009 23:43:38 -0000    1.267
--- src/backend/utils/misc/postgresql.conf.sample    17 Oct 2009 20:37:01 -0000
***************
*** 484,490 ****

  # - Previous PostgreSQL Versions -

- #add_missing_from = off
  #array_nulls = on
  #backslash_quote = safe_encoding    # on, off, or safe_encoding
  #default_with_oids = off
--- 484,489 ----
Index: src/include/parser/parse_relation.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/parser/parse_relation.h,v
retrieving revision 1.64
diff -c -r1.64 parse_relation.h
*** src/include/parser/parse_relation.h    11 Jun 2009 14:49:11 -0000    1.64
--- src/include/parser/parse_relation.h    17 Oct 2009 20:37:01 -0000
***************
*** 16,23 ****

  #include "parser/parse_node.h"

- extern bool add_missing_from;
-
  extern RangeTblEntry *refnameRangeTblEntry(ParseState *pstate,
                       const char *schemaname,
                       const char *refname,
--- 16,21 ----
***************
*** 44,50 ****
                     char *schemaname,
                     char *refname,
                     char *colname,
-                    bool implicitRTEOK,
                     int location);
  extern void markVarForSelectPriv(ParseState *pstate, Var *var,
                       RangeTblEntry *rte);
--- 42,47 ----
***************
*** 87,93 ****
  extern void addRTEtoQuery(ParseState *pstate, RangeTblEntry *rte,
                bool addToJoinList,
                bool addToRelNameSpace, bool addToVarNameSpace);
! extern RangeTblEntry *addImplicitRTE(ParseState *pstate, RangeVar *relation);
  extern void expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
            int location, bool include_dropped,
            List **colnames, List **colvars);
--- 84,90 ----
  extern void addRTEtoQuery(ParseState *pstate, RangeTblEntry *rte,
                bool addToJoinList,
                bool addToRelNameSpace, bool addToVarNameSpace);
! extern void errorMissingRTE(ParseState *pstate, RangeVar *relation);
  extern void expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
            int location, bool include_dropped,
            List **colnames, List **colvars);
Index: src/test/regress/expected/delete.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/delete.out,v
retrieving revision 1.2
diff -c -r1.2 delete.out
*** src/test/regress/expected/delete.out    14 Mar 2006 22:50:46 -0000    1.2
--- src/test/regress/expected/delete.out    17 Oct 2009 20:37:01 -0000
***************
*** 11,24 ****
  DELETE FROM delete_test AS dt WHERE dt.a > 75;
  -- if an alias is specified, don't allow the original table name
  -- to be referenced
- BEGIN;
- SET LOCAL add_missing_from = false;
  DELETE FROM delete_test dt WHERE delete_test.a > 25;
  ERROR:  invalid reference to FROM-clause entry for table "delete_test"
  LINE 1: DELETE FROM delete_test dt WHERE delete_test.a > 25;
                                           ^
  HINT:  Perhaps you meant to reference the table alias "dt".
- ROLLBACK;
  SELECT * FROM delete_test;
   id | a
  ----+----
--- 11,21 ----
Index: src/test/regress/expected/rowtypes.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/rowtypes.out,v
retrieving revision 1.13
diff -c -r1.13 rowtypes.out
*** src/test/regress/expected/rowtypes.out    13 Oct 2008 16:25:20 -0000    1.13
--- src/test/regress/expected/rowtypes.out    17 Oct 2009 20:37:01 -0000
***************
*** 63,75 ****
    2 | ("(,4.4)","(5.5,6.6)")
  (2 rows)

- begin;
- set local add_missing_from = false;
  select f1, q.c1 from quadtable;        -- fails, q is a table reference
  ERROR:  missing FROM-clause entry for table "q"
  LINE 1: select f1, q.c1 from quadtable;
                     ^
- rollback;
  select f1, (q).c1, (qq.q).c1.i from quadtable qq;
   f1 |    c1     |  i
  ----+-----------+-----
--- 63,72 ----
Index: src/test/regress/expected/update.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/update.out,v
retrieving revision 1.5
diff -c -r1.5 update.out
*** src/test/regress/expected/update.out    3 Sep 2006 22:37:06 -0000    1.5
--- src/test/regress/expected/update.out    17 Oct 2009 20:37:01 -0000
***************
*** 82,93 ****
                                          ^
  -- if an alias for the target table is specified, don't allow references
  -- to the original table name
- BEGIN;
- SET LOCAL add_missing_from = false;
  UPDATE update_test AS t SET b = update_test.b + 10 WHERE t.a = 10;
  ERROR:  invalid reference to FROM-clause entry for table "update_test"
  LINE 1: UPDATE update_test AS t SET b = update_test.b + 10 WHERE t.a...
                                          ^
  HINT:  Perhaps you meant to reference the table alias "t".
- ROLLBACK;
  DROP TABLE update_test;
--- 82,90 ----
Index: src/test/regress/sql/delete.sql
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/sql/delete.sql,v
retrieving revision 1.1
diff -c -r1.1 delete.sql
*** src/test/regress/sql/delete.sql    22 Jan 2006 05:20:35 -0000    1.1
--- src/test/regress/sql/delete.sql    17 Oct 2009 20:37:01 -0000
***************
*** 12,22 ****

  -- if an alias is specified, don't allow the original table name
  -- to be referenced
- BEGIN;
- SET LOCAL add_missing_from = false;
  DELETE FROM delete_test dt WHERE delete_test.a > 25;
- ROLLBACK;

  SELECT * FROM delete_test;

! DROP TABLE delete_test;
\ No newline at end of file
--- 12,19 ----

  -- if an alias is specified, don't allow the original table name
  -- to be referenced
  DELETE FROM delete_test dt WHERE delete_test.a > 25;

  SELECT * FROM delete_test;

! DROP TABLE delete_test;
Index: src/test/regress/sql/rowtypes.sql
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/sql/rowtypes.sql,v
retrieving revision 1.9
diff -c -r1.9 rowtypes.sql
*** src/test/regress/sql/rowtypes.sql    13 Oct 2008 16:25:20 -0000    1.9
--- src/test/regress/sql/rowtypes.sql    17 Oct 2009 20:37:01 -0000
***************
*** 35,44 ****

  select * from quadtable;

- begin;
- set local add_missing_from = false;
  select f1, q.c1 from quadtable;        -- fails, q is a table reference
- rollback;

  select f1, (q).c1, (qq.q).c1.i from quadtable qq;

--- 35,41 ----
Index: src/test/regress/sql/update.sql
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/sql/update.sql,v
retrieving revision 1.5
diff -c -r1.5 update.sql
*** src/test/regress/sql/update.sql    3 Sep 2006 22:37:06 -0000    1.5
--- src/test/regress/sql/update.sql    17 Oct 2009 20:37:01 -0000
***************
*** 52,60 ****

  -- if an alias for the target table is specified, don't allow references
  -- to the original table name
- BEGIN;
- SET LOCAL add_missing_from = false;
  UPDATE update_test AS t SET b = update_test.b + 10 WHERE t.a = 10;
- ROLLBACK;

  DROP TABLE update_test;
--- 52,57 ----

Re: Deprecation

От
daveg
Дата:
On Sat, Oct 17, 2009 at 03:01:27PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@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       daveg@sonic.net      510 536 1443    510 282 0869
If simplicity worked, the world would be overrun with insects.