public inbox for [email protected]
help / color / mirror / Atom feedFrom: Aditya Toshniwal <[email protected]>
To: Dave Page <[email protected]>
Cc: Yosry Muhammad <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [GSoC][Patch] Automatic Mode Detection V1
Date: Tue, 25 Jun 2019 17:45:46 +0530
Message-ID: <CAM9w-_kqda=eN4qcY2X9sir0KEWNN7U_3Y-2+o-GW4TCPSs0uA@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxoxd+S-qgK0v=6EeJrXNK3f5AJX3A=kuZD0K3jhYnFnPBw@mail.gmail.com>
References: <CAFSMqn_dG6Ecn2z1to5jSFndyu0Y6QQ9A+RALo0Rr2YucYRogQ@mail.gmail.com>
<CAFSMqn_n4kMF-BYHj7mVpR1oe5-ztZHV+XjVcM5Bc-Tj=srSBA@mail.gmail.com>
<CA+OCxox1FEypF6y46DkSy3gZ77j7CBRa90kacMBF+qULNCcfcw@mail.gmail.com>
<CAFSMqn--z1ro3eG0FRmLSyRYqa-CDb+H3PehMYbTd=i3jyBPtA@mail.gmail.com>
<CA+OCxoz=pZjkffHQm5GiP=n1F66K9XKqk=LTt1eO-HP0jFPHkQ@mail.gmail.com>
<CAFSMqn9So7MQ729EM0CD9CVXG6q=p77r0NvxUryFAtFqaunh-Q@mail.gmail.com>
<CA+OCxozpBQUfmBMuZ8EXN2NRM4Y5pyCmY3SOq6AnbC5VeLG70A@mail.gmail.com>
<CAM9w-_nUKgKScDtraiX+ALKDcO5GP+cA12yzNFa9PQ-CTJWs9w@mail.gmail.com>
<CAFSMqn9qsjRzyW3zO5MD2DYfdiyhsXR5J1+4GPr4TnQuEaYYDQ@mail.gmail.com>
<CAFSMqn87dZ_uKjk6UEwNd=wUM4NvWEc7d1Tqs5yj-MkaVK94Kw@mail.gmail.com>
<CAM9w-_mRyY+Ljuy0Wvq5bUVXMnqO-Tv1RYeBjFgyerkkYg6efw@mail.gmail.com>
<CAFSMqn-62TQoHT-8fKDq7cWhofQ8XtLbHQV5q++u+NQw=dXDOg@mail.gmail.com>
<CAM9w-_n-hp8XwQM2QhiMu5dLumuQxMhU+ZFzEGiA6Ws+sMedaQ@mail.gmail.com>
<CA+OCxoxd+S-qgK0v=6EeJrXNK3f5AJX3A=kuZD0K3jhYnFnPBw@mail.gmail.com>
Hi,
On Tue, Jun 25, 2019 at 4:41 PM Dave Page <[email protected]> wrote:
>
>
> On Tue, Jun 25, 2019 at 7:09 AM Aditya Toshniwal <
> [email protected]> wrote:
>
>> Hi,
>>
>> On Mon, Jun 24, 2019 at 10:13 PM Yosry Muhammad <[email protected]>
>> wrote:
>>
>>> Hi,
>>>
>>> Please find attached a patch file with the following updates (last patch
>>> + updates) attached:
>>> - Changed the color to $color-gray-lighter and added the shortcut for
>>> the new button.
>>> - Added a preferences option to enable/disable prompting on uncommited
>>> transactions on exiting.
>>> - Changed call_render_after_poll_specs test to be in sync with code
>>> changes, also fixed a mix up in the test descriptions in the same file.
>>> - Fixed a bug with a recent patch 'Allow editing of data where a primary
>>> key column includes a % sign in the value.' that occurred when the primary
>>> key was a number.
>>>
>>> - After running python and feature tests, changes were made to nearly
>>>>> all the files (git status shows modifications in a ton of files), is there
>>>>> something I have done wrong?
>>>>>
>>>> What command did you use, can you share the screenshot of the files
>>>> changed?
>>>>
>>>
>>> I tried it again after a proper test_config.json as you mentioned and
>>> everything worked fine. All tests pass for this patch except for 3 feature
>>> tests that all fail because of a TimeoutException related to selenium.
>>> Please find a log file of the feature tests attached.
>>>
>>>
>>>>
>>>> - What else is missing from this patch to make it applicable ? I would
>>>>> like to produce a release-ready patch if possible. If so, I can continue
>>>>> working on the project on following patches, I just want to know what is
>>>>> the minimum amount of work needed to make this patch release-ready
>>>>> (especially that changes are being made in the master branch that require
>>>>> me to re-edit parts of the code that I have written before to keep things
>>>>> in-sync).
>>>>>
>>>> @Dave Page is the right person to answer this.
>>>>
>>>
>>> Waiting for his reply :D
>>>
>>> - For the bug that I reported before (generated queries in Query History
>>>>> appear in a distorted way for the user), to get the actual query that is
>>>>> being executed I can use the mogirfy() function of psycopg2 but I need
>>>>> access to a cursor. I can get one directly in save_changed_data() function
>>>>> by using conn.conn.cursor() but then I would be bypassing the wrapper
>>>>> Connection class. Should I modify the wrapper Connection class and add a
>>>>> function that can provide a cursor (or a wrapper around cursor.mogrify() )?
>>>>> Thoughts?
>>>>>
>>>> Could you please share the query/screenshot ? The query history just
>>>> stores the SQL text and fetches back to show in CodeMirror. No
>>>> modifications/generation of queries is done by Query History.
>>>>
>>>>>
>>>>>
>>> By 'generated queries' I meant the querie that are generated by pgAdmin
>>> to save changes to the data grid to the database. Here is a screenshot from
>>> the released version (not the version I am working on).
>>> [image: pg-query-history-bug.png]
>>> Scenario:
>>> - Opened View Data on a table (public.kweek)
>>> - Modified a cell in a column named media_url with a primary key (id =
>>> 50) to 'new link'
>>> - Instead of showing 'new link' in the query %(media_url) is shown.
>>>
>> The update queries fired internally should not go to history. Queries
>> fired by user only should go. That's what I think.
>>
>
> The conclusion I came to in previous discussion was that both should be
> available, with a checkbox (off by default) to include the internal
> queries, which would include any BEGIN/COMMITs etc.
>
OK. Yosry, How about storing the mogirfied query in the cookie/session when
the query is executed and then modifying query history storing logic to use
it when called ? This way you will not need to change any parsing when
query history is displayed.
> --
> 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 India | Pune
"Don't Complain about Heat, Plant a TREE"
Attachments:
[image/png] pg-query-history-bug.png (34.7K, 3-pg-query-history-bug.png)
download | view image
view thread (27+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: [GSoC][Patch] Automatic Mode Detection V1
In-Reply-To: <CAM9w-_kqda=eN4qcY2X9sir0KEWNN7U_3Y-2+o-GW4TCPSs0uA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox