Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

[Khushboo Vashi]

Hi Joao & Sarah,

On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi Khushboo!
>
> Thanks for your reply!
>
>
>> *SQL Files:*
>>>
>>>- Is there a way to avoid conditionals here?
>>>
>>>
>>>- Maybe we can use the same javascript function to prettify all the
>>>sizes
>>>
>>>
>>> In case of collection node (ex: Databases), I have implemented this
>>> functionality via putting a formatter for a back-grid column. So, it is
>>> applicable for the entire column not for the individual cell. To apply the
>>> javascript function on a single cell for the column (string column), first
>>> we need to identify that cell and then apply the function, I think this is
>>> overhead. Just to avoid conditional sql template I would not prefer this
>>> approach.
>>
>>
> We are not totally sure we understood what you meant on the previous
> statement. Are you saying that the conditionals in SQL are used to ensure
> that we can apply a javascript function at column level instead of cell
> level?
>
> Correct.

> Our concern is that the templates are being made more complex and
> inconsistencies are introduced in the code and the UI.
>

Inconsistencies in the UI can be avoided through making the size_formatter
same as pg_size_pretty function which I will implement.
I have checked the pg_size_pretty function code and it supports till TB
format, so I think we should keep till TB only.

In this particular example, we are allowing the backend to respond
> sometimes with prettified data and sometimes without it, so at UI level we
> will have inconsistencies between screens or more complex Javascript code
> to support sometimes prettifying and sometimes not prettify the same
> fields.
>
> We have separate logic for collection and single node in statistics.js and
we are using javascript code for prettifying only for collection node.


> What we were thinking was to maybe not specify on the SQL level and have
> the same format for "Size" everywhere in the UI.
>
>
Here our main concern is inconsistency in "Size" format in the UI that can
be avoided as I said earlier.
We are using pg_size_pretty function for different fields like Size, Index
Size, Table space size, Tuple length, Size of Temporary files in different
modules and some of them are cell level and we don't require to put
overhead on cell level fields as sorting is not required for individual
node statistics.



> Thanks
> Joao & Sarah
>
> On Tue, Apr 25, 2017 at 11:48 PM, Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi Joao & Sarah,
>>
>> Thanks for reviewing the patch.
>>
>> On Tue, Apr 25, 2017 at 8:34 PM, Joao Pedro De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hello Khushboo,
>>>
>>> We reviewed the this patch and have some suggestions:
>>>
>>> *Python:*
>>> ​
>>> The functionality for adding the "can_prettify" is repeated in multiple
>>> places. Maybe this could be extracted into a function.
>>>
>>> When I have implemented this, my first thought is exactly same as you
>> suggested but  while looking at the code I felt its not a good idea to have
>> a function. In case of a function, we need to pass the whole result-set as
>> well as the list of fields which we need to be prettified. So, only for 2
>> lines, I didn't find any reason to make a function.
>>
>>> *Javascript:*
>>> ​
>>>
>>>- The class Backgrid.SizeFormatter doesn't seem to have any tests.
>>>
>>>
>>> Sure, will do.
>>
>>>
>>>- The function pg_size_pretty displays bytes and Kilobytes
>>>differently.
>>>- Is it possible to add PB as well?
>>>
>>> Will check and add PB.
>>
>>>
>>>-
>>>- The function is a little bit hard to read, is it possible to
>>>refactor using private functions like:
>>>
>>> Will make it more readable.
>>
>>> fromRaw: function (rawData, model) {
>>>var unitIdx = findDataUnitIndex(rawData);
>>>if (unitIdx == 0) {
>>>   return rawData + ' ' + this.dataUnits[i];
>>>}
>>>return formatOutput(rawData, unitIdx);
>>> },
>>>
>>> ​
>>>
>>>
>>>- In statistics.js:326 we believe it would make the code more
>>>readable if we change the variable "c" to "rawColumn" and "col" to 
>>> "column".
>>>
>>>
>>> I will change the variable name from  "c" to  "rawColumn" but I think
>> "col" is appropriate as we already have columns variable and anyone can
>> confuse while reading the code (for columns and column).
>>
>>>
>>> *SQL Files:*
>>> ​
>>>
>>>- Is there a way to avoid conditionals here?
>>>- Maybe we can use the same javascript function to prettify all the
>>>sizes
>>>
>>>
>>> In case of collection node (ex: Databases), I have implemented this
>> functionality via putting a formatter for a back-grid column. So, it is
>> applicable for the entire column not for the individual cell. To apply the
>> javascript function on a single cell for the column (string column), first
>> we need to identify that cell and then apply the function, I think 

Re: [pgadmin-hackers] [Design update] Style guide for pgAdmin4

[Ashesh Vashi]

On Wed, Apr 26, 2017 at 9:56 PM, Dave Page  wrote:

>
>
> On Wed, Apr 26, 2017 at 4:45 PM, Shirley Wang  wrote:
>
>>
>>
>> I think the addition of icons and some copy would help:
>> [image: Screen Shot 2017-04-26 at 11.28.41 AM.png]
>>
>
> Agreed.
>
+1

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company



*http://www.linkedin.com/in/asheshvashi*


>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

[pgadmin-hackers] Issue with SlickGrid

[Joao Pedro De Almeida Pereira]

Hello Hackers,

While doing some changes to the Query Results we found out that there was a
issue with Slick grid.

The issue that we found was with the CellSelectModel, behaved differently
when pressing Ctrl and Command(Mac). We created a PR
 with the change to changes the
behavior of the plugin.

When this PR is applied to the SlickGrid library we need to apply it to the
current version of SlickGrid that we have vendorized.
According to the libraries.txt file we are in version 2.2.4 of the library
but a diff between our code and the libraries version 2.2.4 shows
differences in the code.

Did we do any change to SlickGrid library that is vendorized? Or is just
the information in libraries.txt that is incorrect?
Does anyone know any problem if we bump the version of SlickGrid to the
newer version after the PR is applied?

Thanks
Joao

Re: [pgadmin-hackers] Declarative partitioning in pgAdmin4

[Shirley Wang]

Hello!

On Wed, Apr 26, 2017 at 4:26 AM Dave Page  wrote:

> Hi
>
> [moving to the pgadmin-hackers mailing list as this a pgAdmin feature]
>
> On Wed, Apr 26, 2017 at 8:20 AM, Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi Dave
>>
>> Murtuza and I started thinking about "How to add Declarative
>> Partitioning" support in pgAdmin4. We thought instead of showing Partition
>> Table under existing Tables collection, we should add new collection node
>> "Partition Tables". Showing table under the table node recursively will
>> require lots of code changes in table and it's child nodes (column, index,
>> trigger, etc..) which is more complex and error prone.
>>
>
> Perhaps, but from the user's perspective, there's no reason to list them
> separately - they are just tables with a different structure from others.
> We shouldn't confuse the user just because it's more convenient for us.
>
> I really think it should look like this:
>
> - Tables
>   - t1
> - Columns
> - Constraints
> - Partitions
>   - p1
> - Sub Objects (whatever they may be)
> ...
>   - p2
>   ...
>   - t2
>   ...
>
>

>
>>
>> Below is the design that we can implement:
>>
>>- Create new "Partition Tables" collection node. User will be able to
>>create partition table by clicking "Create -> Partition Table" menu that 
>> we
>>will add on collection node. We will share the dialog prototype later
>>once we will have complete understanding of it.
>>
>> Can you share a mock-up of the dialog? The Figma tool that Shirley shared
> looks like it'll be good for doing that - I can invite you to the team.
>

>>- Once table is created user will be able to create partitions by
>>clicking "Create -> Partitions" menu will be added on each partitioned
>>table node. We will share the dialog prototype later once we will
>>have complete understanding of it.
>>
>> I would expect the user to be able to define the partitioning scheme when
> they create the table; e.g. on a new tab. It shouldn't be a two step
> process.
>
>>
>>- We will have to show sub nodes like (column, index, trigger,
>>constraints, etc..) on main table while some of the sub nodes won't 
>> require
>>for partitions like (column and many more again require some more 
>> knowledge
>>on partitioning).
>>
>> OK.
>
>
>> Apart from above we will have to figure out following:
>>
>>- How to remove partitions(table) from existing tables node as value
>>of relkind column is 'r' for partitions.
>>- Partitioning scheme to show in SQL pane for partitions.
>>- Some unknown issue/features of Declarative partitioning.
>>
>> OK.
>

