Thread: Re: [PATCHES] Warning for missing createlang
Bruce Momjian wrote: >Bruce Momjian wrote: > > >>I have written a patch to issue an hint if someone tries to create a >>function in a language that isn't loaded into the database: >> >> test=> CREATE FUNCTION xx() RETURNS INT AS ' >> test'> select 1' >> test-> LANGUAGE 'plpgsql'; >> ERROR: language "plpgsql" does not exist >> HINT: Perhaps you need to use 'createlang' to load the language into >> the database. >> >>I know Peter didn't like this idea in the past, but we are getting too >>many people who forget createlang, and with our new HINT tags, it seems >>appropriate. >> >> > >OK, Peter and Tom don't like it. :-( > >How about this, that also suggests you mistyped the name: > > > >> HINT: Perhaps you need to use 'createlang' to load the language into >> the database, or you mistyped the language name. >> >> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > Why not list out the languages we *do* know about, and tell them it's not in the list? Or is that too much work? andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> Bruce Momjian wrote:
>>> HINT: Perhaps you need to use 'createlang' to load the language into
>>> the database, or you mistyped the language name.
> Why not list out the languages we *do* know about, and tell them it's
> not in the list? Or is that too much work?
Seems like it would clutter the error message without really addressing
Bruce's concern. I doubt that seeing the list of available languages
would do much to jog a newbie's memory about needing to run createlang.
We could answer my objection about the hint popping out on misspelled
language names if the code were to arrange to put out the hint only when
the language name is one of "plpgsql", "pltcl", "pltclu", etc. This
would have to use a hard-coded list of loadable language names, since
by definition looking in pg_language won't help. It would be enough of
a maintenance PITA that I don't especially want to do it ... but it
would ensure that the hint is likely to be relevant.
regards, tom lane
On Fri, 2003-09-05 at 09:52, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > > Bruce Momjian wrote: > >>> HINT: Perhaps you need to use 'createlang' to load the language into > >>> the database, or you mistyped the language name. > > > Why not list out the languages we *do* know about, and tell them it's > > not in the list? Or is that too much work? > > Seems like it would clutter the error message without really addressing > Bruce's concern. I doubt that seeing the list of available languages > would do much to jog a newbie's memory about needing to run createlang. Ok, it's a hint right? Step 1 is to merge the fuzzystrmatch contrib module into main line code. Step 2 is to use that code to determine a close match. CREATE FUNCTION .... LANGUAGE 'plgsql'; System notices the error, and looks for languages in pg_language that have similar names to 'plgsql', finds 'plpgsql'. ERROR: Language 'plgsql' not found HINT: Perhaps you intended to use the language 'plpgsql? Lots of work (both to implement and maintain) but it would be a neat trick.
Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > Bruce Momjian wrote:
> >>> HINT: Perhaps you need to use 'createlang' to load the language into
> >>> the database, or you mistyped the language name.
>
> > Why not list out the languages we *do* know about, and tell them it's
> > not in the list? Or is that too much work?
>
> Seems like it would clutter the error message without really addressing
> Bruce's concern. I doubt that seeing the list of available languages
> would do much to jog a newbie's memory about needing to run createlang.
>
> We could answer my objection about the hint popping out on misspelled
> language names if the code were to arrange to put out the hint only when
> the language name is one of "plpgsql", "pltcl", "pltclu", etc. This
> would have to use a hard-coded list of loadable language names, since
> by definition looking in pg_language won't help. It would be enough of
> a maintenance PITA that I don't especially want to do it ... but it
> would ensure that the hint is likely to be relevant.
OK, new output is:
test=> create function xx() returns int as '
test'> select 1'
test-> language 'plpgsql';
ERROR: language "plpgsql" does not exist
HINT: You need to use 'createlang' to load the language into the database.
test=> create function xx() returns int as '
test'> select 1'
test-> language 'XXplpgsql';
ERROR: language "xxplpgsql" does not exist
Patch attached.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Index: src/backend/commands/functioncmds.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/commands/functioncmds.c,v
retrieving revision 1.33
diff -c -c -r1.33 functioncmds.c
*** src/backend/commands/functioncmds.c 4 Aug 2003 02:39:58 -0000 1.33
--- src/backend/commands/functioncmds.c 5 Sep 2003 15:56:57 -0000
***************
*** 435,444 ****
PointerGetDatum(languageName),
0, 0, 0);
if (!HeapTupleIsValid(languageTuple))
! ereport(ERROR,
! (errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("language \"%s\" does not exist", languageName)));
!
languageOid = HeapTupleGetOid(languageTuple);
languageStruct = (Form_pg_language) GETSTRUCT(languageTuple);
--- 435,458 ----
PointerGetDatum(languageName),
0, 0, 0);
if (!HeapTupleIsValid(languageTuple))
! {
! /* Add any new languages to this list to invoke the hint. */
! if (strcmp(languageName, "plperl") != 0 &&
! strcmp(languageName, "plpgsql") != 0 &&
! strcmp(languageName, "plpython") != 0 &&
! strcmp(languageName, "plr") != 0 &&
! strcmp(languageName, "plsh") != 0 &&
! strcmp(languageName, "pltcl") != 0)
! ereport(ERROR,
! (errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("language \"%s\" does not exist", languageName)));
! else
! ereport(ERROR,
! (errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("language \"%s\" does not exist", languageName),
! errhint("You need to use 'createlang' to load the language into the database.")));
! }
!
languageOid = HeapTupleGetOid(languageTuple);
languageStruct = (Form_pg_language) GETSTRUCT(languageTuple);
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> We could answer my objection about the hint popping out on misspelled
>> language names if the code were to arrange to put out the hint only when
>> the language name is one of "plpgsql", "pltcl", "pltclu", etc. This
>> would have to use a hard-coded list of loadable language names, since
>> by definition looking in pg_language won't help. It would be enough of
>> a maintenance PITA that I don't especially want to do it ... but it
>> would ensure that the hint is likely to be relevant.
> OK, new output is:
You forgot pltclu, and I believe plpython is now called plpythonu, and
I'm not sure whether there's a plperlu, and if you're going to include
outside-the-distro languages then I am pretty sure there's a plruby.
See what I mean about the maintenance headache this will cause?
BTW, duplicating the ereport is no fun. I'd suggest the coding style
used in some other places, with errhint called in a conditional
expression:
ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("language
\"%s\"does not exist", languageName), known_language(languageName) ? errhint("You need to
use'createlang' to load the language into the database.") : 0));
where known_language() is a little subroutine that has the strcmp()s.
regards, tom lane
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >> We could answer my objection about the hint popping out on misspelled
> >> language names if the code were to arrange to put out the hint only when
> >> the language name is one of "plpgsql", "pltcl", "pltclu", etc. This
> >> would have to use a hard-coded list of loadable language names, since
> >> by definition looking in pg_language won't help. It would be enough of
> >> a maintenance PITA that I don't especially want to do it ... but it
> >> would ensure that the hint is likely to be relevant.
>
> > OK, new output is:
>
> You forgot pltclu, and I believe plpython is now called plpythonu, and
> I'm not sure whether there's a plperlu, and if you're going to include
> outside-the-distro languages then I am pretty sure there's a plruby.
> See what I mean about the maintenance headache this will cause?
I don't mind the maintenance. I just want people to stop getting stuck
creating plpsql functions. Frankly, I don't care if we only test for
plpgsql (the most common case). It doesn't have to be perfect --- it
only prints a HINT. I willing to put some imperfect code in there to
improve usability.
> BTW, duplicating the ereport is no fun. I'd suggest the coding style
> used in some other places, with errhint called in a conditional
> expression:
>
> ereport(ERROR,
> (errcode(ERRCODE_UNDEFINED_OBJECT),
> errmsg("language \"%s\" does not exist", languageName),
> known_language(languageName) ?
> errhint("You need to use 'createlang' to load the language into the database.") : 0));
>
> where known_language() is a little subroutine that has the strcmp()s.
Here is the new output:
test=> create function xx() returns int as '
test'> select 1'
test-> language 'XXplpgsql';
ERROR: language "xxplpgsql" does not exist
test=> create function xx() returns int as '
test'> select 1'
test-> language 'plpgsql';
ERROR: language "plpgsql" does not exist
HINT: You need to use 'createlang' to load the language into the
database.
Why does the ': 0' work? I didn't figure that would work, but it does.
Patch attached.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Index: src/backend/commands/functioncmds.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/commands/functioncmds.c,v
retrieving revision 1.33
diff -c -c -r1.33 functioncmds.c
*** src/backend/commands/functioncmds.c 4 Aug 2003 02:39:58 -0000 1.33
--- src/backend/commands/functioncmds.c 5 Sep 2003 17:08:48 -0000
***************
*** 435,444 ****
PointerGetDatum(languageName),
0, 0, 0);
if (!HeapTupleIsValid(languageTuple))
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("language \"%s\" does not exist", languageName)));
!
languageOid = HeapTupleGetOid(languageTuple);
languageStruct = (Form_pg_language) GETSTRUCT(languageTuple);
--- 435,456 ----
PointerGetDatum(languageName),
0, 0, 0);
if (!HeapTupleIsValid(languageTuple))
+ /* Add any new languages to this list to invoke the hint. */
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("language \"%s\" does not exist", languageName),
! (strcmp(languageName, "plperl") == 0 ||
! strcmp(languageName, "plperlu") == 0 ||
! strcmp(languageName, "plpgsql") == 0 ||
! strcmp(languageName, "plpython") == 0 ||
! strcmp(languageName, "plpythonu") == 0 ||
! strcmp(languageName, "plr") == 0 ||
! strcmp(languageName, "plruby") == 0 ||
! strcmp(languageName, "plsh") == 0 ||
! strcmp(languageName, "pltcl") == 0 ||
! strcmp(languageName, "pltclu") == 0) ?
! errhint("You need to use 'createlang' to load the language into the database.") : 0));
!
languageOid = HeapTupleGetOid(languageTuple);
languageStruct = (Form_pg_language) GETSTRUCT(languageTuple);
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> BTW, duplicating the ereport is no fun. I'd suggest the coding style
>> used in some other places, with errhint called in a conditional
>> expression:
> Why does the ': 0' work? I didn't figure that would work, but it does.
The return values of the errxxx() subfunctions don't matter (they all
just return zero anyway). What they do is stuff their arguments into
a behind-the-scenes data structure that ereport will use when control
finally gets to it. ereport is declared as taking a "..." argument
list, but it makes no attempt to actually look at what was passed as
its arguments. So we don't really care what happens when the
?-expression test fails, as long as errhint() doesn't execute. A
constant zero is about the minimum thing we can put in the else-part
to keep the compiler happy.
regards, tom lane
Bruce Momjian writes: > I don't mind the maintenance. I just want people to stop getting stuck > creating plpsql functions. Then put plpgsql in the default installation. -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut wrote: > Bruce Momjian writes: > > > I don't mind the maintenance. I just want people to stop getting stuck > > creating plpsql functions. > > Then put plpgsql in the default installation. Fine with me. I thought others didn't want it. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
--On Friday, September 05, 2003 22:37:09 +0200 Peter Eisentraut <peter_e@gmx.net> wrote: > Bruce Momjian writes: > >> I don't mind the maintenance. I just want people to stop getting stuck >> creating plpsql functions. > > Then put plpgsql in the default installation. Why don't we do that now? It would seem to me to be a good thing(tm) to have it in. LER -- Larry Rosenman http://www.lerctr.org/~ler Phone: +1 972-414-9812 E-Mail: ler@lerctr.org US Mail: 1905 Steamboat Springs Drive, Garland, TX 75044-6749
On Fri, 2003-09-05 at 16:54, Bruce Momjian wrote: > Peter Eisentraut wrote: > > Bruce Momjian writes: > > > > > I don't mind the maintenance. I just want people to stop getting stuck > > > creating plpsql functions. > > > > Then put plpgsql in the default installation. > > Fine with me. I thought others didn't want it.