Re: add line number as prompt option to psql

Поиск
Список
Период
Сортировка
От Jeevan Chalke
Тема Re: add line number as prompt option to psql
Дата
Msg-id CAM2+6=XWdJb_Nfk8zGCaLgc2NCQgWWWmL0Zh+-DodQT58+21VQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: add line number as prompt option to psql  (Sawada Masahiko <sawada.mshk@gmail.com>)
Ответы Re: add line number as prompt option to psql  (Sawada Masahiko <sawada.mshk@gmail.com>)
Список pgsql-hackers
Hi,

Found new issues with latest patch:


Thank you for reviewing the patch with variable cases.
I have revised the patch, and attached latest patch.

> A:
> Will you please explain the idea behind these changes ?
I thought wrong about adding new to tail of query_buf.
The latest patch does not change related to them.

Thanks.
 
> B:
I added the condition of cur_line < 0.

 
A.

However, this introduced new bug. As I told, when editor number of lines reaches INT_MAX it starts giving negative number. You tried overcoming this issue by adding "< 0" check. But I guess you again fumbled in setting that correctly. You are setting it to INT_MAX - 1. This enforces each new line to show line number as INT_MAX - 1 which is incorrect.

postgres=# \set PROMPT1 '%/[%l]%R%# '
postgres[1]=# \set PROMPT2 '%/[%l]%R%# '
postgres[1]=# \e
postgres[2147483646]-# limit
postgres[2147483646]-# 1;
   relname   
--------------
 pg_statistic
(1 row)

postgres[1]=# \e
postgres[2147483646]-# =
postgres[2147483646]-# '
postgres[2147483646]'# abc
postgres[2147483646]'# '
postgres[2147483646]-# ;
 relname
---------
(0 rows)

postgres[1]=# select
relname
from
pg_class
where
relname
=
'
abc
'
;
 


Again to mimic that, I have simply intialized newline to INT_MAX - 2.
Please don't take me wrong, but it seems that your unit testing is not enough. Above issue I discovered by doing exactly same steps I did in reviewing previous patch. If you had tested your new patch with those steps I guess you have caught that yourself.


B.

+             /* Calculate the line number */
+             if (scan_result != PSCAN_INCOMPLETE)
+             {
+                 /* The one new line is always added to tail of query_buf  */
+                 newline = (newline != 0) ? (newline + 1) : 1;
+                 cur_line += newline;
+             }

Here in above code changes, in any case you are adding 1 to newline. i.e. when it is 0 you are setting it 1 (+1) and when > 0 you are setting nl + 1 (again +1).
So why can't you simply use"
    if (scan_result != PSCAN_INCOMPLETE)
        cur_line += (newline + 1);


Or better, why can't you initialize newline with 1 itself and then directly assign cur_line with newline. That will eliminate above entire code block, isn't it?
Or much better, simply get rid of newline, and use cur_line itself, will this work well for you?


C. Typos:
1.
/* Count the number of new line for calculate ofline number */

Missing space between 'of' and 'line'.
However better improve that to something like (just suggestion):
"Count the number of new lines to correctly adjust current line number"

2.
/* Avoid cur_line and newline exceeds the INT_MAX */

You are saying avoid cur_line AND newline, but there is no adjustment for newline in the code following the comment.

Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Etsuro Fujita
Дата:
Сообщение: Re: inherit support for foreign tables
Следующее
От: Simon Riggs
Дата:
Сообщение: Re: tweaking NTUP_PER_BUCKET