[HACKERS] [PATCH] parser: optionally warn about missing AS for column and table aliases
I wrote the attached patch to optionally emit warnings when column or table
aliases are used without the AS keyword after errors caused by typos in
statements turning unintended things into aliases came up twice this week.
First in a discussion with a colleague who was surprised by a 1 row result
for the query 'SELECT COUNT(*) files' and again in the pl/pgsql 2 thread
as plpgsql currently doesn't throw an error if there are more result columns
than output columns (SELECT a b INTO f1, f2).
The patch is still missing documentation and it needs another patch to
modify all the statements in psql co to use AS so you can use things like
\d and tab-completion without triggering the warnings. I can implement
those changes if others think this patch makes sense.
/ Oskari
From 478e694e5281a3bf91e44177ce925e6625cb44a5 Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa o...@ohmu.fi
Date: Fri, 5 Sep 2014 22:31:22 +0300
Subject: [PATCH] parser: optionally warn about missing AS for column and table
aliases
Add a new GUC missing_as_warning (defaults to false, the previous
behavior) to raise a WARNING when a column or table alias is used without
the AS keyword.
This allows catching some types of errors where another keyword or a comma
was missing and a label is being used as an alias instead of something else,
for example cases like:
SELECT COUNT(*) files;
SELECT * FROM files users;
SELECT path size FROM files INTO f_path, f_size;
---
src/backend/parser/gram.y| 24 +
src/backend/utils/misc/guc.c | 11 +++
src/include/parser/parser.h | 2 +
src/test/regress/expected/select.out | 170 +++
src/test/regress/sql/select.sql | 47 ++
5 files changed, 254 insertions(+)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b46dd7b..06a71dd 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -65,6 +65,10 @@
#include utils/xml.h
+/* GUCs */
+bool missing_as_warning = false;
+
+
/*
* Location tracking support --- simpler than bison's default, since we only
* want to track the start position not the end position of each nonterminal.
@@ -10119,12 +10123,20 @@ alias_clause:
}
| ColId '(' name_list ')'
{
+ if (missing_as_warning)
+ ereport(WARNING,
+ (errmsg(alias
without explicit AS and missing_as_warning enabled),
+
parser_errposition(@3)));
$$ = makeNode(Alias);
$$-aliasname = $1;
$$-colnames = $3;
}
| ColId
{
+ if (missing_as_warning)
+ ereport(WARNING,
+ (errmsg(alias
without explicit AS and missing_as_warning enabled),
+
parser_errposition(@1)));
$$ = makeNode(Alias);
$$-aliasname = $1;
}
@@ -10156,6 +10168,10 @@ func_alias_clause:
| ColId '(' TableFuncElementList ')'
{
Alias *a = makeNode(Alias);
+ if (missing_as_warning)
+ ereport(WARNING,
+ (errmsg(alias
without explicit AS and missing_as_warning enabled),
+
parser_errposition(@1)));
a-aliasname = $1;
$$ = list_make2(a, $3);
}
@@ -10244,6 +10260,10 @@ relation_expr_opt_alias: relation_expr
%prec UMINUS
| relation_expr ColId
{
Alias *alias = makeNode(Alias);
+ if (missing_as_warning)
+ ereport(WARNING,
+ (errmsg(alias
without explicit AS and missing_as_warning enabled),
+
parser_errposition(@2)));
alias-aliasname = $2;
$1-alias = alias;
$$ = $1;
@@ -12577,6
Re: [HACKERS] [PATCH] parser: optionally warn about missing AS for column and table aliases
On 2014-09-05 22:38, Oskari Saarenmaa wrote: I wrote the attached patch to optionally emit warnings when column or table aliases are used without the AS keyword after errors caused by typos in statements turning unintended things into aliases came up twice this week. First in a discussion with a colleague who was surprised by a 1 row result for the query 'SELECT COUNT(*) files' and again in the pl/pgsql 2 thread as plpgsql currently doesn't throw an error if there are more result columns than output columns (SELECT a b INTO f1, f2). The patch is still missing documentation and it needs another patch to modify all the statements in psql co to use AS so you can use things like \d and tab-completion without triggering the warnings. I can implement those changes if others think this patch makes sense. I think this is only problematic for column aliases. I wouldn't want to put these two to be put into the same category, as I always omit the AS keyword for tables aliases (and will continue to do so), but never omit it for column aliases. .marko -- 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] [PATCH] parser: optionally warn about missing AS for column and table aliases
05.09.2014 23:45, Marko Tiikkaja kirjoitti: On 2014-09-05 22:38, Oskari Saarenmaa wrote: I wrote the attached patch to optionally emit warnings when column or table aliases are used without the AS keyword after errors caused by typos in statements turning unintended things into aliases came up twice this week. First in a discussion with a colleague who was surprised by a 1 row result for the query 'SELECT COUNT(*) files' and again in the pl/pgsql 2 thread as plpgsql currently doesn't throw an error if there are more result columns than output columns (SELECT a b INTO f1, f2). The patch is still missing documentation and it needs another patch to modify all the statements in psql co to use AS so you can use things like \d and tab-completion without triggering the warnings. I can implement those changes if others think this patch makes sense. I think this is only problematic for column aliases. I wouldn't want to put these two to be put into the same category, as I always omit the AS keyword for tables aliases (and will continue to do so), but never omit it for column aliases. I prefer using AS for both, but I can see the case for requiring AS in table aliases being a lot weaker. Not emitting warnings for table aliases would also reduce the changes required in psql co as they seem to be using aliases mostly (only?) for tables. What'd be a good name for the GUC controlling this, missing_column_as_warning? / Oskari -- 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] [PATCH] parser: optionally warn about missing AS for column and table aliases
On 2014-09-05 22:56, Oskari Saarenmaa wrote: What'd be a good name for the GUC controlling this, missing_column_as_warning? I don't know about others, but I've previously had the idea of having more warnings like this (my go-to example is DELETE FROM foo WHERE bar IN (SELECT bar FROM barlesstable); where barlesstable doesn't have a column bar). Perhaps it would be better to have a guc with a list of warnings, similarly to what we did for plpgsql.extra_warnings? Now that I start thinking about it, more ideas for such warnings start springing up.. .marko -- 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] [PATCH] parser: optionally warn about missing AS for column and table aliases
2014-09-05 23:06 GMT+02:00 Marko Tiikkaja ma...@joh.to: On 2014-09-05 22:56, Oskari Saarenmaa wrote: What'd be a good name for the GUC controlling this, missing_column_as_warning? I don't know about others, but I've previously had the idea of having more warnings like this (my go-to example is DELETE FROM foo WHERE bar IN (SELECT bar FROM barlesstable); where barlesstable doesn't have a column bar). Perhaps it would be better to have a guc with a list of warnings, similarly to what we did for plpgsql.extra_warnings? Now that I start thinking about it, more ideas for such warnings start springing up.. I had same idea - maybe some general mode prune human errors mode Pavel .marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
