New warning code for missing FROM relations

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I have committed new warning code to alert users who auto-create
relations without knowing it.
The code issues the warning if it auto-creates a range table entry, and
there is already a range table entry identified as coming from a FROM
clause. Correlated subqueries should not be a problem because they are
not auto-created.

I still prefer the suggestion I made before: complain only if the
implicit FROM entry is for a table already present in the rangelist
(under a different alias, obviously). The fact that that choice
would not break any existing regression tests seems relevant...

regards, tom lane

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I have committed new warning code to alert users who auto-create
relations without knowing it.
The code issues the warning if it auto-creates a range table entry, and
there is already a range table entry identified as coming from a FROM
clause. Correlated subqueries should not be a problem because they are
not auto-created.

I still prefer the suggestion I made before: complain only if the
implicit FROM entry is for a table already present in the rangelist
(under a different alias, obviously). The fact that that choice
would not break any existing regression tests seems relevant...

But it seems mine is going to complain if they forget one in a FROM
clause, which sort of makes sense to me. I can do your suggestion, but
this makes more sense. Can we get some other votes?

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I still prefer the suggestion I made before: complain only if the
implicit FROM entry is for a table already present in the rangelist
(under a different alias, obviously). The fact that that choice
would not break any existing regression tests seems relevant...

But it seems mine is going to complain if they forget one in a FROM
clause, which sort of makes sense to me.

Seems like the real question is what is the goal of having the warning.
Are we (a) trying to nag people into writing their queries in an
SQL-compliant way, or are we (b) trying to warn about probable mistakes
while still considering implicit FROM entries as a fully supported
Postgres feature?

If the goal is (a) then your way is better, but I like mine if the goal
is (b). Seems like some discussion is needed here about just what we
want to accomplish.

regards, tom lane

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I still prefer the suggestion I made before: complain only if the
implicit FROM entry is for a table already present in the rangelist
(under a different alias, obviously). The fact that that choice
would not break any existing regression tests seems relevant...

But it seems mine is going to complain if they forget one in a FROM
clause, which sort of makes sense to me.

Seems like the real question is what is the goal of having the warning.
Are we (a) trying to nag people into writing their queries in an
SQL-compliant way, or are we (b) trying to warn about probable mistakes
while still considering implicit FROM entries as a fully supported
Postgres feature?

If the goal is (a) then your way is better, but I like mine if the goal
is (b). Seems like some discussion is needed here about just what we
want to accomplish.

I agree the goal is (b). However, I can not imagine a query with a FROM
clause that would ever want to use auto-creation of range entries.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

At 00:40 3/06/00 -0400, Bruce Momjian wrote:

The regression tests run fine, except for:

SELECT *
INTO TABLE tmp1
FROM tmp
WHERE onek.unique1 < 2;
NOTICE: Adding missing FROM-clause entry for table onek

Personally I would prefer this to generate an error, eg:

Table 'onek' refereenced in the WHERE clause is not in the FROM clause.

Is is worth adding yet another setting, eg. set sql92=strict, which would
disallow such flagrant breaches of the standard? Maybe it could even be set
as the default in template1? I understand that breaking legacy code is a
bad idea, so the warning is a good step, but I would prefer an error if I
ever write such a statement.

Other DBs I've worked with issue warnings for several version before
changing default behaviour, so perhaps in version 8.0, the above code could
prduce an error by default (unless 'set sql92=relaxed' was specified).

P.S. Given that 'set sql92=xxx' may imply that we are enforcing compliance,
which is unlikely, maybe it should be 'set implied-tables' or something
more specific and meaningful. I have no idea how the 'set' command works,
but to avoid a whole lot of new variables, maybe 'set
sql-compiance-options=no-implied-tables, no-something-else,
allow-another-thing' would allow for expansion...

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.C.N. 008 659 498) | /(@) ______---_
Tel: +61-03-5367 7422 | _________ \
Fax: +61-03-5367 7430 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

At 00:40 3/06/00 -0400, Bruce Momjian wrote:

The regression tests run fine, except for:

SELECT *
INTO TABLE tmp1
FROM tmp
WHERE onek.unique1 < 2;
NOTICE: Adding missing FROM-clause entry for table onek

Personally I would prefer this to generate an error, eg:

Table 'onek' refereenced in the WHERE clause is not in the FROM clause.

Yes, that was one of my stated options, return an error.

Is is worth adding yet another setting, eg. set sql92=strict, which would
disallow such flagrant breaches of the standard? Maybe it could even be set
as the default in template1? I understand that breaking legacy code is a
bad idea, so the warning is a good step, but I would prefer an error if I
ever write such a statement.

I am concerned about overloading the SET command. Seems we should just
agree on a behavior.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Is is worth adding yet another setting, eg. set sql92=strict, which would
disallow such flagrant breaches of the standard? Maybe it could even be set
as the default in template1? I understand that breaking legacy code is a
bad idea, so the warning is a good step, but I would prefer an error if I
ever write such a statement.

I am concerned about overloading the SET command. Seems we should just
agree on a behavior.

