Thread: [RM2074][[RM2080]][pgAdmin4] handle large bytea and bytea[] data in datagrid

[RM2074][[RM2080]][pgAdmin4] handle large bytea and bytea[] data in datagrid

From
Harshal Dhumal
Date:
Hi,

Please find attached patch to handle bytea and bytea[] data in datagrid.

Now instead of showing actual data we can show placeholders like <binary data> and <binary data[]>. Also placeholders will only appear if data actually exists otherwise null will be shown.


-- 
Harshal Dhumal
Sr. Software Engineer

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

Re: [RM2074][[RM2080]][pgAdmin4] handle large bytea and bytea[] datain datagrid

From
Murtuza Zabuawala
Date:
Hi Harshal,

This broke the fetching trigger function parameters logic, take a look at raise_a_notice() function parameter in screenshot.

In my opinion we just can't just blindly convert everything to place holder, what if user wants to perform some operation on bytea column like we are doing in web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/trigger/sql/default/properties.sql to fetch these arguments?

Inline image 1

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Fri, Jul 14, 2017 at 6:43 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi,

Please find attached patch to handle bytea and bytea[] data in datagrid.

Now instead of showing actual data we can show placeholders like <binary data> and <binary data[]>. Also placeholders will only appear if data actually exists otherwise null will be shown.


-- 
Harshal Dhumal
Sr. Software Engineer

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

Hi,

Please find updated patch. Now placeholder string for bytea and bytea[] data will only appear in datagrid (view all/1000/500 rows). If user executes query using Query tool then placeholder won't appear (similar to  pgAdminIII behaviour)

-- 
Harshal Dhumal
Sr. Software Engineer

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

On Fri, Jul 14, 2017 at 7:23 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Harshal,

This broke the fetching trigger function parameters logic, take a look at raise_a_notice() function parameter in screenshot.

In my opinion we just can't just blindly convert everything to place holder, what if user wants to perform some operation on bytea column like we are doing in web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/trigger/sql/default/properties.sql to fetch these arguments?

Inline image 1

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Fri, Jul 14, 2017 at 6:43 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi,

Please find attached patch to handle bytea and bytea[] data in datagrid.

Now instead of showing actual data we can show placeholders like <binary data> and <binary data[]>. Also placeholders will only appear if data actually exists otherwise null will be shown.


-- 
Harshal Dhumal
Sr. Software Engineer

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


Attachment
Hi

On Mon, Jul 17, 2017 at 1:09 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi,

Please find updated patch. Now placeholder string for bytea and bytea[] data will only appear in datagrid (view all/1000/500 rows). If user executes query using Query tool then placeholder won't appear (similar to  pgAdminIII behaviour)

I'm getting the following error when testing this:

======================================================================
ERROR: runTest (pgadmin.feature_tests.view_data_dml_queries.CheckForViewDataTest)
Validate Insert, Update operations in View/Edit data with given test data
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py", line 104, in runTest
    self._add_row()
  File "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py", line 264, in _add_row
    self._update_cell(cell_xpath, config_data[str(idx)])
  File "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py", line 164, in _update_cell
    cell_el = self.page.find_by_xpath(xpath)
  File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py", line 148, in find_by_xpath
    return self.wait_for_element(lambda driver: driver.find_element_by_xpath(xpath))
  File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py", line 231, in wait_for_element
    return self._wait_for("element to exist", element_if_it_exists)
  File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py", line 281, in _wait_for
    "Timed out waiting for " + waiting_for_message)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/support/wait.py", line 71, in until
    value = method(self._driver)
  File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py", line 226, in element_if_it_exists
    if element.is_displayed() and element.is_enabled():
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/remote/webelement.py", line 157, in is_enabled
    return self._execute(Command.IS_ELEMENT_ENABLED)['value']
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/remote/webelement.py", line 491, in _execute
    return self._parent.execute(command, params)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/remote/webdriver.py", line 238, in execute
    self.error_handler.check_response(response)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/remote/errorhandler.py", line 193, in check_response
    raise exception_class(message, screen, stacktrace)
StaleElementReferenceException: Message: stale element reference: element is not attached to the page document
  (Session info: chrome=59.0.3071.115)
  (Driver info: chromedriver=2.29.461585 (0be2cd95f834e9ee7c46bcc7cf405b483f5ae83b),platform=Mac OS X 10.12.5 x86_64) 


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

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


On Mon, Jul 17, 2017 at 9:33 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Mon, Jul 17, 2017 at 1:09 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi,