Seems like there are a couple of assumptions being made here:
- Users need to see partitioned tables when expanding parent table
- Users need to view partitioned tables in context to their parent table
(Dave says yes, Akshay and Murtuza say no)
- Users want to create a partitioned table through the browser (Akshay and
Murtuza say yes, Dave says no)

Plus some technical concerns:
- Making code changes in table is complex and error prone
- How to move partitions from one node to another

I think the first assumption is important to validate or invalidate before
even thinking about how to implement or addressing technical concerns. We
may come to learn that there are solutions that don't require a lot of
technical maneuvering, or perhaps learn there's no need for change at all.

Akshay and Murtuza, I'm happy to work with you on doing some research
(interviews to discover user needs and pains, creating mockups, getting
feedback etc) and coming up with some solutions based on user feedback.

Re: [pgadmin-hackers] [Design update] Style guide for pgAdmin4

[Dave Page]

On Wed, Apr 26, 2017 at 4:45 PM, Shirley Wang  wrote:

>
>
> I think the addition of icons and some copy would help:
> [image: Screen Shot 2017-04-26 at 11.28.41 AM.png]
>

Agreed.

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

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

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

[Joao Pedro De Almeida Pereira]

Hello Ashesh,

Thanks for reviewing the patch.

We added the __init__.py files into templates to convert them into packages
so that the tests inside of them can be found by the test runner.

Thanks!
Joao & Sarah

On Wed, Apr 26, 2017 at 1:26 AM, Ashesh Vashi  wrote:

> On Mon, Apr 24, 2017 at 4:43 PM, Dave Page  wrote:
>
>> Ashesh, can you review/commit this please?
>>
>> On Fri, Apr 21, 2017 at 8:42 PM, Joao Pedro De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hi Hackers,
>>>
>>> We found out that when you are connected to a GreenPlum database and try
>>> to get Dependents and Dependencies of an object the application was
>>> returning a SQL error.
>>>
>>> This patch splits the SQL query used to retrieve the Dependents,
>>> Dependencies, and Roles SQL file into multiple versioned files.
>>> Add Unit Tests for each file.
>>> Also added __init__.py files to other test directories to run the tests
>>> in them.
>>>
>> Hi Joao & Sarah,
>
> Why do we need to add __init__.py in the template directory?
> I didn't understand the purpose of the adding __init__.py files in the
> template directories.
>
> NOTE: The headers in those files are not consistent with the other project
> files.
>
> --
>
> Thanks & Regards,
>
> Ashesh Vashi
> EnterpriseDB INDIA: Enterprise PostgreSQL Company
> 
>
>
> *http://www.linkedin.com/in/asheshvashi*
> 
>
>> Add ORDER BY into Copy Selection Feature test to ensure the results are
>>> retrieved always in the same order
>>> Renamed the Scenario of the xss_checks_pgadmin_debugger_test and skip
>>> it for versions less than 9.1
>>>
>>> Thanks
>>>
>>> Joao & Sarah
>>>
>>>
>>> --
>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>>> To make changes to your subscription:
>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>
>>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

[Joao Pedro De Almeida Pereira]

Hi Khushboo!

Thanks for your reply!


> *SQL Files:*
>>
>>- Is there a way to avoid conditionals here?
>>
>>
>>- Maybe we can use the same javascript function to prettify all the
>>sizes
>>
>>
>> In case of collection node (ex: Databases), I have implemented this
>> functionality via putting a formatter for a back-grid column. So, it is
>> applicable for the entire column not for the individual cell. To apply the
>> javascript function on a single cell for the column (string column), first
>> we need to identify that cell and then apply the function, I think this is
>> overhead. Just to avoid conditional sql template I would not prefer this
>> approach.
>
>
We are not totally sure we understood what you meant on the previous
statement. Are you saying that the conditionals in SQL are used to ensure
that we can apply a javascript function at column level instead of cell
level?