Then make it an option at build time.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.C.N. 008 659 498) | /(@) ______---_
Tel: +61-03-5367 7422 | _________ \
Fax: +61-03-5367 7430 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I have committed new warning code to alert users who auto-create
relations without knowing it.
The code issues the warning if it auto-creates a range table entry, and
there is already a range table entry identified as coming from a FROM
clause. Correlated subqueries should not be a problem because they are
not auto-created.

I still prefer the suggestion I made before: complain only if the
implicit FROM entry is for a table already present in the rangelist
(under a different alias, obviously). The fact that that choice
would not break any existing regression tests seems relevant...

But it seems mine is going to complain if they forget one in a FROM
clause, which sort of makes sense to me. I can do your suggestion, but
this makes more sense. Can we get some other votes?

I like it the way you did it. Personally I would even throw an error,
but that would probably be too strict.

I would change the regressiontest to add onek to the from clause,
and not make it throw the warning.
Imho this example is only good to demonstrate how you can
misuse a feature.

There are good examples for using it, but all of those that I can think of
don't have a from clause.

Andreas

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I still prefer the suggestion I made before: complain only if the
implicit FROM entry is for a table already present in the rangelist
(under a different alias, obviously). The fact that that choice
would not break any existing regression tests seems relevant...

But it seems mine is going to complain if they forget one in a FROM
clause, which sort of makes sense to me.

Seems like the real question is what is the goal of having the warning.
Are we (a) trying to nag people into writing their queries in an
SQL-compliant way, or are we (b) trying to warn about probable mistakes
while still considering implicit FROM entries as a fully supported
Postgres feature?

If the goal is (a) then your way is better, but I like mine if the goal
is (b). Seems like some discussion is needed here about just what we
want to accomplish.

I agree the goal is (b). However, I can not imagine a query with a FROM
clause that would ever want to use auto-creation of range entries.

how about:

delete from taba where a=tabb.a;

I think the implicit auto-creation should only be disallowed/warned in
select statements that have a from clause, not update and delete.

Andreas

If the goal is (a) then your way is better, but I like mine if the goal
is (b). Seems like some discussion is needed here about just what we
want to accomplish.

I agree the goal is (b). However, I can not imagine a query with a FROM
clause that would ever want to use auto-creation of range entries.

how about:

delete from taba where a=tabb.a;

I think the implicit auto-creation should only be disallowed/warned in
select statements that have a from clause, not update and delete.

I meant a SELECT FROM clause. A DELETE FROM is not the same, and does
not mark the entry as inFromCl. I should have been more specific.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

I like it the way you did it. Personally I would even throw an error,
but that would probably be too strict.

Yes, I figured the same.

I would change the regressiontest to add onek to the from clause,
and not make it throw the warning.
Imho this example is only good to demonstrate how you can
misuse a feature.

OK.

There are good examples for using it, but all of those that I can think of
don't have a from clause.

Yes, that was my logic.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Philip Warner writes:

SELECT *
INTO TABLE tmp1
FROM tmp
WHERE onek.unique1 < 2;
NOTICE: Adding missing FROM-clause entry for table onek

Is is worth adding yet another setting, eg. set sql92=strict, which
would disallow such flagrant breaches of the standard?

SQL provides for facility called the SQL Flagger, which is supposed to do
exactly that. This might sound like an interesting idea but in order for
it to be useful you'd have to maintain it across the board, which sounds
like a major head ache.

The irony in the given example is that the SELECT INTO command isn't in
the standard in the first place so you'd have to create all sorts of
double standards. Certain things would be "extensions", certain things
would be "misuse". And for all it's worth, we have no idea which is which.

If you want to throw about warnings about "probable" coding errors and the
like one *must* be able to switch them off. Either something is right,
then you shut up. Or it's wrong, then you throw an error. Or you're not
sure, then you better leave it up to the user.

--
Peter Eisentraut Sernanders v�g 10:115
peter_e@gmx.net 75262 Uppsala
http://yi.org/peter-e/ Sweden

SELECT *
INTO TABLE tmp1
FROM tmp
WHERE onek.unique1 < 2;
NOTICE: Adding missing FROM-clause entry for table onek

Is is worth adding yet another setting, eg. set sql92=strict, which
would disallow such flagrant breaches of the standard?

SQL provides for facility called the SQL Flagger, which is
supposed to do
exactly that. This might sound like an interesting idea but
in order for
it to be useful you'd have to maintain it across the board,
which sounds
like a major head ache.

The irony in the given example is that the SELECT INTO
command isn't in
the standard in the first place so you'd have to create all sorts of
double standards. Certain things would be "extensions", certain things
would be "misuse". And for all it's worth, we have no idea
which is which.

If you want to throw about warnings about "probable" coding
errors and the
like one *must* be able to switch them off. Either something is right,
then you shut up. Or it's wrong, then you throw an error. Or
you're not
sure, then you better leave it up to the user.

Yes, only Bruce and I are of the opinion that it *is* an Error, and I guess
we want some consensus.
The notice is imho of the sort: notice this syntax is going to be disallowed
soon.

Andreas

