Thread: Comments with embedded single quotes
/* log will probably just be a table for Saruman's use only. */ Are single quotation marks not allowed in comments? test2=# /* John's cat is fat. */ test2'# test2'# '*/ test2-# ; ERROR: Unterminated quoted string test2=# I have the SQL + comments for creating my database in files that are just imported in psql. curious, R.
> /* log will probably just be a table for Saruman's use only. */ > > Are single quotation marks not allowed in comments? > > test2=# /* John's cat is fat. */ > test2'# > test2'# '*/ > test2-# ; > ERROR: Unterminated quoted string > test2=# > > I have the SQL + comments for creating my database in files that are just > imported in psql. Yikes, bug alert. I will take a look at it, if Peter doesn't beat me to it. -- 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
Richard Harvey Chapman <hchapman@3gfp.com> writes:
> Are single quotation marks not allowed in comments?
> test2=# /* John's cat is fat. */
> test2'#
> test2'# '*/
> test2-# ;
> ERROR: Unterminated quoted string
> test2=#
They are, but it looks like psql's primitive parser is confused here.
What the backend sees when this is sent is
/* comment */
'*/
and it quite properly complains that the string starting '*/ is not
terminated. But it looks like psql mistakenly thinks that ' nests
inside /* ... */:
regression=# /*aaa
regression*# 'sss
regression'# ddd
regression'# */
regression'# 'sss
regression*# */
regression-#
Notice the pattern of the 'state' markers in the prompts. It seems
to get the reverse case correct though:
regression-# 'foo
regression'# /*bar
regression'# '
regression-#
Over to you, Peter...
regards, tom lane
> Richard Harvey Chapman <hchapman@3gfp.com> writes: > > Are single quotation marks not allowed in comments? > > > test2=# /* John's cat is fat. */ > > test2'# > > test2'# '*/ > > test2-# ; > > ERROR: Unterminated quoted string > > test2=# > > They are, but it looks like psql's primitive parser is confused here. > What the backend sees when this is sent is > /* comment */ > > '*/ > > and it quite properly complains that the string starting '*/ is not > terminated. But it looks like psql mistakenly thinks that ' nests > inside /* ... */: > > regression=# /*aaa > regression*# 'sss > regression'# ddd > regression'# */ > regression'# 'sss > regression*# */ > regression-# > > Notice the pattern of the 'state' markers in the prompts. It seems > to get the reverse case correct though: > > regression-# 'foo > regression'# /*bar > regression'# ' > regression-# > > Over to you, Peter... OK, here is the patch. The problem was that quotes were being checked before checking if we were in a comment. Patch applied to current tree only. -- 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 ? config.log ? config.cache ? config.status ? GNUmakefile ? src/GNUmakefile ? src/Makefile.global ? src/log ? src/Makefile.custom ? src/crtags ? src/backend/postgres ? src/backend/catalog/genbki.sh ? src/backend/catalog/global1.bki.source ? src/backend/catalog/global1.description ? src/backend/catalog/local1_template1.bki.source ? src/backend/catalog/local1_template1.description ? src/backend/port/Makefile ? src/backend/utils/Gen_fmgrtab.sh ? src/bin/initdb/initdb ? src/bin/initlocation/initlocation ? src/bin/ipcclean/ipcclean ? src/bin/pg_ctl/pg_ctl ? src/bin/pg_dump/Makefile ? src/bin/pg_dump/pg_dump ? src/bin/pg_id/pg_id ? src/bin/pg_passwd/pg_passwd ? src/bin/pg_version/Makefile ? src/bin/pg_version/pg_version ? src/bin/pgaccess/pgaccess ? src/bin/pgtclsh/mkMakefile.tcldefs.sh ? src/bin/pgtclsh/mkMakefile.tkdefs.sh ? src/bin/pgtclsh/pgtclsh ? src/bin/pgtclsh/Makefile.tkdefs ? src/bin/pgtclsh/Makefile.tcldefs ? src/bin/pgtclsh/pgtksh ? src/bin/psql/Makefile ? src/bin/psql/psql ? src/bin/scripts/createlang ? src/include/version.h ? src/include/config.h ? src/include/parser/parse.h ? src/include/utils/fmgroids.h ? src/interfaces/Makefile ? src/interfaces/ecpg/lib/Makefile ? src/interfaces/ecpg/lib/libecpg.so.3.1.1 ? src/interfaces/ecpg/preproc/Makefile ? src/interfaces/ecpg/preproc/ecpg ? src/interfaces/libpgeasy/Makefile ? src/interfaces/libpgeasy/libpgeasy.so.2.1 ? src/interfaces/libpgtcl/Makefile ? src/interfaces/libpgtcl/libpgtcl.so.2.1 ? src/interfaces/libpq/Makefile ? src/interfaces/libpq/libpq.so.2.1 ? src/interfaces/libpq++/Makefile ? src/interfaces/odbc/GNUmakefile ? src/interfaces/perl5/GNUmakefile ? src/interfaces/python/GNUmakefile ? src/pl/Makefile ? src/pl/plperl/GNUmakefile ? src/pl/plpgsql/src/Makefile ? src/pl/plpgsql/src/mklang.sql ? src/pl/plpgsql/src/libplpgsql.so.1.0 ? src/pl/tcl/mkMakefile.tcldefs.sh ? src/pl/tcl/Makefile.tcldefs ? src/test/regress/GNUmakefile Index: src/bin/psql/mainloop.c =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/bin/psql/mainloop.c,v retrieving revision 1.30 diff -c -r1.30 mainloop.c *** src/bin/psql/mainloop.c 2000/05/12 16:13:44 1.30 --- src/bin/psql/mainloop.c 2000/06/29 16:27:00 *************** *** 18,26 **** #ifndef WIN32 #include <setjmp.h> - sigjmp_buf main_loop_jmp; - #endif --- 18,24 ---- *************** *** 298,315 **** bslash_count = 0; rescan: ! /* in quote? */ ! if (in_quote) { ! /* end of quote */ ! if (line[i] == in_quote && bslash_count % 2 == 0) ! in_quote = '\0'; } - /* start of quote */ - else if (!was_bslash && (line[i] == '\'' || line[i] == '"')) - in_quote = line[i]; - /* in extended comment? */ else if (xcomment) { --- 296,308 ---- bslash_count = 0; rescan: ! /* start of extended comment? */ ! if (line[i] == '/' && line[i + thislen] == '*') { ! xcomment = true; ! ADVANCE_1; } /* in extended comment? */ else if (xcomment) { *************** *** 320,338 **** } } - /* start of extended comment? */ - else if (line[i] == '/' && line[i + thislen] == '*') - { - xcomment = true; - ADVANCE_1; - } - /* single-line comment? truncate line */ else if (line[i] == '-' && line[i + thislen] == '-') { line[i] = '\0'; /* remove comment */ break; } /* count nested parentheses */ else if (line[i] == '(') --- 313,337 ---- } } /* single-line comment? truncate line */ else if (line[i] == '-' && line[i + thislen] == '-') { line[i] = '\0'; /* remove comment */ break; } + + /* in quote? */ + else if (in_quote) + { + /* end of quote */ + if (line[i] == in_quote && bslash_count % 2 == 0) + in_quote = '\0'; + } + + /* start of quote */ + else if (!was_bslash && + (line[i] == '\'' || line[i] == '"')) + in_quote = line[i]; /* count nested parentheses */ else if (line[i] == '(')
Bruce Momjian writes: > OK, here is the patch. The problem was that quotes were being checked > before checking if we were in a comment. Patch applied to current tree > only. Nope, now we have the inverse problem. Try SELECT 'aa--bb'; The timestamp test is now failing because of this. -- 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... ] > Bruce Momjian writes: > > > OK, here is the patch. The problem was that quotes were being checked > > before checking if we were in a comment. Patch applied to current tree > > only. > > Nope, now we have the inverse problem. Try > > SELECT 'aa--bb'; > > The timestamp test is now failing because of this. OK, new patch. I also renamed xcomment to in_xcomment. The important point, now commented, is that we have to test if we are in quote or an xcomment before testing for the start of quotes or xcomments. -- 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 ? config.log ? config.cache ? config.status ? GNUmakefile ? src/GNUmakefile ? src/Makefile.global ? src/log ? src/Makefile.custom ? src/crtags ? src/backend/postgres ? src/backend/catalog/genbki.sh ? src/backend/catalog/global1.bki.source ? src/backend/catalog/global1.description ? src/backend/catalog/local1_template1.bki.source ? src/backend/catalog/local1_template1.description ? src/backend/port/Makefile ? src/backend/utils/Gen_fmgrtab.sh ? src/bin/initdb/initdb ? src/bin/initlocation/initlocation ? src/bin/ipcclean/ipcclean ? src/bin/pg_ctl/pg_ctl ? src/bin/pg_dump/Makefile ? src/bin/pg_dump/pg_dump ? src/bin/pg_id/pg_id ? src/bin/pg_passwd/pg_passwd ? src/bin/pg_version/Makefile ? src/bin/pg_version/pg_version ? src/bin/pgaccess/pgaccess ? src/bin/pgtclsh/mkMakefile.tcldefs.sh ? src/bin/pgtclsh/mkMakefile.tkdefs.sh ? src/bin/pgtclsh/pgtclsh ? src/bin/pgtclsh/Makefile.tkdefs ? src/bin/pgtclsh/Makefile.tcldefs ? src/bin/pgtclsh/pgtksh ? src/bin/psql/psql ? src/bin/psql/Makefile ? src/bin/scripts/createlang ? src/include/version.h ? src/include/config.h ? src/include/parser/parse.h ? src/include/utils/fmgroids.h ? src/interfaces/Makefile ? src/interfaces/ecpg/lib/Makefile ? src/interfaces/ecpg/lib/libecpg.so.3.1.1 ? src/interfaces/ecpg/preproc/Makefile ? src/interfaces/ecpg/preproc/ecpg ? src/interfaces/libpgeasy/Makefile ? src/interfaces/libpgeasy/libpgeasy.so.2.1 ? src/interfaces/libpgtcl/Makefile ? src/interfaces/libpgtcl/libpgtcl.so.2.1 ? src/interfaces/libpq/Makefile ? src/interfaces/libpq/libpq.so.2.1 ? src/interfaces/libpq++/Makefile ? src/interfaces/odbc/GNUmakefile ? src/interfaces/perl5/GNUmakefile ? src/interfaces/python/GNUmakefile ? src/pl/Makefile ? src/pl/plperl/GNUmakefile ? src/pl/plpgsql/src/Makefile ? src/pl/plpgsql/src/mklang.sql ? src/pl/plpgsql/src/libplpgsql.so.1.0 ? src/pl/tcl/mkMakefile.tcldefs.sh ? src/pl/tcl/Makefile.tcldefs ? src/test/regress/GNUmakefile Index: src/bin/psql/mainloop.c =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/bin/psql/mainloop.c,v retrieving revision 1.31 diff -c -r1.31 mainloop.c *** src/bin/psql/mainloop.c 2000/06/29 16:27:57 1.31 --- src/bin/psql/mainloop.c 2000/06/30 18:02:06 *************** *** 44,50 **** bool success; volatile char in_quote; /* == 0 for no in_quote */ ! volatile bool xcomment; /* in extended comment */ volatile int paren_level; unsigned int query_start; volatile int count_eof = 0; --- 44,50 ---- bool success; volatile char in_quote; /* == 0 for no in_quote */ ! volatile bool in_xcomment; /* in extended comment */ volatile int paren_level; unsigned int query_start; volatile int count_eof = 0; *************** *** 80,86 **** exit(EXIT_FAILURE); } ! xcomment = false; in_quote = 0; paren_level = 0; slashCmdStatus = CMD_UNKNOWN; /* set default */ --- 80,86 ---- exit(EXIT_FAILURE); } ! in_xcomment = false; in_quote = 0; paren_level = 0; slashCmdStatus = CMD_UNKNOWN; /* set default */ *************** *** 123,129 **** resetPQExpBuffer(query_buf); /* reset parsing state */ ! xcomment = false; in_quote = 0; paren_level = 0; count_eof = 0; --- 123,129 ---- resetPQExpBuffer(query_buf); /* reset parsing state */ ! in_xcomment = false; in_quote = 0; paren_level = 0; count_eof = 0; *************** *** 147,153 **** line = xstrdup(query_buf->data); resetPQExpBuffer(query_buf); /* reset parsing state since we are rescanning whole line */ ! xcomment = false; in_quote = 0; paren_level = 0; slashCmdStatus = CMD_UNKNOWN; --- 147,153 ---- line = xstrdup(query_buf->data); resetPQExpBuffer(query_buf); /* reset parsing state since we are rescanning whole line */ ! in_xcomment = false; in_quote = 0; paren_level = 0; slashCmdStatus = CMD_UNKNOWN; *************** *** 168,174 **** prompt_status = PROMPT_SINGLEQUOTE; else if (in_quote && in_quote == '"') prompt_status = PROMPT_DOUBLEQUOTE; ! else if (xcomment) prompt_status = PROMPT_COMMENT; else if (paren_level) prompt_status = PROMPT_PAREN; --- 168,174 ---- prompt_status = PROMPT_SINGLEQUOTE; else if (in_quote && in_quote == '"') prompt_status = PROMPT_DOUBLEQUOTE; ! else if (in_xcomment) prompt_status = PROMPT_COMMENT; else if (paren_level) prompt_status = PROMPT_PAREN; *************** *** 296,337 **** bslash_count = 0; rescan: ! /* start of extended comment? */ ! if (line[i] == '/' && line[i + thislen] == '*') { ! xcomment = true; ! ADVANCE_1; } /* in extended comment? */ ! else if (xcomment) { if (line[i] == '*' && line[i + thislen] == '/') { ! xcomment = false; ADVANCE_1; } } - - /* single-line comment? truncate line */ - else if (line[i] == '-' && line[i + thislen] == '-') - { - line[i] = '\0'; /* remove comment */ - break; - } ! /* in quote? */ ! else if (in_quote) { ! /* end of quote */ ! if (line[i] == in_quote && bslash_count % 2 == 0) ! in_quote = '\0'; } /* start of quote */ else if (!was_bslash && (line[i] == '\'' || line[i] == '"')) in_quote = line[i]; /* count nested parentheses */ else if (line[i] == '(') --- 296,345 ---- bslash_count = 0; rescan: ! /* ! * It is important to place the in_* test routines ! * before the in_* detection routines. ! * i.e. we have to test if we are in a quote before ! * testing for comments. bjm 2000-06-30 ! */ ! ! /* in quote? */ ! if (in_quote) { ! /* end of quote */ ! if (line[i] == in_quote && bslash_count % 2 == 0) ! in_quote = '\0'; } /* in extended comment? */ ! else if (in_xcomment) { if (line[i] == '*' && line[i + thislen] == '/') { ! in_xcomment = false; ADVANCE_1; } } ! /* start of extended comment? */ ! else if (line[i] == '/' && line[i + thislen] == '*') { ! in_xcomment = true; ! ADVANCE_1; } /* start of quote */ else if (!was_bslash && (line[i] == '\'' || line[i] == '"')) in_quote = line[i]; + + + /* single-line comment? truncate line */ + else if (line[i] == '-' && line[i + thislen] == '-') + { + line[i] = '\0'; /* remove comment */ + break; + } /* count nested parentheses */ else if (line[i] == '(')
OK, I feel silly having to ask this, but I'm paranoid enough that I will anyway. The company I work for is about to plop down a very large amount of money on bigger, better machines to handle the work. We're currently planning on an 8-way machine running postgres for the database work, and a group of dual CPU web servers running the CGI applications. Before we get up to our necks in hardware purchases, I'd like to have some peace of mind that the performance isn't going to decrease horribly because of the bandwidth limitations in the ethernet connections. To describe the setup, the database server will be connected via a gigabit fiber cable to a switch, with 100-megabit connections to each of the application servers. So, until we get a larger number of app servers, the limiting factor will be the 100 megabit connections to each individual server. I'd like to feel sure that transmitting the result sets over the ethernet isn't going to cause a significant decrease in response time. After working through the numbers, I don't think that it will - but I'd love to hear from those who have done similar setups. We enabled some logging in the database library that we use, and here is some information on our result sets: average result set size: 1.4 kilobytes maximum result set size: 633 kilobytes Of those results, here's a breakdown: Size Frequency ------------------- 0-8K 96% 8K-17K 2% 17K - 24K 0.02% and it goes down from there. So, it seems that it would take a very large number of transactions per second to saturate either the 100 mbit link to the servers, or the gigabit pipe to the server. Am I correct, or should I reevaluate? Any comments are welcome. steve
Since most RAID servers can't even flood a 100 mbit connection, you're more than safe with that much bandwidth if most of your traffic is going to be database related. You might want to factor in all the other network traffic that will be going over those lines though. For instance, if the app servers are going to be mounting partitions off of a RAID server on the same network, and your Internet connection comes in there too, you might start to run into problems. The database shouldn't even come close though. At 06:49 PM 6/30/00, you wrote: > OK, I feel silly having to ask this, but I'm paranoid enough that I will >anyway. > > The company I work for is about to plop down a very large amount of money >on bigger, better machines to handle the work. We're currently planning on >an 8-way machine running postgres for the database work, and a group >of dual CPU web servers running the CGI applications. > > Before we get up to our necks in hardware purchases, I'd like to have >some peace of mind that the performance isn't going to decrease horribly >because of the bandwidth limitations in the ethernet connections. > > To describe the setup, the database server will be connected via a >gigabit fiber cable to a switch, with 100-megabit connections to each of the >application servers. So, until we get a larger number of app servers, the >limiting factor will be the 100 megabit connections to each individual >server. > > I'd like to feel sure that transmitting the result sets over the ethernet >isn't going to cause a significant decrease in response time. After working >through the numbers, I don't think that it will - but I'd love to hear from >those who have done similar setups. > > We enabled some logging in the database library that we use, and here is >some information on our result sets: > >average result set size: 1.4 kilobytes >maximum result set size: 633 kilobytes > > Of those results, here's a breakdown: > >Size Frequency >------------------- >0-8K 96% >8K-17K 2% >17K - 24K 0.02% > >and it goes down from there. > > So, it seems that it would take a very large number of transactions per >second to saturate either the 100 mbit link to the servers, or the gigabit >pipe to the server. > > Am I correct, or should I reevaluate? Any comments are welcome. > >steve
> Since most RAID servers can't even flood a 100 mbit connection, you're more > than safe with that much bandwidth if most of your traffic is going to be > database related. You might want to factor in all the other network > traffic that will be going over those lines though. For instance, if the > app servers are going to be mounting partitions off of a RAID server on the > same network, and your Internet connection comes in there too, you might > start to run into problems. The database shouldn't even come close though. Thank you, I appreciate the comments. Each app server is actually going to have two 100 mbit cards, one to connect it to the RAID array, and one to connect it to the database machine. Your comments give me even more hope that this will work as well as we hope. ; ) After this is all set up, if anyone would like, I may type up an explanation of how things were done as well as costs, for those going through the same sort of growing pains. It's certainly been a lot of work for us to hammer out all of the details, hopefully that would help someone else avoid the work. steve
> After this is all set up, if anyone would like, I may type up an
> explanation of how things were done as well as costs, for those going
> through the same sort of growing pains. It's certainly been a lot of
> work for us to hammer out all of the details, hopefully that would
> help someone else avoid the work.
That would be great. It would be fun to see some performance numbers
too. And it would be a great contribution to the "user cases" that help
people see the range of applications and runtime environments which have
been successful.
- Thomas
"Steve Wolfe" <steve@iboats.com> writes:
> After this is all set up, if anyone would like, I may type up an
> explanation of how things were done as well as costs, for those going
> through the same sort of growing pains. It's certainly been a lot of work
> for us to hammer out all of the details, hopefully that would help someone
> else avoid the work.
Please do --- I think a lot of people would find it interesting.
regards, tom lane