Re: [pgAdmin4][RM4017] Query Tool History persistence

On Wed, Mar 13, 2019 at 6:26 AM Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:

> Hi,
>
> On Tue, Mar 12, 2019 at 6:27 PM Aditya Toshniwal <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>>
>>
>> On Tue, Mar 12, 2019 at 6:14 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On Tue, Mar 12, 2019 at 12:33 PM Aditya Toshniwal <
>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Dave,
>>>>
>>>> On Tue, Mar 12, 2019 at 4:02 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> On Tue, Mar 12, 2019 at 7:00 AM Aditya Toshniwal <
>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Hackers,
>>>>>>
>>>>>> Attached is the patch to make query history persistent. The number of
>>>>>> records to persist is configurable using MAX_QUERY_HIST_STORED (config.py)
>>>>>> which is per user/server/database based (currently set to 20). Records will
>>>>>> be stored in the sqlite3 db.
>>>>>> Test cases are included.
>>>>>>
>>>>>
>>>>> Some initial review comments:
>>>>>
>>>>> - When first clicking the History tab, the top query is selected and
>>>>> details shown, *except* the SQL query in the details panel. It displays
>>>>> correctly if the query is clicked.
>>>>>
>>>>> - The date is shown in two different formats on the history panel:
>>>>> "Mar 12 2019" in the list, and "03/12/2019" on the details panel. They
>>>>> should be consistent.
>>>>>
>>>> I'll use the locale string here on both sides.
>>>>
>>>>>
>>>>> - The date in the details panel is still being shown in US format,
>>>>> which is confusing for me. Should we just use Date.toLocaleDateString()?.
>>>>>
>>>>> - Deleting a server should delete the users history for that server.
>>>>>
>>>>> - I think we should reference the database by name in the config
>>>>> database, so that if the user drops and recreates the database, they can
>>>>> retain the history.
>>>>>
>>>> I have implemented per user/database/server based. Here, user means
> pgadmin user and not database user right ? If database user, then should
> pgadmin user also be considered ?
>

Yes, pgAdmin user - though aren't they one and the same in this case?

The data should be keyed in sqlite by the pgAdmin user, server and
database. The Postgres user is part of the server definition.

>
>>>>> Thanks!
>>>>>
>>>>> --
>>>>> Dave Page
>>>>> Blog: http://pgsnake.blogspot.com
>>>>> Twitter: @pgsnake
>>>>>
>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>> The Enterprise PostgreSQL Company
>>>>>
>>>>
>>>> I have a question here. Now, the query history is loaded in view/edit
>>>> table data also, which I think should not. What do you suggest ?
>>>>
>>>
>>> Good question. I want to minimise the differences between the two modes,
>>> as eventually we want it to be seemless. Can we keep the history as it
>>> works now (i.e. no pre-loading/saving), and disable the "Copy to editor"
>>> button?
>>>
>> OK. I will make changes to remove View/Edit data query tool from history
>> persistence membership. And plus disabling the button - "Copy to editor".
>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>> --
>> Thanks and Regards,
>> Aditya Toshniwal
>> Software Engineer | EnterpriseDB Software Solutions | Pune
>> "Don't Complain about Heat, Plant a tree"
>>
>
>
> --
> Thanks and Regards,
> Aditya Toshniwal
> Software Engineer | EnterpriseDB Software Solutions | Pune
> "Don't Complain about Heat, Plant a tree"
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company