Review: query result history in psql

Поиск
Список
Период
Сортировка
От ian link
Тема Review: query result history in psql
Дата
Msg-id CAOOwM5+bhSnxTyEs=k_UdR2xROcfRPOxHQOfqP60+TT_pahwcA@mail.gmail.com
обсуждение исходный текст
Ответы Re: Review: query result history in psql  (Maciej Gajewski <maciej.gajewski0@gmail.com>)
Список pgsql-hackers
Hi Maciej,

I've been reviewing your patch for the ongoing commitfest. First let me say that I think it's a great idea and provides some very useful functionality.

However, there are a few minor problems. There were a few english/grammatical mistakes that I went ahead and fixed. Additionally, I think some of the string manipulation might be placed outside of the main ans.c file. I don't know if there's a better place for 'EscapeForCopy' and 'GetEscapedLen'. Not really a big deal, just an organizational idea. I also changed 'EscapeForCopy' to 'EscapeAndCopy'. I think that better describes the functionality. 'EscapeForCopy' kind of implies that another function is needed to copy the string. 

What does 'ans' stand for? I am not sure how it relates to the concept of a query history. It didn't stop my understanding of the code, but I don't know if a user will immediately know the meaning.

Probably the biggest problem is that the query history list is missing a maximum size variable. I think this could be valuable for preventing users from shooting themselves in the foot. If the user is running large queries, they might accidentally store too much data. This probably somewhat of an edge-case but I believe it is worth considering. We could provide a sensible default limit (10 queries?) and also allow the user to change it. 

Finally, is it worth resetting the query history every time a user reconnects to the database? I can see how this might interrupt a user's workflow. If the user suddenly disconnects (network connection interrupted, etc) then they would lose their history. I think this is definitely up for debate. It would add more management overhead (psql options etc) and might just be unnecessary. However, with a sane limit to the size of the query history, I don't know if there would be too many drawbacks from a storage perspective.

Those issues aside - I think it's a great feature! I can add the grammatical fixes I made whenever the final patch is ready. Or earlier, whatever works for you. Also, this is my first time reviewing a patch, so please let me know if I can improve on anything. Thanks!

Ian Link

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

Предыдущее
От: Jeff Janes
Дата:
Сообщение: Re: XLogInsert scaling, revisited
Следующее
От: Antonin Houska
Дата:
Сообщение: Re: LATERAL quals revisited