Please find updated patch. Now placeholder string for bytea and bytea[] data will only appear in datagrid (view all/1000/500 rows). If user executes query using Query tool then placeholder won't appear (similar to  pgAdminIII behaviour)

I'm getting the following error when testing this:

I ran all feature test cases 5-6 times and each time they ran successfully.
May be this is occasional failure which we get after running test cases for many times.

Please let me know if you are getting this failure consistently at your end.

 

======================================================================
ERROR: runTest (pgadmin.feature_tests.view_data_dml_queries.CheckForViewDataTest)
Validate Insert, Update operations in View/Edit data with given test data
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py", line 104, in runTest
    self._add_row()
  File "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py", line 264, in _add_row
    self._update_cell(cell_xpath, config_data[str(idx)])
  File "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py", line 164, in _update_cell
    cell_el = self.page.find_by_xpath(xpath)
  File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py", line 148, in find_by_xpath
    return self.wait_for_element(lambda driver: driver.find_element_by_xpath(xpath))
  File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py", line 231, in wait_for_element
    return self._wait_for("element to exist", element_if_it_exists)
  File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py", line 281, in _wait_for
    "Timed out waiting for " + waiting_for_message)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/support/wait.py", line 71, in until
    value = method(self._driver)
  File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py", line 226, in element_if_it_exists
    if element.is_displayed() and element.is_enabled():
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/remote/webelement.py", line 157, in is_enabled
    return self._execute(Command.IS_ELEMENT_ENABLED)['value']
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/remote/webelement.py", line 491, in _execute
    return self._parent.execute(command, params)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/remote/webdriver.py", line 238, in execute
    self.error_handler.check_response(response)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/remote/errorhandler.py", line 193, in check_response
    raise exception_class(message, screen, stacktrace)
StaleElementReferenceException: Message: stale element reference: element is not attached to the page document
  (Session info: chrome=59.0.3071.115)
  (Driver info: chromedriver=2.29.461585 (0be2cd95f834e9ee7c46bcc7cf405b483f5ae83b),platform=Mac OS X 10.12.5 x86_64) 


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

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



On Tue, Jul 18, 2017 at 8:26 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,


On Mon, Jul 17, 2017 at 9:33 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Mon, Jul 17, 2017 at 1:09 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi,

Please find updated patch. Now placeholder string for bytea and bytea[] data will only appear in datagrid (view all/1000/500 rows). If user executes query using Query tool then placeholder won't appear (similar to  pgAdminIII behaviour)

I'm getting the following error when testing this:

I ran all feature test cases 5-6 times and each time they ran successfully.
May be this is occasional failure which we get after running test cases for many times.

Please let me know if you are getting this failure consistently at your end.

I ran it twice and it failed twice. I can't keep running tests over and over again as they take quite a while to run and I can't use my machine for anything else at the same time as occasionally the test browser will grab focus.
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

On Tue, Jul 18, 2017 at 1:24 PM, Dave Page <dpage@pgadmin.org> wrote:


On Tue, Jul 18, 2017 at 8:26 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,


On Mon, Jul 17, 2017 at 9:33 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Mon, Jul 17, 2017 at 1:09 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi,

Please find updated patch. Now placeholder string for bytea and bytea[] data will only appear in datagrid (view all/1000/500 rows). If user executes query using Query tool then placeholder won't appear (similar to  pgAdminIII behaviour)

I'm getting the following error when testing this:

I ran all feature test cases 5-6 times and each time they ran successfully.
May be this is occasional failure which we get after running test cases for many times.

Please let me know if you are getting this failure consistently at your end.

I ran it twice and it failed twice. I can't keep running tests over and over again as they take quite a while to run and I can't use my machine for anything else at the same time as occasionally the test browser will grab focus.