Our concern is that the templates are being made more complex and
inconsistencies are introduced in the code and the UI. In this particular
example, we are allowing the backend to respond sometimes with prettified
data and sometimes without it, so at UI level we will have inconsistencies
between screens or more complex Javascript code to support sometimes
prettifying and sometimes not prettify the same fields.

What we were thinking was to maybe not specify on the SQL level and have
the same format for "Size" everywhere in the UI.

Thanks
Joao & Sarah

On Tue, Apr 25, 2017 at 11:48 PM, Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi Joao & Sarah,
>
> Thanks for reviewing the patch.
>
> On Tue, Apr 25, 2017 at 8:34 PM, Joao Pedro De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Khushboo,
>>
>> We reviewed the this patch and have some suggestions:
>>
>> *Python:*
>> ​
>> The functionality for adding the "can_prettify" is repeated in multiple
>> places. Maybe this could be extracted into a function.
>>
>> When I have implemented this, my first thought is exactly same as you
> suggested but  while looking at the code I felt its not a good idea to have
> a function. In case of a function, we need to pass the whole result-set as
> well as the list of fields which we need to be prettified. So, only for 2
> lines, I didn't find any reason to make a function.
>
>> *Javascript:*
>> ​
>>
>>- The class Backgrid.SizeFormatter doesn't seem to have any tests.
>>
>>
>> Sure, will do.
>
>>
>>- The function pg_size_pretty displays bytes and Kilobytes
>>differently.
>>- Is it possible to add PB as well?
>>
>> Will check and add PB.
>
>>
>>-
>>- The function is a little bit hard to read, is it possible to
>>refactor using private functions like:
>>
>> Will make it more readable.
>
>> fromRaw: function (rawData, model) {
>>var unitIdx = findDataUnitIndex(rawData);
>>if (unitIdx == 0) {
>>   return rawData + ' ' + this.dataUnits[i];
>>}
>>return formatOutput(rawData, unitIdx);
>> },
>>
>> ​
>>
>>
>>- In statistics.js:326 we believe it would make the code more
>>readable if we change the variable "c" to "rawColumn" and "col" to 
>> "column".
>>
>>
>> I will change the variable name from  "c" to  "rawColumn" but I think
> "col" is appropriate as we already have columns variable and anyone can
> confuse while reading the code (for columns and column).
>
>>
>> *SQL Files:*
>> ​
>>
>>- Is there a way to avoid conditionals here?
>>- Maybe we can use the same javascript function to prettify all the
>>sizes
>>
>>
>> In case of collection node (ex: Databases), I have implemented this
> functionality via putting a formatter for a back-grid column. So, it is
> applicable for the entire column not for the individual cell. To apply the
> javascript function on a single cell for the column (string column), first
> we need to identify that cell and then apply the function, I think this is
> overhead. Just to avoid conditional sql template I would not prefer this
> approach.
>
>>
>> Visually we saw a difference between "Databases" statistics and a
>> specific database statistics. In "Databases" statistics the "Size" is "7.4
>> MB" but when you are in the specific database the "Size" is "7420 kB".
>> Is this the intended behavior?
>>
>> Only for the Databases (collection node), the client side functionality
> is implemented not for individual node , so this behaviour is different.
> For the individual node still, we are using pg_size_pretty function
>
>>
>>
>
>> Thanks
>> Joao & Sarah
>>
>> On Tue, Apr 25, 2017 at 7:58 AM, Dave Page  wrote:
>>
>>> Ashesh, can you review/commit this please?
>>>
>>> Thanks.
>>>
>>> On Tue, Apr 25, 2017 at 10:18 AM, Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
 Hi,

 Fixed RM #2315 : Sorting by size is broken.

 Removed the pg_size_pretty function from query for the collection and
 introduced the client side function to convert size into human readable
 

Re: [pgadmin-hackers] Declarative partitioning in pgAdmin4

[Dave Page]

Hi

[moving to the pgadmin-hackers mailing list as this a pgAdmin feature]

On Wed, Apr 26, 2017 at 8:20 AM, Akshay Joshi  wrote:

> Hi Dave
>
> Murtuza and I started thinking about "How to add Declarative
> Partitioning" support in pgAdmin4. We thought instead of showing Partition
> Table under existing Tables collection, we should add new collection node
> "Partition Tables". Showing table under the table node recursively will
> require lots of code changes in table and it's child nodes (column, index,
> trigger, etc..) which is more complex and error prone.
>

Perhaps, but from the user's perspective, there's no reason to list them
separately - they are just tables with a different structure from others.
We shouldn't confuse the user just because it's more convenient for us.

I really think it should look like this:

- Tables
  - t1
- Columns
- Constraints
- Partitions
  - p1
- Sub Objects (whatever they may be)
...
  - p2
  ...
  - t2
  ...



>
> Below is the design that we can implement:
>
>- Create new "Partition Tables" collection node. User will be able to
>create partition table by clicking "Create -> Partition Table" menu that we
>will add on collection node. We will share the dialog prototype later
>once we will have complete understanding of it.
>
> Can you share a mock-up of the dialog? The Figma tool that Shirley shared
looks like it'll be good for doing that - I can invite you to the team.

>
>- Once table is created user will be able to create partitions by
>clicking "Create -> Partitions" menu will be added on each partitioned
>table node. We will share the dialog prototype later once we will have
>complete understanding of it.
>
> I would expect the user to be able to define the partitioning scheme when
they create the table; e.g. on a new tab. It shouldn't be a two step
process.

>
>- We will have to show sub nodes like (column, index, trigger,
>constraints, etc..) on main table while some of the sub nodes won't require
>for partitions like (column and many more again require some more knowledge
>on partitioning).
>
> OK.


> Apart from above we will have to figure out following:
>
>- How to remove partitions(table) from existing tables node as value
>of relkind column is 'r' for partitions.
>- Partitioning scheme to show in SQL pane for partitions.
>- Some unknown issue/features of Declarative partitioning.
>
> OK.


> The above implementation may take more time, so it might possible that we
> may not be able to finish it by 14th May (deadline).
>

It would be nice to have it by then, but the true deadline will be a later
beta (TBD, but probably beta 2 which is sufficiently far off).

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

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

Re: [pgadmin-hackers] [Design update] Style guide for pgAdmin4

[Dave Page]

Hi!

On Tue, Apr 25, 2017 at 9:33 PM, Shirley Wang  wrote:

> Hello!
>
> *Update on alerts*
> Here are mockups of error and update alert styles, as modeled by
> bootstrap.
>
> Success messages appear
> - after running a query
> - adding a database (database connected)
>
> Error messages appear
> - after running query
> - missing info in dialogs
>
> Let me know if I'm missing any areas that have different styling.
>
>
> *Running a query*
> *[image: success-query.png]*
> [image: error-query.png]
> I've added rounded corners - it doesn't seem like there are any other
> areas where sharp corners are used so I think these should be round.
>

Nice!


>
> *Dialog messages*
>
> *[image: database-error.png]*
>
> *[image: connectserver-error.png]*
>
> A note about 'connect to server' message. Currently the error appears
> above where you enter a password. I think all messages should live at the
> bottom of the dialog, above the buttons.
>

Agreed. I like the change - though I prefer the version in Figma (below):

[image: Inline image 1]

I think it looks more attractive, and is more obviously a "transient
element" (for want of a better term) - i.e. something that will go away
again. I think we also need to style the error highlighting on the actual
field to use the same shades of red.


>
>
>
> *Update on tools*
> I played around with Lucidcharts and it seems like the only units
> available for drawing out shapes are inches or centimeters, neither of
> which is particularly useful for specifying sizes of buttons, menus etc.
>
> I found Figma, which is a fairly feature robust web app, similar to Sketch
> and designed for collaboration (it's also free!). Other pros:
> - saved asset library for projects (perfect for mocking up future pgAdmin
> screens w/o the need to recreate assets)
> - version control
> - link for public access
> - generates some CSS from the image
>
> Here's the link to the project. https://www.figma.com/file/
> UjGekp34w6iBizjlGu2D4ccp/4.21pgAdmin-styles-Page-1
>
> Let me know what you think.
>

I'll have a play with it. Initial thoughts (after literally 5 minutes) are
that it seems like it could be very useful. I particularly like the asset
library.

Thanks!

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

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