The irony in the given example is that the SELECT INTO
command isn't in
the standard in the first place so you'd have to create all sorts of
double standards. Certain things would be "extensions", certain things
would be "misuse". And for all it's worth, we have no idea
which is which.

If you want to throw about warnings about "probable" coding
errors and the
like one *must* be able to switch them off. Either something is right,
then you shut up. Or it's wrong, then you throw an error. Or
you're not
sure, then you better leave it up to the user.

Yes, only Bruce and I are of the opinion that it *is* an Error, and I guess
we want some consensus.
The notice is imho of the sort: notice this syntax is going to be disallowed
soon.

I see the notice as "Hey, you probably did something you didn't want to do".

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

At 10:02 5/06/00 +0200, Zeugswetter Andreas SB wrote:

SELECT *
INTO TABLE tmp1
FROM tmp
WHERE onek.unique1 < 2;
NOTICE: Adding missing FROM-clause entry for table onek

Yes, only Bruce and I are of the opinion that it *is* an Error, and I guess
we want some consensus.
The notice is imho of the sort: notice this syntax is going to be disallowed
soon.

FWIW, I think it's an error too. For me, this situation is *far* more
likely to result from typos than intention, and I want to be told when it
happens. I also want to be prevented from doing it.

I take it that there is no chance of a compile-time or runtime option to
disallow this kind of thing in all cases?

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.C.N. 008 659 498) | /(@) ______---_
Tel: +61-03-5367 7422 | _________ \
Fax: +61-03-5367 7430 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Philip Warner <pjw@rhyme.com.au> writes:

I take it that there is no chance of a compile-time or runtime option to
disallow this kind of thing in all cases?

Define "all cases" ... also, just how strict do you want to be?
Disallow use of *any* Postgres feature that is not in SQL92 (that
would include all user datatypes and functions, for example)?

Postgres was never designed as an SQL compatibility checker, and
I doubt that it'd be cost-effective to try to turn it into one.
A standalone tool might be a better approach.

Perhaps there is someone out there who wants this feature badly
enough to do the legwork, but I'm not him ;-)

regards, tom lane

Philip Warner <pjw@rhyme.com.au> writes:

I take it that there is no chance of a compile-time or runtime option to
disallow this kind of thing in all cases?

Define "all cases" ... also, just how strict do you want to be?
Disallow use of *any* Postgres feature that is not in SQL92 (that
would include all user datatypes and functions, for example)?

Postgres was never designed as an SQL compatibility checker, and
I doubt that it'd be cost-effective to try to turn it into one.
A standalone tool might be a better approach.

Perhaps there is someone out there who wants this feature badly
enough to do the legwork, but I'm not him ;-)

Agreed. Seems the current warning level as configured is in the middle
of people's expections on this issue.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

At 11:44 5/06/00 -0400, Tom Lane wrote:

Philip Warner <pjw@rhyme.com.au> writes:

I take it that there is no chance of a compile-time or runtime option to
disallow this kind of thing in all cases?

Define "all cases" ... also, just how strict do you want to be?
Disallow use of *any* Postgres feature that is not in SQL92 (that
would include all user datatypes and functions, for example)?

Sorry, I should have been a bit more specific! I would like some kind of
option to disable all cases of adding tables to FROM clauses by
implication. The main issue I have with this feature is it is more likely
to be used (by me) by accident (as a result of a typo), and consequently
introduce very strange results.

I am unaware (yet? ;-}) of any other non-SQL features in PostgreSQL that
are likely to cause me the same level of concern. Adding tables to a query
seems very dangerous: some people might, for example, expect an automatic
natural join on primary/foreign keys if you add a table.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.C.N. 008 659 498) | /(@) ______---_
Tel: +61-500 83 82 81 | _________ \
Fax: +61-500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Bruce Momjian writes:

I see the notice as "Hey, you probably did something you didn't want
to do".

Again, "probably" means you're not sure, so you leave it up to the user to
turn it on or off.

--
Peter Eisentraut Sernanders v�g 10:115
peter_e@gmx.net 75262 Uppsala
http://yi.org/peter-e/ Sweden

Zeugswetter Andreas SB writes:

Yes, only Bruce and I are of the opinion that it *is* an Error, and I
guess we want some consensus.

I agree that it is an error.

The notice is imho of the sort: notice this syntax is going to be
disallowed soon.

If you can guarantee that each user will only see the notice once, then
okay. :)

--
Peter Eisentraut Sernanders v�g 10:115
peter_e@gmx.net 75262 Uppsala
http://yi.org/peter-e/ Sweden

[ Charset ISO-8859-1 unsupported, converting... ]

Zeugswetter Andreas SB writes:

Yes, only Bruce and I are of the opinion that it *is* an Error, and I
guess we want some consensus.

I agree that it is an error.

The notice is imho of the sort: notice this syntax is going to be
disallowed soon.

If you can guarantee that each user will only see the notice once, then
okay. :)

There is no sense that this is a warning about the syntax changing at
some point. It is to warn queries that are probably broken.

Seems if they already have a FROM clause, there is no purpose for some
tables being in the FROM clause, and others begin auto-created. In
other cases, it issues no warning.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026