Re: Trailing semicolons in psql patch - Mailing list pgsql-patches

I like it.  Several people have complained because they forget and add
the semicolon.  I like that you made it per-command so commands that
need to see the semicolon see it.  I will change it slightly to use
true/false rather than 0/1.

Your patch has been added to the PostgreSQL unapplied patches list at:

    http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> How about a patch to strip semicolons in psql? I *know*
> some of you have done this before:
>
> \d mytable;
>
> (Did not find any relation named "mytable;")
>
> Since there is no reason to have a table named "mytable;"
> why not just have psql do the smart thing and silently
> strip the trailing semicolons? The attached patch only addresses
> a few backslash commands as a proof of concept, but some others
> could probably use it without harm. The patch affects:
>
> \C
> \c
> \d
> \e
> \i
> \o
> \s
> \z
>
> Greg Sabino Mullane
> greg@turnstep.com
> PGP Key: 0x14964AC8 200109271811
>
> P.S. Re: microtiming: EXPLAIN ANALYZE sounds interesting, and as
> soon as cvs is fixed so that anoncvs/postgresql works, I'll take
> a look at it. :)
>
>
> -----BEGIN PGP SIGNATURE-----
> Comment: http://www.turnstep.com/pgp.html
>
> iQA/AwUBO7OkzrybkGcUlkrIEQIPrgCgurQBAqZLt6m5JvbKOJyVIOx3ZQAAn0Wg
> KD097oaniENYAeikbSM7LdZg
> =erHC
> -----END PGP SIGNATURE-----

