Re: Bug #3083 fix

[Akshay Joshi]

Thanks Joao.

On Wed, Mar 28, 2018 at 11:36 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hey Akshay and Neethu
>
> We refactored the patch to add tests for the resize feature.  We were able
> to write test cases for the drag event by using spies and setting the rect
> dimensions.  In cases like this, we can just test some components in order
> to have enough confidence in the code.  So we isolated the function that
> implements the behavior of this feature and tested that it was performing
> as expected.
>
> We ran the patch through the pipelines and all of the tests passed.
>
> Sincerely,
>
> Joao and Victoria
>
> On Wed, Mar 28, 2018 at 8:03 AM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi
>>
>> On Fri, Mar 2, 2018 at 3:40 AM, Joao De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hello Neethu,
>>> We passed the patch through our CI pipeline and all tests pass.
>>> The code looks good, but we are trying to decouple files as much  as we
>>> can so that we do not end up with files with over 1000 lines, that are hard
>>> to read and to maintain. Also we are trying to create Unit Tests to have
>>> more coverage in our Javascript code.
>>>
>>> Can you split the new implementation code into it's own file and create
>>> some tests to ensure the behavior will not be broken in the future?iYou
>>> have some examples on: pgadmin/browser/server_groups/servers/databases/
>>> external_tables/*
>>>
>>
>> I have spilt the new implementation into different file. Its' been
>> hard to write jasmine/feature test case as it requires drag event and exact
>> co-ordinate to resize the slickgrid cell.
>> Attached is the modified patch.
>>
>>
>>>
>>> Thanks
>>> Joao
>>>
>>> On Thu, Mar 1, 2018 at 10:37 AM Neethu Mariya Joy <
>>> neethumariya...@gmail.com> wrote:
>>>
 Hi,
 I am Neethu Mariya Joy, an undergraduate pursuing BE in Computer
 Science at BITS Pilani.

 I've attempted to fix https://redmine.postgresql.org/issues/3083.
 Since the textarea resize feature is the default HTML feature, I have not
 changed it. Instead, I've added draggable borders to the wrapper which
 expands the textarea inside it.

 I'm attaching my patch as bug3083.diff below as per the contribution
 guidelines.

 Hope this helps. Thank you for your consideration!

 Sincerely,
 Neethu Mariya Joy
 GitHub  | Linkedin
 



>>
>>
>> --
>> *Akshay Joshi*
>>
>> *Sr. Software Architect *
>>
>>
>>
>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
>> <+91%2097678%2088246>*
>>
>


-- 
*Akshay Joshi*

*Sr. Software Architect *



*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*

[pgAdmin4][Patch]: RM #3180 Index node is missing from the tree view of the table node

[Akshay Joshi]

Hi Hackers,

Please find the attached patch to fix RM #3180 Index node is missing from
the tree view of the table node. This is a regression of one of the older
commit.

-- 
*Akshay Joshi*

*Sr. Software Architect *



*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*


RM_3180.patch
Description: Binary data

Re: [pgAdmin4][Patch]: RM #1978 - Add an option to allow user to disable alertifyjs and acitree animations

[Khushboo Vashi]

Hi Joao,

On Tue, Mar 27, 2018 at 12:12 AM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi Khushboo,
>
> Looks like you have a typo on your CSS where it reads 'zoomeIn' it should
> be 'zoomIn'.
>
Thanks, I have sent the updated patch.

> Also setting more parameters into the window, in our experience, is never
> good, so maybe it is time to create a real settings cache that can retrieve
> from the backend the settings, like this one.
>
> We already have a cache for preferences, so I have used that in the
updated patch.

> Thanks
> Victoria & Joao
>
> Thanks,
Khushboo

> On Mon, Mar 26, 2018 at 8:38 AM Dave Page  wrote:
>
>> Hi
>>
>> On Mon, Mar 26, 2018 at 7:23 AM, Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached patch to fix RM #1978: Add an option to allow
>>> user to disable alertifyjs and acitree animations.
>>>
>>
>> I think these really need to be per-user settings, not per-installation..
>> Whether or not animations are shown is really a matter of personal taste
>> and circumstance.
>>
>> Thanks.
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>

Re: Bug #3083 fix

[Dave Page]

Hi

On Wed, Mar 28, 2018 at 7:06 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hey Akshay and Neethu
>
> We refactored the patch to add tests for the resize feature.  We were able
> to write test cases for the drag event by using spies and setting the rect
> dimensions.  In cases like this, we can just test some components in order
> to have enough confidence in the code.  So we isolated the function that
> implements the behavior of this feature and tested that it was performing
> as expected.
>
> We ran the patch through the pipelines and all of the tests passed.
>

I'm consistently seeing the feature test failure below with this patch
applied:

==
FAIL: 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 125, in runTest
self._verify_row_data(True)
  File
"/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py",
line 325, in _verify_row_data
self.assertEquals(cells[idx], config_data[str(idx)][1])
AssertionError: u'[null]' != u'1'
- [null]
+ 1


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

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

Re: [pgAdmin4][Patch]: RM #3180 Index node is missing from the tree view of the table node

[Murtuza Zabuawala]

Hi Akshay,

I have concerns regarding the fix, As you negate the condition, Before the
fix it was not displaying Index node for Tables but after the fix it will
not display it for Partition tables.
But when I read the Postgres docs it say,
*Partitions may themselves be defined as partitioned tables, using what is
called sub-partitioning. Partitions may have their own indexes, constraints
and default values, distinct from those of other partitions. Indexes must
be created separately for each partition. See CREATE TABLE
 for more
details on creating partitioned tables and partitions.*
Ref: https://www.postgresql.org/docs/10/static/ddl-partitioning.html (Sec:
5.10.2)



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


On Thu, Mar 29, 2018 at 6:05 PM, Akshay Joshi  wrote:

> Hi Hackers,
>
> Please find the attached patch to fix RM #3180 Index node is missing from
> the tree view of the table node. This is a regression of one of the older
> commit.
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>

pgAdmin 4 commit: Fix index node display on PG 10. Fixes #3180

[Dave Page]

Fix index node display on PG 10. Fixes #3180

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=61eb94a23c043930034d66a18ea1189343f0ee8a
Author: Akshay Joshi 

Modified Files
--
docs/en_US/release_notes_3_0.rst| 1 +
.../server_groups/servers/databases/schemas/tables/indexes/__init__.py  | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)

Re: [pgAdmin4][Patch]: RM #3180 Index node is missing from the tree view of the table node

[Dave Page]

Thanks, applied.

On Thu, Mar 29, 2018 at 1:35 PM, Akshay Joshi  wrote:

> Hi Hackers,
>
> Please find the attached patch to fix RM #3180 Index node is missing from
> the tree view of the table node. This is a regression of one of the older
> commit.
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
> <+91%2097678%2088246>*
>



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

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

Re: [pgAdmin4][Patch]: RM #1978 - Add an option to allow user to disable alertifyjs and acitree animations

[Dave Page]

Hi

On Thu, Mar 29, 2018 at 1:51 PM, Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

>
>
> On Mon, Mar 26, 2018 at 6:07 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Mon, Mar 26, 2018 at 7:23 AM, Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached patch to fix RM #1978: Add an option to allow
>>> user to disable alertifyjs and acitree animations.
>>>
>>
>> I think these really need to be per-user settings, not per-installation..
>> Whether or not animations are shown is really a matter of personal taste
>> and circumstance.
>>
>> Right, it should be per-user settings.  Please find the attached updated
> patch.
>

I found some issues I'm afraid:

- The label "Enable dialogues/notifications animation?" should read "Enable
dialogue/notification animation?"

- Disabling treeview animation only seems to affect the main browser
treeview, and not others in the application (e.g. the one on the
Preferences panel).

 - After disabling dialogue/notification animations, I cannot re-enable
notification animations. If I flip the switch back on, dialogue animations
immediately start working again, but notification animations don't even
work following a reload.

Thanks.

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

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

Re: [pgAdmin4][Patch]: RM #1978 - Add an option to allow user to disable alertifyjs and acitree animations

[Joao De Almeida Pereira]

Hi Khushboo,

The patch looks like it is heading on the correct direction, we passed it
through our test pipeline and all tests passed.

We found the same issues as Dave mentioned in his email, also after some
code review we have the following questions/comments:

. Why does modify_animation.js have a dependency on sources/pgadmin if it
doesnt use it?
. Can we convert modify_animation.js to ES6 without requirejs?
. Why does modifyAnimation function have 2 arguments if we never use the
second one?
. Can we convert modify_animation_spec.js to ES6 without requirejs?
. Why is pgBrowser.tree.options function called 4 times in the tests?
   As an aside, when you use toHaveBeenCalledWith it is redundant to expect
on toHaveBeenCalled
. Looks like we are missing some coverage of the alertify modification as
well


As an aside get_preferences, the "cache", is still broken, if the cache as
no value it will retrieve it but returns undefined to the caller. This
behavior need to be addressed. We should change get preferences to be a
Promise based thing or else this might become a problem

As another aside, one of our goals should be to move away from requirejs
into a full ES6, webpack javascript build. In order to do that we should
try to write the least amount of code possible using requirejs syntax. If
we really need to write something in requirejs let it be a wrapper that
call our ES6 function/class

Thanks
Joao

On Thu, Mar 29, 2018 at 9:25 AM Dave Page  wrote:

> Hi
>
> On Thu, Mar 29, 2018 at 1:51 PM, Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>>
>>
>> On Mon, Mar 26, 2018 at 6:07 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Mon, Mar 26, 2018 at 7:23 AM, Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
 Hi,

 Please find the attached patch to fix RM #1978: Add an option to allow
 user to disable alertifyjs and acitree animations.

>>>
>>> I think these really need to be per-user settings, not
>>> per-installation.. Whether or not animations are shown is really a matter
>>> of personal taste and circumstance.
>>>
>>> Right, it should be per-user settings.  Please find the attached updated
>> patch.
>>
>
> I found some issues I'm afraid:
>
> - The label "Enable dialogues/notifications animation?" should read
> "Enable dialogue/notification animation?"
>
> - Disabling treeview animation only seems to affect the main browser
> treeview, and not others in the application (e.g. the one on the
> Preferences panel).
>
>  - After disabling dialogue/notification animations, I cannot re-enable
> notification animations. If I flip the switch back on, dialogue animations
> immediately start working again, but notification animations don't even
> work following a reload.
>
> Thanks.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

[Joao De Almeida Pereira]

Hi Murtuza,

After changing the setting in the preferences nothing happened, we had to
reset the layout or refresh the app to see it working. It only looks the
right side. Was this the intended behavior?

Not sure if this is the expected behavior or not. I would expect that any
change I do in the preferences would start working after I press the Save
button. This also happens with other preferences that only take effect
after refresh on the browser.
This being said, not sure if having the templated variable in the
javascript file is the best approach in this case.

Do you think you can remove the requirejs tags on the tests?

At the testing file you do not need to create 3 different variables for the
panels, you can reuse it, because the beforeEach will run for every test

Thanks
Joao

On Thu, Mar 29, 2018 at 9:48 AM Dave Page  wrote:

> Hi
>
> On Thu, Mar 29, 2018 at 2:15 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA patch which will allow user to lock the panels and it will not allow
>> user to drag & drop them.
>>
>
> Tests pass, but when I lock the layout, I can still drag panels and adjust
> the splitters etc. After doing so,  reset the layout and now have the
> broken layout seen in the attached screenshot. I have rebuilt the bundle,
> reloaded etc.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: Bug #3083 fix

[Joao De Almeida Pereira]

Hi Dave,
That looks like in the surrounding area of the change. We run our pipeline
and everything was green.
Can you provide more details, which python version are you using? OS?

Thanks
Joao

On Thu, Mar 29, 2018 at 9:03 AM Dave Page  wrote:

> Hi
>
> On Wed, Mar 28, 2018 at 7:06 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hey Akshay and Neethu
>>
>> We refactored the patch to add tests for the resize feature.  We were
>> able to write test cases for the drag event by using spies and setting the
>> rect dimensions.  In cases like this, we can just test some components in
>> order to have enough confidence in the code.  So we isolated the function
>> that implements the behavior of this feature and tested that it was
>> performing as expected.
>>
>> We ran the patch through the pipelines and all of the tests passed.
>>
>
> I'm consistently seeing the feature test failure below with this patch
> applied:
>
> ==
> FAIL: 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 125, in runTest
> self._verify_row_data(True)
>   File
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py",
> line 325, in _verify_row_data
> self.assertEquals(cells[idx], config_data[str(idx)][1])
> AssertionError: u'[null]' != u'1'
> - [null]
> + 1
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [pgAdmin4][Patch]: RM #3180 Index node is missing from the tree view of the table node

[Joao De Almeida Pereira]

Hi Murtuza,
Lets imagine you have

CREATE TABLE cities (
city_id bigserial not null,
name text not null,
population   int
) PARTITION BY LIST (initcap(name));

CREATE TABLE cities_west
PARTITION OF cities (
CONSTRAINT city_id_nonzero CHECK (city_id != 0)
) FOR VALUES IN ('Los Angeles', 'San Francisco') PARTITION BY RANGE
(population);

CREATE TABLE cities_west_1_to_10
PARTITION OF cities_west FOR VALUES FROM (1) TO (10);

​

You can only create an index in *cities_west_1_to_10* because
postgresql assumes that *cities_west* is also a partitioned table. So the
implementation looks correct, despite the fact that there are no tests
around it.

Thanks
Joao

On Thu, Mar 29, 2018 at 9:26 AM Akshay Joshi 
wrote:

> Hi Murtuza
>
> On Thu, Mar 29, 2018 at 6:36 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Akshay,
>>
>> I have concerns regarding the fix, As you negate the condition, Before
>> the fix it was not displaying Index node for Tables but after the fix it
>> will not display it for Partition tables.
>> But when I read the Postgres docs it say,
>> *Partitions may themselves be defined as partitioned tables, using what
>> is called sub-partitioning. Partitions may have their own indexes,
>> constraints and default values, distinct from those of other partitions.
>> Indexes must be created separately for each partition. See CREATE TABLE
>>  for more
>> details on creating partitioned tables and partitions.*
>> Ref: https://www.postgresql.org/docs/10/static/ddl-partitioning.html
>> (Sec: 5.10.2)
>>
>
> Yes that is correct, but it's about Partitions(child tables), not the
> *Partitioned* table. We are showing indexes on Partitions. Please refer
> "Index_on_Partitioned_table.png"
>
>>
>>
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>> On Thu, Mar 29, 2018 at 6:05 PM, Akshay Joshi <
>> akshay.jo...@enterprisedb.com> wrote:
>>
>>> Hi Hackers,
>>>
>>> Please find the attached patch to fix RM #3180 Index node is missing
>>> from the tree view of the table node. This is a regression of one of the
>>> older commit.
>>>
>>> --
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect *
>>>
>>>
>>>
>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
>>> 976-788-8246 <+91%2097678%2088246>*
>>>
>>
>>
>
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
> <+91%2097678%2088246>*
>

Re: [pgAdmin4][Patch]: RM #3180 Index node is missing from the tree view of the table node

[Murtuza Zabuawala]

Thanks Akshay & Joao, got it.

My bad sorry for the noise.

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


On Thu, Mar 29, 2018 at 9:06 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi Murtuza,
> Lets imagine you have
>
> CREATE TABLE cities (
> city_id bigserial not null,
> name text not null,
> population   int
> ) PARTITION BY LIST (initcap(name));
>
> CREATE TABLE cities_west
> PARTITION OF cities (
> CONSTRAINT city_id_nonzero CHECK (city_id != 0)
> ) FOR VALUES IN ('Los Angeles', 'San Francisco') PARTITION BY RANGE 
> (population);
>
> CREATE TABLE cities_west_1_to_10
> PARTITION OF cities_west FOR VALUES FROM (1) TO (10);
>
> ​
>
> You can only create an index in *cities_west_1_to_10* because
> postgresql assumes that *cities_west* is also a partitioned table. So the
> implementation looks correct, despite the fact that there are no tests
> around it.
>
> Thanks
> Joao
>
> On Thu, Mar 29, 2018 at 9:26 AM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi Murtuza
>>
>> On Thu, Mar 29, 2018 at 6:36 PM, Murtuza Zabuawala > enterprisedb.com> wrote:
>>
>>> Hi Akshay,
>>>
>>> I have concerns regarding the fix, As you negate the condition, Before
>>> the fix it was not displaying Index node for Tables but after the fix it
>>> will not display it for Partition tables.
>>> But when I read the Postgres docs it say,
>>> *Partitions may themselves be defined as partitioned tables, using what
>>> is called sub-partitioning. Partitions may have their own indexes,
>>> constraints and default values, distinct from those of other partitions.
>>> Indexes must be created separately for each partition. See CREATE TABLE
>>>  for more
>>> details on creating partitioned tables and partitions.*
>>> Ref: https://www.postgresql.org/docs/10/static/ddl-partitioning.html
>>> (Sec: 5.10.2)
>>>
>>
>> Yes that is correct, but it's about Partitions(child tables), not the
>> *Partitioned* table. We are showing indexes on Partitions. Please refer
>> "Index_on_Partitioned_table.png"
>>
>>>
>>>
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>>
>>> On Thu, Mar 29, 2018 at 6:05 PM, Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>
 Hi Hackers,

 Please find the attached patch to fix RM #3180 Index node is missing
 from the tree view of the table node. This is a regression of one of the
 older commit.

 --
 *Akshay Joshi*

 *Sr. Software Architect *



 *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
 976-788-8246 <+91%2097678%2088246>*

>>>
>>>
>>
>>
>> --
>> *Akshay Joshi*
>>
>> *Sr. Software Architect *
>>
>>
>>
>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
>> <+91%2097678%2088246>*
>>
>

Re: [pgAdmin4][Patch]: RM #3180 Index node is missing from the tree view of the table node

[Robert Eckhardt]

On Thu, Mar 29, 2018 at 12:45 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Thanks Akshay & Joao, got it.
>
> My bad sorry for the noise.
>

The question helped me, nothing wrong with questions.

-- Rob


>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Thu, Mar 29, 2018 at 9:06 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hi Murtuza,
>> Lets imagine you have
>>
>> CREATE TABLE cities (
>> city_id bigserial not null,
>> name text not null,
>> population   int
>> ) PARTITION BY LIST (initcap(name));
>>
>> CREATE TABLE cities_west
>> PARTITION OF cities (
>> CONSTRAINT city_id_nonzero CHECK (city_id != 0)
>> ) FOR VALUES IN ('Los Angeles', 'San Francisco') PARTITION BY RANGE 
>> (population);
>>
>> CREATE TABLE cities_west_1_to_10
>> PARTITION OF cities_west FOR VALUES FROM (1) TO (10);
>>
>> ​
>>
>> You can only create an index in *cities_west_1_to_10* because
>> postgresql assumes that *cities_west* is also a partitioned table. So
>> the implementation looks correct, despite the fact that there are no tests
>> around it.
>>
>> Thanks
>> Joao
>>
>> On Thu, Mar 29, 2018 at 9:26 AM Akshay Joshi <
>> akshay.jo...@enterprisedb.com> wrote:
>>
>>> Hi Murtuza
>>>
>>> On Thu, Mar 29, 2018 at 6:36 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Akshay,

 I have concerns regarding the fix, As you negate the condition, Before
 the fix it was not displaying Index node for Tables but after the fix it
 will not display it for Partition tables.
 But when I read the Postgres docs it say,
 *Partitions may themselves be defined as partitioned tables, using what
 is called sub-partitioning. Partitions may have their own indexes,
 constraints and default values, distinct from those of other partitions.
 Indexes must be created separately for each partition. See CREATE TABLE
  for more
 details on creating partitioned tables and partitions.*
 Ref: https://www.postgresql.org/docs/10/static/ddl-partitioning.html
 (Sec: 5.10.2)

>>>
>>> Yes that is correct, but it's about Partitions(child tables), not
>>> the *Partitioned* table. We are showing indexes on Partitions. Please refer
>>> "Index_on_Partitioned_table.png"
>>>



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


 On Thu, Mar 29, 2018 at 6:05 PM, Akshay Joshi <
 akshay.jo...@enterprisedb.com> wrote:

> Hi Hackers,
>
> Please find the attached patch to fix RM #3180 Index node is missing
> from the tree view of the table node. This is a regression of one of the
> older commit.
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
> 976-788-8246 <+91%2097678%2088246>*
>


>>>
>>>
>>> --
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect *
>>>
>>>
>>>
>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
>>> 976-788-8246 <+91%2097678%2088246>*
>>>
>>
>

Re: [pgAdmin4][Patch][Feature #1998] Appends .sql if extension not given when using 'save' or 'save as' feature

[Joao De Almeida Pereira]

Hi Rahul,
I see you extracted some code, that is a pretty good move :D

We run the patch through the testing pipeline and everything is green GJ :D
Also tested the functionality by hand and looks like it is working except
for "add the .sql extension when format is set to SQL." if you set it to
All Files  the extension is also added. Not sure if this is a big deal or
not. Lets see what other people think.

Codewise here are some of my comments:
. You added the yarn-error.log file and a migration to the patch doesn't
look intentional. Can you please remove them?
. Also in the patch there are 2 file (moc_LogWindow.cpp and ui_LogWindow.h)
that look like they do not belong to the patch (Did you rebase your branch
before trying to create the patch?

The test file: test_save_query_to_file.py is empty, it is missing some
tests there.

As a convention we user lower case names for functions and UpperCase for
class

Please, regenerate the patch following my previous comments.

Thanks
Joao

On Thu, Mar 29, 2018 at 12:54 PM Rahul Soshte 
wrote:

> Hi,
> When using save or save as feature if .sql is not provided this Patch
> appends it.
> as clearly mentioned in this link.
>
> https://redmine.postgresql.org/issues/1998
>
> I have ran pep8,regression and Jasmine tests too.
>
> I have primarily changed these files
>  web/pgadmin/tools/sqleditor/__init__.py
>  web/pgadmin/tools/sqleditor/static/js/sqleditor.js
>  web/pgadmin/tools/sqleditor/utils/save_query_to_file.py
>
>
> Regards,
> Rahul Soshte (Hunter)
>
>