Please find updated patch.
I have slightly modified feature test case for 'view data dml queries' (though I wasn't getting failure as mentioned).
 
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

Attachment


On Tue, Jul 18, 2017 at 2:40 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,

On Tue, Jul 18, 2017 at 1:24 PM, Dave Page <dpage@pgadmin.org> wrote:


On Tue, Jul 18, 2017 at 8:26 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,


On Mon, Jul 17, 2017 at 9:33 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Mon, Jul 17, 2017 at 1:09 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi,

Please find updated patch. Now placeholder string for bytea and bytea[] data will only appear in datagrid (view all/1000/500 rows). If user executes query using Query tool then placeholder won't appear (similar to  pgAdminIII behaviour)

I'm getting the following error when testing this:

I ran all feature test cases 5-6 times and each time they ran successfully.
May be this is occasional failure which we get after running test cases for many times.

Please let me know if you are getting this failure consistently at your end.

I ran it twice and it failed twice. I can't keep running tests over and over again as they take quite a while to run and I can't use my machine for anything else at the same time as occasionally the test browser will grab focus.

Please find updated patch.
I have slightly modified feature test case for 'view data dml queries' (though I wasn't getting failure as mentioned).


Thanks, that passes.

Now I've played with it though, I think that the query tool should also hide the binary data.  I don't see a good reason to see it in one place but not another - ultimately it's still not human readable.

Also, we need to make it look like the [null] placeholder I think; e.g. use [ ] instead of < > and give it a lighter colour.


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

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

On Tue, Jul 18, 2017 at 8:31 PM, Dave Page <dpage@pgadmin.org> wrote:


On Tue, Jul 18, 2017 at 2:40 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,

On Tue, Jul 18, 2017 at 1:24 PM, Dave Page <dpage@pgadmin.org> wrote:


On Tue, Jul 18, 2017 at 8:26 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,


On Mon, Jul 17, 2017 at 9:33 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Mon, Jul 17, 2017 at 1:09 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi,

Please find updated patch. Now placeholder string for bytea and bytea[] data will only appear in datagrid (view all/1000/500 rows). If user executes query using Query tool then placeholder won't appear (similar to  pgAdminIII behaviour)

I'm getting the following error when testing this:

I ran all feature test cases 5-6 times and each time they ran successfully.
May be this is occasional failure which we get after running test cases for many times.

Please let me know if you are getting this failure consistently at your end.

I ran it twice and it failed twice. I can't keep running tests over and over again as they take quite a while to run and I can't use my machine for anything else at the same time as occasionally the test browser will grab focus.

Please find updated patch.
I have slightly modified feature test case for 'view data dml queries' (though I wasn't getting failure as mentioned).


Thanks, that passes.

Now I've played with it though, I think that the query tool should also hide the binary data.  I don't see a good reason to see it in one place but not another - ultimately it's still not human readable.

Also, we need to make it look like the [null] placeholder I think; e.g. use [ ] instead of < > and give it a lighter colour.

Please find updated patch. I have fixed all of above review comments.
 

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

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

Attachment
Hi

On Wed, Jul 19, 2017 at 1:58 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,

On Tue, Jul 18, 2017 at 8:31 PM, Dave Page <dpage@pgadmin.org> wrote:

Thanks, that passes.

Now I've played with it though, I think that the query tool should also hide the binary data.  I don't see a good reason to see it in one place but not another - ultimately it's still not human readable.

Also, we need to make it look like the [null] placeholder I think; e.g. use [ ] instead of < > and give it a lighter colour.

Please find updated patch. I have fixed all of above review comments.

Hmm, not that I can see. See the attached screenshot - I'm getting "binary data" in black, not "[binary data]" in grey (to match [null]). 


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

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


On Thu, Jul 20, 2017 at 1:02 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Wed, Jul 19, 2017 at 1:58 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,

On Tue, Jul 18, 2017 at 8:31 PM, Dave Page <dpage@pgadmin.org> wrote:

Thanks, that passes.

Now I've played with it though, I think that the query tool should also hide the binary data.  I don't see a good reason to see it in one place but not another - ultimately it's still not human readable.

Also, we need to make it look like the [null] placeholder I think; e.g. use [ ] instead of < > and give it a lighter colour.

Please find updated patch. I have fixed all of above review comments.

Hmm, not that I can see. See the attached screenshot - I'm getting "binary data" in black, not "[binary data]" in grey (to match [null]). 

Nevermind - I forgot to run make bundle...

Sorry.

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

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


On Thu, Jul 20, 2017 at 1:04 PM, Dave Page <dpage@pgadmin.org> wrote:


On Thu, Jul 20, 2017 at 1:02 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Wed, Jul 19, 2017 at 1:58 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,

On Tue, Jul 18, 2017 at 8:31 PM, Dave Page <dpage@pgadmin.org> wrote:

Thanks, that passes.

Now I've played with it though, I think that the query tool should also hide the binary data.  I don't see a good reason to see it in one place but not another - ultimately it's still not human readable.

Also, we need to make it look like the [null] placeholder I think; e.g. use [ ] instead of < > and give it a lighter colour.

Please find updated patch. I have fixed all of above review comments.

Hmm, not that I can see. See the attached screenshot - I'm getting "binary data" in black, not "[binary data]" in grey (to match [null]). 

Nevermind - I forgot to run make bundle...

Sorry.

Looks good now - patch applied!

Thanks.

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

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