> *** ./src/bin/psql/command.c.orig    Tue Sep 25 13:21:50 2001
> --- ./src/bin/psql/command.c    Tue Sep 25 18:06:37 2001
> ***************
> *** 55,61 ****
>   {
>       OT_NORMAL, OT_SQLID, OT_FILEPIPE
>   };
> ! static char *scan_option(char **string, enum option_type type, char *quote);
>   static char *unescape(const unsigned char *source, size_t len);
>
>   static bool do_edit(const char *filename_arg, PQExpBuffer query_buf);
> --- 55,61 ----
>   {
>       OT_NORMAL, OT_SQLID, OT_FILEPIPE
>   };
> ! static char *scan_option(char **string, enum option_type type, char *quote, bool semicolon);
>   static char *unescape(const unsigned char *source, size_t len);
>
>   static bool do_edit(const char *filename_arg, PQExpBuffer query_buf);
> ***************
> *** 216,222 ****
>       /* \C -- override table title (formerly change HTML caption) */
>       else if (strcmp(cmd, "C") == 0)
>       {
> !         char       *opt = scan_option(&string, OT_NORMAL, NULL);
>
>           success = do_pset("title", opt, &pset.popt, quiet);
>           free(opt);
> --- 216,222 ----
>       /* \C -- override table title (formerly change HTML caption) */
>       else if (strcmp(cmd, "C") == 0)
>       {
> !         char       *opt = scan_option(&string, OT_NORMAL, NULL, 1);
>
>           success = do_pset("title", opt, &pset.popt, quiet);
>           free(opt);
> ***************
> *** 238,245 ****
>           char        opt1q,
>                       opt2q;
>
> !         opt1 = scan_option(&string, OT_NORMAL, &opt1q);
> !         opt2 = scan_option(&string, OT_NORMAL, &opt2q);
>
>           if (opt2)
>               /* gave username */
> --- 238,245 ----
>           char        opt1q,
>                       opt2q;
>
> !         opt1 = scan_option(&string, OT_NORMAL, &opt1q, 1);
> !         opt2 = scan_option(&string, OT_NORMAL, &opt2q, 1);
>
>           if (opt2)
>               /* gave username */
> ***************
> *** 273,280 ****
>       {
>           char       *name;
>           bool        show_verbose;
>
> -         name = scan_option(&string, OT_SQLID, NULL);
>           show_verbose = strchr(cmd, '+') ? true : false;
>
>           switch (cmd[1])
> --- 273,280 ----
>       {
>           char       *name;
>           bool        show_verbose;
> +         name = scan_option(&string, OT_SQLID, NULL, 1);
>
>           show_verbose = strchr(cmd, '+') ? true : false;
>
>           switch (cmd[1])
> ***************
> *** 337,343 ****
>           }
>           else
>           {
> !             fname = scan_option(&string, OT_NORMAL, NULL);
>               status = do_edit(fname, query_buf) ? CMD_NEWEDIT : CMD_ERROR;
>               free(fname);
>           }
> --- 337,343 ----
>           }
>           else
>           {
> !             fname = scan_option(&string, OT_NORMAL, NULL, 1);
>               status = do_edit(fname, query_buf) ? CMD_NEWEDIT : CMD_ERROR;
>               free(fname);
>           }
> ***************
> *** 356,363 ****
>               fout = pset.queryFout;
>           else
>               fout = stdout;
> !
> !         while ((value = scan_option(&string, OT_NORMAL, "ed)))
>           {
>               if (!quoted && strcmp(value, "-n") == 0)
>                   no_newline = true;
> --- 356,363 ----
>               fout = pset.queryFout;
>           else
>               fout = stdout;
> !
> !         while ((value = scan_option(&string, OT_NORMAL, "ed, 0)))
>           {
>               if (!quoted && strcmp(value, "-n") == 0)
>                   no_newline = true;
> ***************
> *** 378,384 ****
>       /* \encoding -- set/show client side encoding */
>       else if (strcmp(cmd, "encoding") == 0)
>       {
> !         char       *encoding = scan_option(&string, OT_NORMAL, NULL);
>
>           if (!encoding)
>               /* show encoding */
> --- 378,384 ----
>       /* \encoding -- set/show client side encoding */
>       else if (strcmp(cmd, "encoding") == 0)
>       {
> !         char       *encoding = scan_option(&string, OT_NORMAL, NULL, 0);
>
>           if (!encoding)
>               /* show encoding */
> ***************
> *** 406,412 ****
>       /* \f -- change field separator */
>       else if (strcmp(cmd, "f") == 0)
>       {
> !         char       *fname = scan_option(&string, OT_NORMAL, NULL);
>
>           success = do_pset("fieldsep", fname, &pset.popt, quiet);
>           free(fname);
> --- 406,412 ----
>       /* \f -- change field separator */
>       else if (strcmp(cmd, "f") == 0)
>       {
> !         char       *fname = scan_option(&string, OT_NORMAL, NULL, 0);
>
>           success = do_pset("fieldsep", fname, &pset.popt, quiet);
>           free(fname);
> ***************
> *** 415,421 ****
>       /* \g means send query */
>       else if (strcmp(cmd, "g") == 0)
>       {
> !         char       *fname = scan_option(&string, OT_FILEPIPE, NULL);
>
>           if (!fname)
>               pset.gfname = NULL;
> --- 415,421 ----
>       /* \g means send query */
>       else if (strcmp(cmd, "g") == 0)
>       {
> !         char       *fname = scan_option(&string, OT_FILEPIPE, NULL, 0);
>
>           if (!fname)
>               pset.gfname = NULL;
> ***************
> *** 447,453 ****
>       /* \i is include file */
>       else if (strcmp(cmd, "i") == 0 || strcmp(cmd, "include") == 0)
>       {
> !         char       *fname = scan_option(&string, OT_NORMAL, NULL);
>
>           if (!fname)
>           {
> --- 447,453 ----
>       /* \i is include file */
>       else if (strcmp(cmd, "i") == 0 || strcmp(cmd, "include") == 0)
>       {
> !         char       *fname = scan_option(&string, OT_NORMAL, NULL, 1);
>
>           if (!fname)
>           {
> ***************
> *** 475,482 ****
>           char       *opt1,
>                      *opt2;
>
> !         opt1 = scan_option(&string, OT_NORMAL, NULL);
> !         opt2 = scan_option(&string, OT_NORMAL, NULL);
>
>           if (strcmp(cmd + 3, "export") == 0)
>           {
> --- 475,482 ----
>           char       *opt1,
>                      *opt2;
>
> !         opt1 = scan_option(&string, OT_NORMAL, NULL, 1);
> !         opt2 = scan_option(&string, OT_NORMAL, NULL, 1);
>
>           if (strcmp(cmd + 3, "export") == 0)
>           {
> ***************
> *** 528,534 ****
>       /* \o -- set query output */
>       else if (strcmp(cmd, "o") == 0 || strcmp(cmd, "out") == 0)
>       {
> !         char       *fname = scan_option(&string, OT_FILEPIPE, NULL);
>
>           success = setQFout(fname);
>           free(fname);
> --- 528,534 ----
>       /* \o -- set query output */
>       else if (strcmp(cmd, "o") == 0 || strcmp(cmd, "out") == 0)
>       {
> !         char       *fname = scan_option(&string, OT_FILEPIPE, NULL, 1);
>
>           success = setQFout(fname);
>           free(fname);
> ***************
> *** 547,554 ****
>       /* \pset -- set printing parameters */
>       else if (strcmp(cmd, "pset") == 0)
>       {
> !         char       *opt0 = scan_option(&string, OT_NORMAL, NULL);
> !         char       *opt1 = scan_option(&string, OT_NORMAL, NULL);
>
>           if (!opt0)
>           {
> --- 547,554 ----
>       /* \pset -- set printing parameters */
>       else if (strcmp(cmd, "pset") == 0)
>       {
> !         char       *opt0 = scan_option(&string, OT_NORMAL, NULL, 0);
> !         char       *opt1 = scan_option(&string, OT_NORMAL, NULL, 0);
>
>           if (!opt0)
>           {
> ***************
> *** 577,583 ****
>       /* \s save history in a file or show it on the screen */
>       else if (strcmp(cmd, "s") == 0)
>       {
> !         char       *fname = scan_option(&string, OT_NORMAL, NULL);
>
>           success = saveHistory(fname ? fname : "/dev/tty");
>
> --- 577,583 ----
>       /* \s save history in a file or show it on the screen */
>       else if (strcmp(cmd, "s") == 0)
>       {
> !         char       *fname = scan_option(&string, OT_NORMAL, NULL, 1);
>
>           success = saveHistory(fname ? fname : "/dev/tty");
>
> ***************
> *** 589,595 ****
>       /* \set -- generalized set variable/option command */
>       else if (strcmp(cmd, "set") == 0)
>       {
> !         char       *opt0 = scan_option(&string, OT_NORMAL, NULL);
>
>           if (!opt0)
>           {
> --- 589,595 ----
>       /* \set -- generalized set variable/option command */
>       else if (strcmp(cmd, "set") == 0)
>       {
> !         char       *opt0 = scan_option(&string, OT_NORMAL, NULL, 0);
>
>           if (!opt0)
>           {
> ***************
> *** 614,624 ****
>               char       *newval = NULL;
>               char       *opt;
>
> !             opt = scan_option(&string, OT_NORMAL, NULL);
>               newval = xstrdup(opt ? opt : "");
>               free(opt);
>
> !             while ((opt = scan_option(&string, OT_NORMAL, NULL)))
>               {
>                   newval = realloc(newval, strlen(newval) + strlen(opt) + 1);
>                   if (!newval)
> --- 614,624 ----
>               char       *newval = NULL;
>               char       *opt;
>
> !             opt = scan_option(&string, OT_NORMAL, NULL, 0);
>               newval = xstrdup(opt ? opt : "");
>               free(opt);
>
> !             while ((opt = scan_option(&string, OT_NORMAL, NULL, 0)))
>               {
>                   newval = realloc(newval, strlen(newval) + strlen(opt) + 1);
>                   if (!newval)
> ***************
> *** 648,654 ****
>       /* \T -- define html <table ...> attributes */
>       else if (strcmp(cmd, "T") == 0)
>       {
> !         char       *value = scan_option(&string, OT_NORMAL, NULL);
>
>           success = do_pset("tableattr", value, &pset.popt, quiet);
>           free(value);
> --- 648,654 ----
>       /* \T -- define html <table ...> attributes */
>       else if (strcmp(cmd, "T") == 0)
>       {
> !         char       *value = scan_option(&string, OT_NORMAL, NULL, 0);
>
>           success = do_pset("tableattr", value, &pset.popt, quiet);
>           free(value);
> ***************
> *** 657,663 ****
>       /* \unset */
>       else if (strcmp(cmd, "unset") == 0)
>       {
> !         char       *opt = scan_option(&string, OT_NORMAL, NULL);
>
>           if (!opt)
>           {
> --- 657,663 ----
>       /* \unset */
>       else if (strcmp(cmd, "unset") == 0)
>       {
> !         char       *opt = scan_option(&string, OT_NORMAL, NULL, 0);
>
>           if (!opt)
>           {
> ***************
> *** 686,692 ****
>           }
>           else
>           {
> !             fname = scan_option(&string, OT_FILEPIPE, NULL);
>
>               if (!fname)
>               {
> --- 686,692 ----
>           }
>           else
>           {
> !             fname = scan_option(&string, OT_FILEPIPE, NULL, 1);
>
>               if (!fname)
>               {
> ***************
> *** 741,747 ****
>       /* \z -- list table rights (grant/revoke) */
>       else if (strcmp(cmd, "z") == 0)
>       {
> !         char       *opt = scan_option(&string, OT_SQLID, NULL);
>
>           success = permissionsList(opt);
>           free(opt);
> --- 741,747 ----
>       /* \z -- list table rights (grant/revoke) */
>       else if (strcmp(cmd, "z") == 0)
>       {
> !         char       *opt = scan_option(&string, OT_SQLID, NULL, 1);
>
>           success = permissionsList(opt);
>           free(opt);
> ***************
> *** 772,778 ****
>           char       *value;
>
>           fprintf(stderr, "+ optstr = |%s|\n", options_string);
> !         while ((value = scan_option(&string, OT_NORMAL, NULL)))
>           {
>               fprintf(stderr, "+ opt(%d) = |%s|\n", i++, value);
>               free(value);
> --- 772,778 ----
>           char       *value;
>
>           fprintf(stderr, "+ optstr = |%s|\n", options_string);
> !         while ((value = scan_option(&string, OT_NORMAL, NULL, 1)))
>           {
>               fprintf(stderr, "+ opt(%d) = |%s|\n", i++, value);
>               free(value);
> ***************
> *** 787,793 ****
>           status = CMD_ERROR;
>
>       /* eat the rest of the options string */
> !     while ((val = scan_option(&string, OT_NORMAL, NULL)))
>       {
>           if (status != CMD_UNKNOWN)
>               psql_error("\\%s: extra argument '%s' ignored\n", cmd, val);
> --- 787,793 ----
>           status = CMD_ERROR;
>
>       /* eat the rest of the options string */
> !     while ((val = scan_option(&string, OT_NORMAL, NULL, 0)))
>       {
>           if (status != CMD_UNKNOWN)
>               psql_error("\\%s: extra argument '%s' ignored\n", cmd, val);
> ***************
> *** 806,812 ****
>    * scan_option()
>    */
>   static char *
> ! scan_option(char **string, enum option_type type, char *quote)
>   {
>       unsigned int pos = 0;
>       char       *options_string;
> --- 806,812 ----
>    * scan_option()
>    */
>   static char *
> ! scan_option(char **string, enum option_type type, char *quote, bool semicolon)
>   {
>       unsigned int pos = 0;
>       char       *options_string;
> ***************
> *** 822,827 ****
> --- 822,835 ----
>       /* skip leading whitespace */
>       pos += strspn(options_string + pos, " \t\n\r");
>
> +
> +     /* Strip any trailing semi-colons for some types */
> +     if (semicolon) {
> +         int i;
> +         for (i = strlen(options_string)-1; i && options_string[i]==';'; i--);
> +         if (i<strlen(options_string)-1) options_string[i+1]='\0';
> +     }
> +
>       switch (options_string[pos])
>       {
>
> ***************
> *** 863,868 ****
> --- 871,877 ----
>                    * If this is expected to be an SQL identifier like option
>                    * then we strip out the double quotes
>                    */
> +
>                   if (type == OT_SQLID)
>                   {
>                       unsigned int k,
> ***************
> *** 894,900 ****
>                   *string = options_string + jj + 1;
>                   if (quote)
>                       *quote = '"';
> !
>                   return return_val;
>               }
>
> --- 903,909 ----
>                   *string = options_string + jj + 1;
>                   if (quote)
>                       *quote = '"';
> !
>                   return return_val;
>               }
>

>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  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