Re: [GSoC][Patch] Automatic Mode Detection V1

On Tue, Jun 25, 2019 at 7:09 AM Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:

> Hi,
>
> On Mon, Jun 24, 2019 at 10:13 PM Yosry Muhammad <yosrym93(at)gmail(dot)com>
> 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.

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

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