Обсуждение: pgAdmin4 PATCH: Domain Module
Hi,
Please find attached patch for the Domain Module.Please review it and let me know the feedback.
Вложения
Resending patch with binary option.
On Wed, Jan 20, 2016 at 10:18 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
KhushbooThe patch will be modified after Types module implementation as we need to populate Base Type and some Type related validations from the Types module.Hi,Please find attached patch for the Domain Module.Please review it and let me know the feedback.Thanks,
Вложения
Hi Khushboo,
While applying the patch file, we are getting below warnings.
#########################################
domains (1).patch:1340: trailing whitespace.
oid: undefined,
domains (1).patch:1483: trailing whitespace.
(nspname = 'pg_catalog' AND EXISTS
domains (1).patch:1487: trailing whitespace.
OR (nspname = 'information_schema' AND EXISTS
domains (1).patch:1489: trailing whitespace.
OR (nspname LIKE '_%' AND EXISTS
domains (1).patch:1642: trailing whitespace.
(select 1 from pg_class where relnamespace=typnamespace and relname = typname and relkind != 'c') AND (typname not like '_%' OR NOT EXISTS (select 1 from pg_class where relnamespace=typnamespace and relname = substring(typname from 2)::name and relkind != 'c'))
warning: squelched 4 whitespace errors
warning: 9 lines add whitespace errors.
#########################################
Can you please remove the whitespace and regenerate the patch ?
Thanks,
Neel Patel
On Wed, Jan 20, 2016 at 12:37 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Resending patch with binary option.On Wed, Jan 20, 2016 at 10:18 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:KhushbooThe patch will be modified after Types module implementation as we need to populate Base Type and some Type related validations from the Types module.Hi,Please find attached patch for the Domain Module.Please review it and let me know the feedback.Thanks,
--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Hi Neel,
Please find updated patch.On Wed, Jan 20, 2016 at 12:50 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:
Hi Khushboo,While applying the patch file, we are getting below warnings.#########################################domains (1).patch:1340: trailing whitespace.oid: undefined,domains (1).patch:1483: trailing whitespace.(nspname = 'pg_catalog' AND EXISTSdomains (1).patch:1487: trailing whitespace.OR (nspname = 'information_schema' AND EXISTSdomains (1).patch:1489: trailing whitespace.OR (nspname LIKE '_%' AND EXISTSdomains (1).patch:1642: trailing whitespace.(select 1 from pg_class where relnamespace=typnamespace and relname = typname and relkind != 'c') AND (typname not like '_%' OR NOT EXISTS (select 1 from pg_class where relnamespace=typnamespace and relname = substring(typname from 2)::name and relkind != 'c'))warning: squelched 4 whitespace errorswarning: 9 lines add whitespace errors.#########################################Can you please remove the whitespace and regenerate the patch ?Thanks,Neel PatelOn Wed, Jan 20, 2016 at 12:37 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:--Resending patch with binary option.On Wed, Jan 20, 2016 at 10:18 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:KhushbooThe patch will be modified after Types module implementation as we need to populate Base Type and some Type related validations from the Types module.Hi,Please find attached patch for the Domain Module.Please review it and let me know the feedback.Thanks,
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Вложения
Hi Khushboo,
Please find below review comments.
- While creating new Domain and clicking on SQL tab, python side we are getting error saying "TypeError: 'bool' object is not callable".
We are not able to create any domain. Fix this issue so that we can test other functionality.
- Implement the reverse engineering SQL generation for the domain node.
- As per the checklist, remove the "Use Slony" from Constraints tab, as it is not required.
- No need to pass "qtIdent=self.qtIdent" as function argument in "create" and "getSQL" function in domains/__init__.py
- In "Security" tab , provider and security label fields are not editable.
- In PG version 9.1, when we update the existing domain name then "ALTER DOMAIN" is not supported.
Currently there is no checking for the PG version 9.1 and 9.2_plus. It will fail when we connect to database 9.1
e.g.
For PG version 9.1 - Update command should be as below.
ALTER TYPE xyz RENAME TO abc;
For PG version 9.2 onwards - Update command should be as below.
ALTER DOMAIN xyz RENAME TO abc;
- Some of the SQL file, qtIdent is not used. Please check all the related SQL files.
e.g. - In update.sql file "data.owner" should be "conn|qtIdent(data.owner)"
{% if data.owner %}
ALTER DOMAIN {{ conn|qtIdent(o_data.basensp, name) }}
OWNER TO {{ data.owner }};
{% endif %}
Let us know for any issues.
Thanks,
Neel Patel
On Wed, Jan 20, 2016 at 2:50 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
KhushbooThanks,Hi Neel,Please find updated patch.On Wed, Jan 20, 2016 at 12:50 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:Hi Khushboo,While applying the patch file, we are getting below warnings.#########################################domains (1).patch:1340: trailing whitespace.oid: undefined,domains (1).patch:1483: trailing whitespace.(nspname = 'pg_catalog' AND EXISTSdomains (1).patch:1487: trailing whitespace.OR (nspname = 'information_schema' AND EXISTSdomains (1).patch:1489: trailing whitespace.OR (nspname LIKE '_%' AND EXISTSdomains (1).patch:1642: trailing whitespace.(select 1 from pg_class where relnamespace=typnamespace and relname = typname and relkind != 'c') AND (typname not like '_%' OR NOT EXISTS (select 1 from pg_class where relnamespace=typnamespace and relname = substring(typname from 2)::name and relkind != 'c'))warning: squelched 4 whitespace errorswarning: 9 lines add whitespace errors.#########################################Can you please remove the whitespace and regenerate the patch ?Thanks,Neel PatelOn Wed, Jan 20, 2016 at 12:37 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:--Resending patch with binary option.On Wed, Jan 20, 2016 at 10:18 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:KhushbooThe patch will be modified after Types module implementation as we need to populate Base Type and some Type related validations from the Types module.Hi,Please find attached patch for the Domain Module.Please review it and let me know the feedback.Thanks,
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Hi Neel,
Thanks for reviewing my patch.On Wed, Jan 20, 2016 at 10:56 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:
Hi Khushboo,Please find below review comments.- While creating new Domain and clicking on SQL tab, python side we are getting error saying "TypeError: 'bool' object is not callable".We are not able to create any domain. Fix this issue so that we can test other functionality.- Implement the reverse engineering SQL generation for the domain node.- As per the checklist, remove the "Use Slony" from Constraints tab, as it is not required.- No need to pass "qtIdent=self.qtIdent" as function argument in "create" and "getSQL" function in domains/__init__.py- In "Security" tab , provider and security label fields are not editable.- In PG version 9.1, when we update the existing domain name then "ALTER DOMAIN" is not supported.Currently there is no checking for the PG version 9.1 and 9.2_plus. It will fail when we connect to database 9.1e.g.For PG version 9.1 - Update command should be as below.ALTER TYPE xyz RENAME TO abc;For PG version 9.2 onwards - Update command should be as below.ALTER DOMAIN xyz RENAME TO abc;- Some of the SQL file, qtIdent is not used. Please check all the related SQL files.e.g. - In update.sql file "data.owner" should be "conn|qtIdent(data.owner)"{% if data.owner %}ALTER DOMAIN {{ conn|qtIdent(o_data.basensp, name) }}OWNER TO {{ data.owner }};{% endif %}Let us know for any issues.Thanks,Neel PatelOn Wed, Jan 20, 2016 at 2:50 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:KhushbooThanks,Hi Neel,Please find updated patch.On Wed, Jan 20, 2016 at 12:50 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:Hi Khushboo,While applying the patch file, we are getting below warnings.#########################################domains (1).patch:1340: trailing whitespace.oid: undefined,domains (1).patch:1483: trailing whitespace.(nspname = 'pg_catalog' AND EXISTSdomains (1).patch:1487: trailing whitespace.OR (nspname = 'information_schema' AND EXISTSdomains (1).patch:1489: trailing whitespace.OR (nspname LIKE '_%' AND EXISTSdomains (1).patch:1642: trailing whitespace.(select 1 from pg_class where relnamespace=typnamespace and relname = typname and relkind != 'c') AND (typname not like '_%' OR NOT EXISTS (select 1 from pg_class where relnamespace=typnamespace and relname = substring(typname from 2)::name and relkind != 'c'))warning: squelched 4 whitespace errorswarning: 9 lines add whitespace errors.#########################################Can you please remove the whitespace and regenerate the patch ?Thanks,Neel PatelOn Wed, Jan 20, 2016 at 12:37 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:--Resending patch with binary option.On Wed, Jan 20, 2016 at 10:18 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:KhushbooThe patch will be modified after Types module implementation as we need to populate Base Type and some Type related validations from the Types module.Hi,Please find attached patch for the Domain Module.Please review it and let me know the feedback.Thanks,
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Вложения
Hi Khushboo,
Please find below review comments.
- Reverse engineering SQL generation is not implemented for domain node.
- "Length" and "Precision" fields should be enabled/disabled based on the selection of "Base Type" value.
- Query is not getting generated properly. Some of the parameters are not reflected in query. As we have provided Length and Precision value in
below numeric base type. Also do proper indentation in generated query.
Wrong Query :-
CREATE DOMAIN my_schema.test_123
AS "numeric"
DEFAULT 5
;
Correct Query :-
CREATE DOMAIN my_schema.test_123
AS numeric(22,4)
DEFAULT 5
NOT NULL;
- After creation of new domain with base type "aclitem" , wrong "Length" field value is getting displayed.
- We are getting error saying "TypeError: the JSON object must be str, not 'dict'" when we add constraint with "NOT VALID" and NO INHERIT.
- We should add property "System Domain?" when we select any domain node.
- We think for creation of Security Label, we should include the schema name along with domain name.
Wrong Query :-
SECURITY LABEL FOR pv_label ON DOMAIN test_123 IS 'label_val';
Correct Query :-
SECURITY LABEL FOR pv_label ON DOMAIN <schema_name>.test_123 IS 'label_val';
Let us know in case of any issues.
Thanks,
Neel Patel
On Tue, Feb 2, 2016 at 3:51 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
KhushbooThanks,Please find attached patch for the same.I have modified the code as per your suggestions and also fixed some of the issues got while doing unit testing.Hi Neel,Thanks for reviewing my patch.On Wed, Jan 20, 2016 at 10:56 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:Hi Khushboo,Please find below review comments.- While creating new Domain and clicking on SQL tab, python side we are getting error saying "TypeError: 'bool' object is not callable".We are not able to create any domain. Fix this issue so that we can test other functionality.- Implement the reverse engineering SQL generation for the domain node.- As per the checklist, remove the "Use Slony" from Constraints tab, as it is not required.- No need to pass "qtIdent=self.qtIdent" as function argument in "create" and "getSQL" function in domains/__init__.py- In "Security" tab , provider and security label fields are not editable.- In PG version 9.1, when we update the existing domain name then "ALTER DOMAIN" is not supported.Currently there is no checking for the PG version 9.1 and 9.2_plus. It will fail when we connect to database 9.1e.g.For PG version 9.1 - Update command should be as below.ALTER TYPE xyz RENAME TO abc;For PG version 9.2 onwards - Update command should be as below.ALTER DOMAIN xyz RENAME TO abc;- Some of the SQL file, qtIdent is not used. Please check all the related SQL files.e.g. - In update.sql file "data.owner" should be "conn|qtIdent(data.owner)"{% if data.owner %}ALTER DOMAIN {{ conn|qtIdent(o_data.basensp, name) }}OWNER TO {{ data.owner }};{% endif %}Let us know for any issues.Thanks,Neel PatelOn Wed, Jan 20, 2016 at 2:50 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:KhushbooThanks,Hi Neel,Please find updated patch.On Wed, Jan 20, 2016 at 12:50 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:Hi Khushboo,While applying the patch file, we are getting below warnings.#########################################domains (1).patch:1340: trailing whitespace.oid: undefined,domains (1).patch:1483: trailing whitespace.(nspname = 'pg_catalog' AND EXISTSdomains (1).patch:1487: trailing whitespace.OR (nspname = 'information_schema' AND EXISTSdomains (1).patch:1489: trailing whitespace.OR (nspname LIKE '_%' AND EXISTSdomains (1).patch:1642: trailing whitespace.(select 1 from pg_class where relnamespace=typnamespace and relname = typname and relkind != 'c') AND (typname not like '_%' OR NOT EXISTS (select 1 from pg_class where relnamespace=typnamespace and relname = substring(typname from 2)::name and relkind != 'c'))warning: squelched 4 whitespace errorswarning: 9 lines add whitespace errors.#########################################Can you please remove the whitespace and regenerate the patch ?Thanks,Neel PatelOn Wed, Jan 20, 2016 at 12:37 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:--Resending patch with binary option.On Wed, Jan 20, 2016 at 10:18 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:KhushbooThe patch will be modified after Types module implementation as we need to populate Base Type and some Type related validations from the Types module.Hi,Please find attached patch for the Domain Module.Please review it and let me know the feedback.Thanks,
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Hi,
Please find attached Revised patch for the Domain module and also my comments inline as below.
Thanks,Please find attached Revised patch for the Domain module and also my comments inline as below.
On Wed, Feb 3, 2016 at 4:22 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:
Hi Khushboo,Please find below review comments.- Reverse engineering SQL generation is not implemented for domain node.
Done
- "Length" and "Precision" fields should be enabled/disabled based on the selection of "Base Type" value.
This implementation is dependent on the 'Type' module. Once that will be done, I will merge my code.
- Query is not getting generated properly. Some of the parameters are not reflected in query. As we have provided Length and Precision value inbelow numeric base type. Also do proper indentation in generated query.Wrong Query :-CREATE DOMAIN my_schema.test_123AS "numeric"DEFAULT 5;Correct Query :-CREATE DOMAIN my_schema.test_123AS numeric(22,4)DEFAULT 5NOT NULL;
Done
- After creation of new domain with base type "aclitem" , wrong "Length" field value is getting displayed.
Done
- We are getting error saying "TypeError: the JSON object must be str, not 'dict'" when we add constraint w
ith "NOT VALID" and NO INHERIT.
Done
- We should add property "System Domain?" when we select any domain node.
Done
- We think for creation of Security Label, we should include the schema name along with domain name.Wrong Query :-SECURITY LABEL FOR pv_label ON DOMAIN test_123 IS 'label_val';Correct Query :-SECURITY LABEL FOR pv_label ON DOMAIN <schema_name>.test_123 IS 'label_val';
Done
Let us know in case of any issues.Thanks,Neel PatelOn Tue, Feb 2, 2016 at 3:51 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:KhushbooThanks,Please find attached patch for the same.I have modified the code as per your suggestions and also fixed some of the issues got while doing unit testing.Hi Neel,Thanks for reviewing my patch.On Wed, Jan 20, 2016 at 10:56 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:Hi Khushboo,Please find below review comments.- While creating new Domain and clicking on SQL tab, python side we are getting error saying "TypeError: 'bool' object is not callable".We are not able to create any domain. Fix this issue so that we can test other functionality.- Implement the reverse engineering SQL generation for the domain node.- As per the checklist, remove the "Use Slony" from Constraints tab, as it is not required.- No need to pass "qtIdent=self.qtIdent" as function argument in "create" and "getSQL" function in domains/__init__.py- In "Security" tab , provider and security label fields are not editable.- In PG version 9.1, when we update the existing domain name then "ALTER DOMAIN" is not supported.Currently there is no checking for the PG version 9.1 and 9.2_plus. It will fail when we connect to database 9.1e.g.For PG version 9.1 - Update command should be as below.ALTER TYPE xyz RENAME TO abc;For PG version 9.2 onwards - Update command should be as below.ALTER DOMAIN xyz RENAME TO abc;- Some of the SQL file, qtIdent is not used. Please check all the related SQL files.e.g. - In update.sql file "data.owner" should be "conn|qtIdent(data.owner)"{% if data.owner %}ALTER DOMAIN {{ conn|qtIdent(o_data.basensp, name) }}OWNER TO {{ data.owner }};{% endif %}Let us know for any issues.Thanks,Neel PatelOn Wed, Jan 20, 2016 at 2:50 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:KhushbooThanks,Hi Neel,Please find updated patch.On Wed, Jan 20, 2016 at 12:50 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:Hi Khushboo,While applying the patch file, we are getting below warnings.#########################################domains (1).patch:1340: trailing whitespace.oid: undefined,domains (1).patch:1483: trailing whitespace.(nspname = 'pg_catalog' AND EXISTSdomains (1).patch:1487: trailing whitespace.OR (nspname = 'information_schema' AND EXISTSdomains (1).patch:1489: trailing whitespace.OR (nspname LIKE '_%' AND EXISTSdomains (1).patch:1642: trailing whitespace.(select 1 from pg_class where relnamespace=typnamespace and relname = typname and relkind != 'c') AND (typname not like '_%' OR NOT EXISTS (select 1 from pg_class where relnamespace=typnamespace and relname = substring(typname from 2)::name and relkind != 'c'))warning: squelched 4 whitespace errorswarning: 9 lines add whitespace errors.#########################################Can you please remove the whitespace and regenerate the patch ?Thanks,Neel PatelOn Wed, Jan 20, 2016 at 12:37 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:--Resending patch with binary option.On Wed, Jan 20, 2016 at 10:18 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:KhushbooThe patch will be modified after Types module implementation as we need to populate Base Type and some Type related validations from the Types module.Hi,Please find attached patch for the Domain Module.Please review it and let me know the feedback.Thanks,
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Вложения
Hi,
I have updated the Domain module as below:- Used 'NodeByListControl' to get schemas, in domains.js file as suggested by Ashesh to avoid code redundancy.
- Applied 'Security Label Macro' Patch (Implemented by Harshal) and removed same changes from the Domain Patch. To test Domain patch, 'Security Label Macro' patch must be applied first as that is not committed yet.
Please find attached Domain Module Patch.
Thanks,
Khushboo
On Tue, Feb 23, 2016 at 12:37 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
KhushbooHi,Thanks,
Please find attached Revised patch for the Domain module and also my comments inline as below.On Wed, Feb 3, 2016 at 4:22 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:Hi Khushboo,Please find below review comments.- Reverse engineering SQL generation is not implemented for domain node.Done- "Length" and "Precision" fields should be enabled/disabled based on the selection of "Base Type" value.This implementation is dependent on the 'Type' module. Once that will be done, I will merge my code.- Query is not getting generated properly. Some of the parameters are not reflected in query. As we have provided Length and Precision value inbelow numeric base type. Also do proper indentation in generated query.Wrong Query :-CREATE DOMAIN my_schema.test_123AS "numeric"DEFAULT 5;Correct Query :-CREATE DOMAIN my_schema.test_123AS numeric(22,4)DEFAULT 5NOT NULL;Done- After creation of new domain with base type "aclitem" , wrong "Length" field value is getting displayed.Done- We are getting error saying "TypeError: the JSON object must be str, not 'dict'" when we add constraint with "NOT VALID" and NO INHERIT.Done- We should add property "System Domain?" when we select any domain node.Done- We think for creation of Security Label, we should include the schema name along with domain name.Wrong Query :-SECURITY LABEL FOR pv_label ON DOMAIN test_123 IS 'label_val';Correct Query :-SECURITY LABEL FOR pv_label ON DOMAIN <schema_name>.test_123 IS 'label_val';DoneLet us know in case of any issues.Thanks,Neel PatelOn Tue, Feb 2, 2016 at 3:51 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:KhushbooThanks,Please find attached patch for the same.I have modified the code as per your suggestions and also fixed some of the issues got while doing unit testing.Hi Neel,Thanks for reviewing my patch.On Wed, Jan 20, 2016 at 10:56 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:Hi Khushboo,Please find below review comments.- While creating new Domain and clicking on SQL tab, python side we are getting error saying "TypeError: 'bool' object is not callable".We are not able to create any domain. Fix this issue so that we can test other functionality.- Implement the reverse engineering SQL generation for the domain node.- As per the checklist, remove the "Use Slony" from Constraints tab, as it is not required.- No need to pass "qtIdent=self.qtIdent" as function argument in "create" and "getSQL" function in domains/__init__.py- In "Security" tab , provider and security label fields are not editable.- In PG version 9.1, when we update the existing domain name then "ALTER DOMAIN" is not supported.Currently there is no checking for the PG version 9.1 and 9.2_plus. It will fail when we connect to database 9.1e.g.For PG version 9.1 - Update command should be as below.ALTER TYPE xyz RENAME TO abc;For PG version 9.2 onwards - Update command should be as below.ALTER DOMAIN xyz RENAME TO abc;- Some of the SQL file, qtIdent is not used. Please check all the related SQL files.e.g. - In update.sql file "data.owner" should be "conn|qtIdent(data.owner)"{% if data.owner %}ALTER DOMAIN {{ conn|qtIdent(o_data.basensp, name) }}OWNER TO {{ data.owner }};{% endif %}Let us know for any issues.Thanks,Neel PatelOn Wed, Jan 20, 2016 at 2:50 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:KhushbooThanks,Hi Neel,Please find updated patch.On Wed, Jan 20, 2016 at 12:50 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:Hi Khushboo,While applying the patch file, we are getting below warnings.#########################################domains (1).patch:1340: trailing whitespace.oid: undefined,domains (1).patch:1483: trailing whitespace.(nspname = 'pg_catalog' AND EXISTSdomains (1).patch:1487: trailing whitespace.OR (nspname = 'information_schema' AND EXISTSdomains (1).patch:1489: trailing whitespace.OR (nspname LIKE '_%' AND EXISTSdomains (1).patch:1642: trailing whitespace.(select 1 from pg_class where relnamespace=typnamespace and relname = typname and relkind != 'c') AND (typname not like '_%' OR NOT EXISTS (select 1 from pg_class where relnamespace=typnamespace and relname = substring(typname from 2)::name and relkind != 'c'))warning: squelched 4 whitespace errorswarning: 9 lines add whitespace errors.#########################################Can you please remove the whitespace and regenerate the patch ?Thanks,Neel PatelOn Wed, Jan 20, 2016 at 12:37 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:--Resending patch with binary option.On Wed, Jan 20, 2016 at 10:18 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:KhushbooThe patch will be modified after Types module implementation as we need to populate Base Type and some Type related validations from the Types module.Hi,Please find attached patch for the Domain Module.Please review it and let me know the feedback.Thanks,
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Вложения
Hi
--
On Wed, Feb 24, 2016 at 9:24 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi,I have updated the Domain module as below:- Used 'NodeByListControl' to get schemas, in domains.js file as suggested by Ashesh to avoid code redundancy.- Applied 'Security Label Macro' Patch (Implemented by Harshal) and removed same changes from the Domain Patch.To test Domain patch, 'Security Label Macro' patch must be applied first as that is not committed yet.Please find attached Domain Module Patch.
Initial feedback:
- Owner and schema should be allowed to be left blank (and then default to the current user/schema)
- Length and Precision fields should only be enabled if appropriate for the data type.
- SQL generation for new Domains doesn't work:
- When adding constraints, I should be able to type directly into the grid. Expanding the row should be optional.
- The comment column on the constraints grid expands when the text reaches ~50% of the width. It should be a fixed size (and use 100% of the space available, less appropriate margins).
- Backend support checks should not special-case Slony schemas.
- 4 character indentation not used consistently in SQL templates.
- Error seen when saving a domain: "macros/schemas/security.macros"
016-02-25 11:55:10,728: INFO werkzeug: 127.0.0.1 - - [25/Feb/2016 11:55:10] "GET /browser/domain/msql/1/1/24587/2200/?name=email&owner=postgres&basensp=public&description=This+is+an+email+data+type&basetype=text&typlen=&precision=&typdefault=&typnotnull=true&collname=&constraints=%5B%5D&seclabels=%5B%5D&_=1456401124386 HTTP/1.1" 500 -
Traceback (most recent call last):
File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1836, in __call__
return self.wsgi_app(environ, start_response)
File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1820, in wsgi_app
response = self.make_response(self.handle_exception(e))
File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1403, in handle_exception
reraise(exc_type, exc_value, tb)
File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1817, in wsgi_app
response = self.full_dispatch_request()
File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1477, in full_dispatch_request
rv = self.handle_user_exception(e)
File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1381, in handle_user_exception
reraise(exc_type, exc_value, tb)
File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1475, in full_dispatch_request
rv = self.dispatch_request()
File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1461, in dispatch_request
return self.view_functions[rule.endpoint](**req.view_args)
File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/views.py", line 84, in view
return self.dispatch_request(*args, **kwargs)
File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/utils.py", line 248, in dispatch_request
return method(*args, **kwargs)
File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/__init__.py", line 277, in wrap
return f(*args, **kwargs)
File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/__init__.py", line 232, in wrap
return f(self, **kwargs)
File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/__init__.py", line 700, in msql
status=200
File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/ajax.py", line 41, in make_json_response
response=json.dumps(doc, cls=DataTypeJSONEncoder),
File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/__init__.py", line 386, in dumps
**kw).encode(obj)
File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/encoder.py", line 269, in encode
chunks = self.iterencode(o, _one_shot=True)
File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/encoder.py", line 348, in iterencode
return _iterencode(o, 0)
File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/ajax.py", line 26, in default
return json.JSONEncoder.default(self, obj)
File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/encoder.py", line 246, in default
raise TypeError(repr(o) + " is not JSON serializable")
TypeError: TemplateNotFound() is not JSON serializable
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Per discussion with Khushboo, the patch at http://www.postgresql.org/message-id/attachment/41939/schemas_macros_10_Feb_2.patch is a pre-req for this. Updated comments below...
--
On Thu, Feb 25, 2016 at 12:13 PM, Dave Page <dpage@pgadmin.org> wrote:
HiOn Wed, Feb 24, 2016 at 9:24 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi,I have updated the Domain module as below:- Used 'NodeByListControl' to get schemas, in domains.js file as suggested by Ashesh to avoid code redundancy.- Applied 'Security Label Macro' Patch (Implemented by Harshal) and removed same changes from the Domain Patch.To test Domain patch, 'Security Label Macro' patch must be applied first as that is not committed yet.Please find attached Domain Module Patch.Initial feedback:- Owner and schema should be allowed to be left blank (and then default to the current user/schema)- Length and Precision fields should only be enabled if appropriate for the data type.
The above still apply.
- SQL generation for new Domains doesn't work:
This now works.
- When adding constraints, I should be able to type directly into the grid. Expanding the row should be optional.- The comment column on the constraints grid expands when the text reaches ~50% of the width. It should be a fixed size (and use 100% of the space available, less appropriate margins).- Backend support checks should not special-case Slony schemas.- 4 character indentation not used consistently in SQL templates.
These still apply.
- Error seen when saving a domain: "macros/schemas/security.macros"016-02-25 11:55:10,728: INFO werkzeug: 127.0.0.1 - - [25/Feb/2016 11:55:10] "GET /browser/domain/msql/1/1/24587/2200/?name=email&owner=postgres&basensp=public&description=This+is+an+email+data+type&basetype=text&typlen=&precision=&typdefault=&typnotnull=true&collname=&constraints=%5B%5D&seclabels=%5B%5D&_=1456401124386 HTTP/1.1" 500 -Traceback (most recent call last):File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1836, in __call__return self.wsgi_app(environ, start_response)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1820, in wsgi_appresponse = self.make_response(self.handle_exception(e))File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1403, in handle_exceptionreraise(exc_type, exc_value, tb)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1817, in wsgi_appresponse = self.full_dispatch_request()File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1477, in full_dispatch_requestrv = self.handle_user_exception(e)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1381, in handle_user_exceptionreraise(exc_type, exc_value, tb)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1475, in full_dispatch_requestrv = self.dispatch_request()File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1461, in dispatch_requestreturn self.view_functions[rule.endpoint](**req.view_args)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/views.py", line 84, in viewreturn self.dispatch_request(*args, **kwargs)File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/utils.py", line 248, in dispatch_requestreturn method(*args, **kwargs)File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/__init__.py", line 277, in wrapreturn f(*args, **kwargs)File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/__init__.py", line 232, in wrapreturn f(self, **kwargs)File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/__init__.py", line 700, in msqlstatus=200File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/ajax.py", line 41, in make_json_responseresponse=json.dumps(doc, cls=DataTypeJSONEncoder),File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/__init__.py", line 386, in dumps**kw).encode(obj)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/encoder.py", line 269, in encodechunks = self.iterencode(o, _one_shot=True)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/encoder.py", line 348, in iterencodereturn _iterencode(o, 0)File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/ajax.py", line 26, in defaultreturn json.JSONEncoder.default(self, obj)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/encoder.py", line 246, in defaultraise TypeError(repr(o) + " is not JSON serializable")TypeError: TemplateNotFound() is not JSON serializable
This issue is resolved.
Additional issues:
- We can add a comment to constraints (and view them), however they are not saved.
- The domain is not created as a single SQL statement, but by creating a domain over the base type, then adding constraints. Can this be done in one query?
- Reverse engineered SQL doesn't include the normal header and commented-out drop statement.
Thanks.
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi,
Please find the updated patch for the Domain Module.To test this, the Data-type Reader patch needs to be applied first.
Also, please find in-line comments below.
Thanks,
Khushboo
On Thu, Feb 25, 2016 at 7:00 PM, Dave Page <dpage@pgadmin.org> wrote:
Per discussion with Khushboo, the patch at http://www.postgresql.org/message-id/attachment/41939/schemas_macros_10_Feb_2.patch is a pre-req for this. Updated comments below...On Thu, Feb 25, 2016 at 12:13 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, Feb 24, 2016 at 9:24 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi,I have updated the Domain module as below:- Used 'NodeByListControl' to get schemas, in domains.js file as suggested by Ashesh to avoid code redundancy.- Applied 'Security Label Macro' Patch (Implemented by Harshal) and removed same changes from the Domain Patch.To test Domain patch, 'Security Label Macro' patch must be applied first as that is not committed yet.Please find attached Domain Module Patch.Initial feedback:- Owner and schema should be allowed to be left blank (and then default to the current user/schema)
Done
- Length and Precision fields should only be enabled if appropriate for the data type.
Done
The above still apply.- SQL generation for new Domains doesn't work:This now works.- When adding constraints, I should be able to type directly into the grid. Expanding the row should be optional.
I have made the grid non-editable explicitly as the Check constraint control is multi-line control and right now there is no support in the grid for the multi-line control.
- The comment column on the constraints grid expands when the text reaches ~50% of the width. It should be a fixed size (and use 100% of the space available, less appropriate margins)
I have applied the size for the each header of the grid, but if the given input will be without space in the grid then it will expand. For this, we can make table layout fixed. So, please suggest, should I do that or not?
- Backend support checks should not special-case Slony schemas.
Done
- 4 character indentation not used consistently in SQL templates.
Done
These still apply.- Error seen when saving a domain: "macros/schemas/security.macros"016-02-25 11:55:10,728: INFO werkzeug: 127.0.0.1 - - [25/Feb/2016 11:55:10] "GET /browser/domain/msql/1/1/24587/2200/?name=email&owner=postgres&basensp=public&description=This+is+an+email+data+type&basetype=text&typlen=&precision=&typdefault=&typnotnull=true&collname=&constraints=%5B%5D&seclabels=%5B%5D&_=1456401124386 HTTP/1.1" 500 -Traceback (most recent call last):File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1836, in __call__return self.wsgi_app(environ, start_response)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1820, in wsgi_appresponse = self.make_response(self.handle_exception(e))File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1403, in handle_exceptionreraise(exc_type, exc_value, tb)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1817, in wsgi_appresponse = self.full_dispatch_request()File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1477, in full_dispatch_requestrv = self.handle_user_exception(e)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1381, in handle_user_exceptionreraise(exc_type, exc_value, tb)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1475, in full_dispatch_requestrv = self.dispatch_request()File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py", line 1461, in dispatch_requestreturn self.view_functions[rule.endpoint](**req.view_args)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/views.py", line 84, in viewreturn self.dispatch_request(*args, **kwargs)File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/utils.py", line 248, in dispatch_requestreturn method(*args, **kwargs)File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/__init__.py", line 277, in wrapreturn f(*args, **kwargs)File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/__init__.py", line 232, in wrapreturn f(self, **kwargs)File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/__init__.py", line 700, in msqlstatus=200File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/ajax.py", line 41, in make_json_responseresponse=json.dumps(doc, cls=DataTypeJSONEncoder),File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/__init__.py", line 386, in dumps**kw).encode(obj)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/encoder.py", line 269, in encodechunks = self.iterencode(o, _one_shot=True)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/encoder.py", line 348, in iterencodereturn _iterencode(o, 0)File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/ajax.py", line 26, in defaultreturn json.JSONEncoder.default(self, obj)File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/encoder.py", line 246, in defaultraise TypeError(repr(o) + " is not JSON serializable")TypeError: TemplateNotFound() is not JSON serializableThis issue is resolved.Additional issues:- We can add a comment to constraints (and view them), however they are not saved.
Done
- The domain is not created as a single SQL statement, but by creating a domain over the base type, then adding constraints. Can this be done in one query?
Done
- Reverse engineered SQL doesn't include the normal header and commented-out drop statement.
Done
Thanks.--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения
Hi On Wed, Mar 16, 2016 at 9:18 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: > >>> - Owner and schema should be allowed to be left blank (and then default >>> to the current user/schema) > > Done Oh, sorry - that design changed a while back, and I've updated all the existing nodes already. I thought I'd mentioned that. All we do now is pre-set the default values for those two fields. >>> - When adding constraints, I should be able to type directly into the >>> grid. Expanding the row should be optional. > > I have made the grid non-editable explicitly as the Check constraint control > is multi-line control and right now there is no support in the grid for the > multi-line control. Not sure I follow - the mockup design you sent months ago allowed you to type into the grid, and expand a row to show all fields if you wanted. That is an *absolutely essential* feature enhancement for pgAdmin 4 - it's required by the table design (though this will change a little in other ways, like positioning of the expand row button), and should be used here: https://www.lucidchart.com/documents/edit/610ce42d-c397-48ff-a5e7-bd92c4995715/0 >>> - The comment column on the constraints grid expands when the text >>> reaches ~50% of the width. It should be a fixed size (and use 100% of the >>> space available, less appropriate margins) > > I have applied the size for the each header of the grid, but if the given > input will be without space in the grid then it will expand. For this, we > can make table layout fixed. So, please suggest, should I do that or not? Yes, I think it should be fixed. If the grid row is expanded, presumably it'll show in a multi-line field anyway? Plus the properties will use a multi-line field as well. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hi,
On Wed, Mar 16, 2016 at 2:55 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi
On Wed, Mar 16, 2016 at 9:18 AM, Khushboo Vashi
<khushboo.vashi@enterprisedb.com> wrote:
>
>>> - Owner and schema should be allowed to be left blank (and then default
>>> to the current user/schema)
>
> Done
Oh, sorry - that design changed a while back, and I've updated all the
existing nodes already. I thought I'd mentioned that. All we do now is
pre-set the default values for those two fields.
I have done in this way only. Sorry for the misunderstanding.
>>> - When adding constraints, I should be able to type directly into the
>>> grid. Expanding the row should be optional.
>
> I have made the grid non-editable explicitly as the Check constraint control
> is multi-line control and right now there is no support in the grid for the
> multi-line control.
Not sure I follow - the mockup design you sent months ago allowed you
to type into the grid, and expand a row to show all fields if you
wanted. That is an *absolutely essential* feature enhancement for
pgAdmin 4 - it's required by the table design (though this will change
a little in other ways, like positioning of the expand row button),
and should be used here:
https://www.lucidchart.com/documents/edit/610ce42d-c397-48ff-a5e7-bd92c4995715/0
All other controls other than text-area are supported in back-grid.
I will try to incorporate text-area as well, so we can directly type into the grid for this control also.
>>> - The comment column on the constraints grid expands when the text
>>> reaches ~50% of the width. It should be a fixed size (and use 100% of the
>>> space available, less appropriate margins)
>
> I have applied the size for the each header of the grid, but if the given
> input will be without space in the grid then it will expand. For this, we
> can make table layout fixed. So, please suggest, should I do that or not?
Yes, I think it should be fixed. If the grid row is expanded,
presumably it'll show in a multi-line field anyway? Plus the
properties will use a multi-line field as well.
Okay.
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi,
Please find the updated Domain Module Patch.On Wed, Mar 16, 2016 at 3:02 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi,On Wed, Mar 16, 2016 at 2:55 PM, Dave Page <dpage@pgadmin.org> wrote:Hi
On Wed, Mar 16, 2016 at 9:18 AM, Khushboo Vashi
<khushboo.vashi@enterprisedb.com> wrote:
>
>>> - Owner and schema should be allowed to be left blank (and then default
>>> to the current user/schema)
>
> Done
Oh, sorry - that design changed a while back, and I've updated all the
existing nodes already. I thought I'd mentioned that. All we do now is
pre-set the default values for those two fields.I have done in this way only. Sorry for the misunderstanding.>>> - When adding constraints, I should be able to type directly into the
>>> grid. Expanding the row should be optional.
>
> I have made the grid non-editable explicitly as the Check constraint control
> is multi-line control and right now there is no support in the grid for the
> multi-line control.
Not sure I follow - the mockup design you sent months ago allowed you
to type into the grid, and expand a row to show all fields if you
wanted. That is an *absolutely essential* feature enhancement for
pgAdmin 4 - it's required by the table design (though this will change
a little in other ways, like positioning of the expand row button),
and should be used here:
https://www.lucidchart.com/documents/edit/610ce42d-c397-48ff-a5e7-bd92c4995715/0All other controls other than text-area are supported in back-grid.I will try to incorporate text-area as well, so we can directly type into the grid for this control also.
>>> - The comment column on the constraints grid expands when the text
>>> reaches ~50% of the width. It should be a fixed size (and use 100% of the
>>> space available, less appropriate margins)
>
> I have applied the size for the each header of the grid, but if the given
> input will be without space in the grid then it will expand. For this, we
> can make table layout fixed. So, please suggest, should I do that or not?
Yes, I think it should be fixed. If the grid row is expanded,
presumably it'll show in a multi-line field anyway? Plus the
properties will use a multi-line field as well.Okay.--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения
Hi On Wed, Mar 16, 2016 at 2:03 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: > Hi, > > Please find the updated Domain Module Patch. > > To test this patch, please apply Backgrid Textarea Cell Patch before this. Thanks. I believe with the following fixes, we'll be done :-) - Default values should be auto-quoted when necessary (ie. strings, on a text-based domain). - "System Domain?" should be in the General section, between owner and comment. - The switches should use the same colouring/styling as other objects, e.g. options: { 'onText': 'Yes', 'offText': 'No', 'onColor': 'success', 'offColor': 'primary', 'size': 'small' } - Please remove the Schema property from the main properties tab (not the properties dialogue). - No icon is show for Checks on the Dependents tab for a domain. - The add button on the Security Labels tab is spelt "Add". Why is that? Other instances of this grid use "ADD" which is the default in backform.pgadmin.js. - Dependencies on domain check constraints are listed as being on a "Type" not a "Domain". - If adding a domain constraint using the grid on the Domain dialogue, I cannot specify "NOT VALID". We need a checkbox for that in a narrow columns at the end. Unchecking it for an existing constraint should be the equivalent of doing "ALTER DOMAIN ... VALIDATE CONSTRAINT" - If I switch the "Don't Validate" switch on a constraint, there are leading blank lines in the generated SQL. The same occurs when adding a comment to a constraint. - I think we need to reverse the meaning of "Don't Validate" and rename to match the "Valid?" field that's on the properties list. Otherwise it's not clear they're the same thing. - s/Not Null/Not Null?/ -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hi Dave,
I have a query regarding your below feedback :- Default values should be auto-quoted when necessary (ie. strings, on a text-based domain).
On Wed, Mar 16, 2016 at 9:40 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi
On Wed, Mar 16, 2016 at 2:03 PM, Khushboo Vashi
<khushboo.vashi@enterprisedb.com> wrote:
> Hi,
>
> Please find the updated Domain Module Patch.
>
> To test this patch, please apply Backgrid Textarea Cell Patch before this.
Thanks. I believe with the following fixes, we'll be done :-)
- Default values should be auto-quoted when necessary (ie. strings, on
a text-based domain).
- "System Domain?" should be in the General section, between owner and comment.
- The switches should use the same colouring/styling as other objects, e.g.
options: {
'onText': 'Yes', 'offText': 'No',
'onColor': 'success', 'offColor': 'primary',
'size': 'small'
}
- Please remove the Schema property from the main properties tab (not
the properties dialogue).
- No icon is show for Checks on the Dependents tab for a domain.
- The add button on the Security Labels tab is spelt "Add". Why is
that? Other instances of this grid use "ADD" which is the default in
backform.pgadmin.js.
- Dependencies on domain check constraints are listed as being on a
"Type" not a "Domain".
- If adding a domain constraint using the grid on the Domain dialogue,
I cannot specify "NOT VALID". We need a checkbox for that in a narrow
columns at the end. Unchecking it for an existing constraint should be
the equivalent of doing "ALTER DOMAIN ... VALIDATE CONSTRAINT"
- If I switch the "Don't Validate" switch on a constraint, there are
leading blank lines in the generated SQL. The same occurs when adding
a comment to a constraint.
- I think we need to reverse the meaning of "Don't Validate" and
rename to match the "Valid?" field that's on the properties list.
Otherwise it's not clear they're the same thing.
- s/Not Null/Not Null?/
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Thu, Mar 17, 2016 at 5:39 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: > Hi Dave, > > I have a query regarding your below feedback : > > - Default values should be auto-quoted when necessary (ie. strings, on a > text-based domain). > > To resolve this, I have checked the typcategory field from pg_type for the > base_type selected for the Domain. > If the typcategory is String type (i.e. S), then only I have used qtLiteral > function to quote the default value. > > Is this right approach or not? Yes, I think that's a good approach (at least, I don't see any downsides right now :-) ) -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hi Dave,
On Fri, Mar 18, 2016 at 8:31 PM, Dave Page <dpage@pgadmin.org> wrote:
I have tried pg_get_expr(typdefaultbin, 0) to fetch only default value (without datatype), but it returns the whole expression.
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Thu, Mar 17, 2016 at 5:39 PM, Khushboo Vashi
<khushboo.vashi@enterprisedb.com> wrote:
> Hi Dave,
>
> I have a query regarding your below feedback :
>
> - Default values should be auto-quoted when necessary (ie. strings, on a
> text-based domain).
>
> To resolve this, I have checked the typcategory field from pg_type for the
> base_type selected for the Domain.
> If the typcategory is String type (i.e. S), then only I have used qtLiteral
> function to quote the default value.
>
> Is this right approach or not?
Yes, I think that's a good approach (at least, I don't see any
downsides right now :-) )
After implementing above approach, I found some issues:
1> If I put quotes explicitly, user can not set any expression as a default value
For example,
CREATE OR REPLACE FUNCTION test_text_return() RETURNS TEXT AS
$$
SELECT now()::text;
$$ LANGUAGE 'sql';
CREATE DOMAIN text_domin AS TEXT DEFAULT public.test_text_return();
$$
SELECT now()::text;
$$ LANGUAGE 'sql';
CREATE DOMAIN text_domin AS TEXT DEFAULT public.test_text_return();
In this case, if I put quotes for default value, it will be set as a string rather than an expression.
2> When I set any string for the default value, it is getting stored with the datatype like 'test_default'::text
So, in the Default Value (Properties Dialogue), it is showing 'test_default'::text
I have tried pg_get_expr(typdefaultbin, 0) to fetch only default value (without datatype), but it returns the whole expression.
As per my discussion with Ashesh, we feel - we should not quote the default value after looking at the above example.
Please, let me know what should I do further?
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Mon, Mar 21, 2016 at 7:41 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: > Hi Dave, > > > On Fri, Mar 18, 2016 at 8:31 PM, Dave Page <dpage@pgadmin.org> wrote: >> >> On Thu, Mar 17, 2016 at 5:39 PM, Khushboo Vashi >> <khushboo.vashi@enterprisedb.com> wrote: >> > Hi Dave, >> > >> > I have a query regarding your below feedback : >> > >> > - Default values should be auto-quoted when necessary (ie. strings, on a >> > text-based domain). >> > >> > To resolve this, I have checked the typcategory field from pg_type for >> > the >> > base_type selected for the Domain. >> > If the typcategory is String type (i.e. S), then only I have used >> > qtLiteral >> > function to quote the default value. >> > >> > Is this right approach or not? >> >> Yes, I think that's a good approach (at least, I don't see any >> downsides right now :-) ) >> > > After implementing above approach, I found some issues: > > 1> If I put quotes explicitly, user can not set any expression as a default > value > For example, > > CREATE OR REPLACE FUNCTION test_text_return() RETURNS TEXT AS > $$ > SELECT now()::text; > $$ LANGUAGE 'sql'; > > > CREATE DOMAIN text_domin AS TEXT DEFAULT public.test_text_return(); > > In this case, if I put quotes for default value, it will be set as a > string rather than an expression. > > > 2> When I set any string for the default value, it is getting stored with > the datatype like 'test_default'::text > So, in the Default Value (Properties Dialogue), it is showing > 'test_default'::text > > I have tried pg_get_expr(typdefaultbin, 0) to fetch only default value > (without datatype), but it returns the whole expression. > > > As per my discussion with Ashesh, we feel - we should not quote the default > value after looking at the above example. > > Please, let me know what should I do further? Urgh, yes - I hadn't thought of that. Leave it unquoted please. I guess we can add a hint to the field: Enter an expression or value. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hi,
Please find attached updated patch for the Domains module.On Wed, Mar 16, 2016 at 9:40 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi
On Wed, Mar 16, 2016 at 2:03 PM, Khushboo Vashi
<khushboo.vashi@enterprisedb.com> wrote:
> Hi,
>
> Please find the updated Domain Module Patch.
>
> To test this patch, please apply Backgrid Textarea Cell Patch before this.
Thanks. I believe with the following fixes, we'll be done :-)
- Default values should be auto-quoted when necessary (ie. strings, on
a text-based domain).
As per our discussion, this should leave unquoted.
- "System Domain?" should be in the General section, between owner and comment.
Done
- The switches should use the same colouring/styling as other objects, e.g.
options: {
'onText': 'Yes', 'offText': 'No',
'onColor': 'success', 'offColor': 'primary',
'size': 'small'
}
Done
- Please remove the Schema property from the main properties tab (not
the properties dialogue).
Done
- No icon is show for Checks on the Dependents tab for a domain.
Done
- The add button on the Security Labels tab is spelt "Add". Why is
that? Other instances of this grid use "ADD" which is the default in
backform.pgadmin.js.
Done
- Dependencies on domain check constraints are listed as being on a
"Type" not a "Domain".
Done
- If adding a domain constraint using the grid on the Domain dialogue,
I cannot specify "NOT VALID". We need a checkbox for that in a narrow
columns at the end. Unchecking it for an existing constraint should be
the equivalent of doing "ALTER DOMAIN ... VALIDATE CONSTRAINT"
Done
- If I switch the "Don't Validate" switch on a constraint, there are
leading blank lines in the generated SQL. The same occurs when adding
a comment to a constraint.
Done
- I think we need to reverse the meaning of "Don't Validate" and
rename to match the "Valid?" field that's on the properties list.
Otherwise it's not clear they're the same thing.
Done
- s/Not Null/Not Null?/
Done
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения
Updated one comment.
On Wed, Mar 23, 2016 at 12:48 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi,Please find attached updated patch for the Domains module.On Wed, Mar 16, 2016 at 9:40 PM, Dave Page <dpage@pgadmin.org> wrote:Hi
On Wed, Mar 16, 2016 at 2:03 PM, Khushboo Vashi
<khushboo.vashi@enterprisedb.com> wrote:
> Hi,
>
> Please find the updated Domain Module Patch.
>
> To test this patch, please apply Backgrid Textarea Cell Patch before this.
Thanks. I believe with the following fixes, we'll be done :-)
- Default values should be auto-quoted when necessary (ie. strings, on
a text-based domain).As per our discussion, this should leave unquoted.
And also added a hint to the field stated 'Enter an expression or a value.'
- "System Domain?" should be in the General section, between owner and comment.Done- The switches should use the same colouring/styling as other objects, e.g.
options: {
'onText': 'Yes', 'offText': 'No',
'onColor': 'success', 'offColor': 'primary',
'size': 'small'
}Done- Please remove the Schema property from the main properties tab (not
the properties dialogue).Done- No icon is show for Checks on the Dependents tab for a domain.Done- The add button on the Security Labels tab is spelt "Add". Why is
that? Other instances of this grid use "ADD" which is the default in
backform.pgadmin.js.Done- Dependencies on domain check constraints are listed as being on a
"Type" not a "Domain".Done- If adding a domain constraint using the grid on the Domain dialogue,
I cannot specify "NOT VALID". We need a checkbox for that in a narrow
columns at the end. Unchecking it for an existing constraint should be
the equivalent of doing "ALTER DOMAIN ... VALIDATE CONSTRAINT"Done- If I switch the "Don't Validate" switch on a constraint, there are
leading blank lines in the generated SQL. The same occurs when adding
a comment to a constraint.Done- I think we need to reverse the meaning of "Don't Validate" and
rename to match the "Valid?" field that's on the properties list.
Otherwise it's not clear they're the same thing.Done- s/Not Null/Not Null?/Done
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Did you forget to attach the latest patch? On Wed, Mar 23, 2016 at 7:27 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: > Updated one comment. > > On Wed, Mar 23, 2016 at 12:48 PM, Khushboo Vashi > <khushboo.vashi@enterprisedb.com> wrote: >> >> Hi, >> >> Please find attached updated patch for the Domains module. >> >> On Wed, Mar 16, 2016 at 9:40 PM, Dave Page <dpage@pgadmin.org> wrote: >>> >>> Hi >>> >>> On Wed, Mar 16, 2016 at 2:03 PM, Khushboo Vashi >>> <khushboo.vashi@enterprisedb.com> wrote: >>> > Hi, >>> > >>> > Please find the updated Domain Module Patch. >>> > >>> > To test this patch, please apply Backgrid Textarea Cell Patch before >>> > this. >>> >>> Thanks. I believe with the following fixes, we'll be done :-) >>> >>> - Default values should be auto-quoted when necessary (ie. strings, on >>> a text-based domain). >> >> As per our discussion, this should leave unquoted. > > And also added a hint to the field stated 'Enter an expression or a value.' >> >> >>> - "System Domain?" should be in the General section, between owner and >>> comment. >> >> Done >>> >>> - The switches should use the same colouring/styling as other objects, >>> e.g. >>> >>> options: { >>> 'onText': 'Yes', 'offText': 'No', >>> 'onColor': 'success', 'offColor': 'primary', >>> 'size': 'small' >>> } >> >> Done >>> >>> - Please remove the Schema property from the main properties tab (not >>> the properties dialogue). >> >> Done >>> >>> - No icon is show for Checks on the Dependents tab for a domain. >> >> Done >>> >>> - The add button on the Security Labels tab is spelt "Add". Why is >>> that? Other instances of this grid use "ADD" which is the default in >>> backform.pgadmin.js. >> >> Done >>> >>> - Dependencies on domain check constraints are listed as being on a >>> "Type" not a "Domain". >> >> Done >>> >>> - If adding a domain constraint using the grid on the Domain dialogue, >>> I cannot specify "NOT VALID". We need a checkbox for that in a narrow >>> columns at the end. Unchecking it for an existing constraint should be >>> the equivalent of doing "ALTER DOMAIN ... VALIDATE CONSTRAINT" >> >> Done >>> >>> - If I switch the "Don't Validate" switch on a constraint, there are >>> leading blank lines in the generated SQL. The same occurs when adding >>> a comment to a constraint. >> >> Done >>> >>> - I think we need to reverse the meaning of "Don't Validate" and >>> rename to match the "Valid?" field that's on the properties list. >>> Otherwise it's not clear they're the same thing. >> >> Done >>> >>> - s/Not Null/Not Null?/ >> >> Done >>> >>> >>> >>> -- >>> Dave Page >>> Blog: http://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EnterpriseDB UK: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >> >> > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Wed, Mar 23, 2016 at 3:09 PM, Dave Page <dpage@pgadmin.org> wrote:
Did you forget to attach the latest patch?
She did it in the previous patch only.
She forgot to mention about the help string, which she did in later mail.
--
Thanks & Regards,
Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
On Wed, Mar 23, 2016 at 7:27 AM, Khushboo Vashi
<khushboo.vashi@enterprisedb.com> wrote:
> Updated one comment.
>
> On Wed, Mar 23, 2016 at 12:48 PM, Khushboo Vashi
> <khushboo.vashi@enterprisedb.com> wrote:
>>
>> Hi,
>>
>> Please find attached updated patch for the Domains module.
>>
>> On Wed, Mar 16, 2016 at 9:40 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>
>>> Hi
>>>
>>> On Wed, Mar 16, 2016 at 2:03 PM, Khushboo Vashi
>>> <khushboo.vashi@enterprisedb.com> wrote:
>>> > Hi,
>>> >
>>> > Please find the updated Domain Module Patch.
>>> >
>>> > To test this patch, please apply Backgrid Textarea Cell Patch before
>>> > this.
>>>
>>> Thanks. I believe with the following fixes, we'll be done :-)
>>>
>>> - Default values should be auto-quoted when necessary (ie. strings, on
>>> a text-based domain).
>>
>> As per our discussion, this should leave unquoted.
>
> And also added a hint to the field stated 'Enter an expression or a value.'
>>
>>
>>> - "System Domain?" should be in the General section, between owner and
>>> comment.
>>
>> Done
>>>
>>> - The switches should use the same colouring/styling as other objects,
>>> e.g.
>>>
>>> options: {
>>> 'onText': 'Yes', 'offText': 'No',
>>> 'onColor': 'success', 'offColor': 'primary',
>>> 'size': 'small'
>>> }
>>
>> Done
>>>
>>> - Please remove the Schema property from the main properties tab (not
>>> the properties dialogue).
>>
>> Done
>>>
>>> - No icon is show for Checks on the Dependents tab for a domain.
>>
>> Done
>>>
>>> - The add button on the Security Labels tab is spelt "Add". Why is
>>> that? Other instances of this grid use "ADD" which is the default in
>>> backform.pgadmin.js.
>>
>> Done
>>>
>>> - Dependencies on domain check constraints are listed as being on a
>>> "Type" not a "Domain".
>>
>> Done
>>>
>>> - If adding a domain constraint using the grid on the Domain dialogue,
>>> I cannot specify "NOT VALID". We need a checkbox for that in a narrow
>>> columns at the end. Unchecking it for an existing constraint should be
>>> the equivalent of doing "ALTER DOMAIN ... VALIDATE CONSTRAINT"
>>
>> Done
>>>
>>> - If I switch the "Don't Validate" switch on a constraint, there are
>>> leading blank lines in the generated SQL. The same occurs when adding
>>> a comment to a constraint.
>>
>> Done
>>>
>>> - I think we need to reverse the meaning of "Don't Validate" and
>>> rename to match the "Valid?" field that's on the properties list.
>>> Otherwise it's not clear they're the same thing.
>>
>> Done
>>>
>>> - s/Not Null/Not Null?/
>>
>> Done
>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>
>>
>
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Hi Almost there :-s - The hint for default should be a placeholder in the textbox itself (like combos have "Select from the list" - I should be able to check the "Validate?" option on one or more constraints from within the Domain dialogue - Please ensure the capitalisation of all property labels is consistent - it should be "Base type" not "Base Type", "System domain?" not "System Domain?" etc. - The check constraint reverse engineered SQL should include the path to the constraint, e.g. -- CHECK: schema.domain.check_at Once that's done, it can be committed I think. Thanks. On Wed, Mar 23, 2016 at 7:27 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: > Updated one comment. > > On Wed, Mar 23, 2016 at 12:48 PM, Khushboo Vashi > <khushboo.vashi@enterprisedb.com> wrote: >> >> Hi, >> >> Please find attached updated patch for the Domains module. >> >> On Wed, Mar 16, 2016 at 9:40 PM, Dave Page <dpage@pgadmin.org> wrote: >>> >>> Hi >>> >>> On Wed, Mar 16, 2016 at 2:03 PM, Khushboo Vashi >>> <khushboo.vashi@enterprisedb.com> wrote: >>> > Hi, >>> > >>> > Please find the updated Domain Module Patch. >>> > >>> > To test this patch, please apply Backgrid Textarea Cell Patch before >>> > this. >>> >>> Thanks. I believe with the following fixes, we'll be done :-) >>> >>> - Default values should be auto-quoted when necessary (ie. strings, on >>> a text-based domain). >> >> As per our discussion, this should leave unquoted. > > And also added a hint to the field stated 'Enter an expression or a value.' >> >> >>> - "System Domain?" should be in the General section, between owner and >>> comment. >> >> Done >>> >>> - The switches should use the same colouring/styling as other objects, >>> e.g. >>> >>> options: { >>> 'onText': 'Yes', 'offText': 'No', >>> 'onColor': 'success', 'offColor': 'primary', >>> 'size': 'small' >>> } >> >> Done >>> >>> - Please remove the Schema property from the main properties tab (not >>> the properties dialogue). >> >> Done >>> >>> - No icon is show for Checks on the Dependents tab for a domain. >> >> Done >>> >>> - The add button on the Security Labels tab is spelt "Add". Why is >>> that? Other instances of this grid use "ADD" which is the default in >>> backform.pgadmin.js. >> >> Done >>> >>> - Dependencies on domain check constraints are listed as being on a >>> "Type" not a "Domain". >> >> Done >>> >>> - If adding a domain constraint using the grid on the Domain dialogue, >>> I cannot specify "NOT VALID". We need a checkbox for that in a narrow >>> columns at the end. Unchecking it for an existing constraint should be >>> the equivalent of doing "ALTER DOMAIN ... VALIDATE CONSTRAINT" >> >> Done >>> >>> - If I switch the "Don't Validate" switch on a constraint, there are >>> leading blank lines in the generated SQL. The same occurs when adding >>> a comment to a constraint. >> >> Done >>> >>> - I think we need to reverse the meaning of "Don't Validate" and >>> rename to match the "Valid?" field that's on the properties list. >>> Otherwise it's not clear they're the same thing. >> >> Done >>> >>> - s/Not Null/Not Null?/ >> >> Done >>> >>> >>> >>> -- >>> Dave Page >>> Blog: http://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EnterpriseDB UK: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >> >> > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hi,
Please find the attached updated patch for the Domain module.On Wed, Mar 23, 2016 at 6:35 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi
Almost there :-s
- The hint for default should be a placeholder in the textbox itself
(like combos have "Select from the list"
Done
- I should be able to check the "Validate?" option on one or more
constraints from within the Domain dialogue
Done.
Constraint Name can also be changed through the Domain dialogue.
- Please ensure the capitalisation of all property labels is
consistent - it should be "Base type" not "Base Type", "System
domain?" not "System Domain?" etc.
Done
- The check constraint reverse engineered SQL should include the path
to the constraint, e.g.
Done
-- CHECK: schema.domain.check_at
Once that's done, it can be committed I think.
Thanks.
On Wed, Mar 23, 2016 at 7:27 AM, Khushboo Vashi
<khushboo.vashi@enterprisedb.com> wrote:
> Updated one comment.
>
> On Wed, Mar 23, 2016 at 12:48 PM, Khushboo Vashi
> <khushboo.vashi@enterprisedb.com> wrote:
>>
>> Hi,
>>
>> Please find attached updated patch for the Domains module.
>>
>> On Wed, Mar 16, 2016 at 9:40 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>
>>> Hi
>>>
>>> On Wed, Mar 16, 2016 at 2:03 PM, Khushboo Vashi
>>> <khushboo.vashi@enterprisedb.com> wrote:
>>> > Hi,
>>> >
>>> > Please find the updated Domain Module Patch.
>>> >
>>> > To test this patch, please apply Backgrid Textarea Cell Patch before
>>> > this.
>>>
>>> Thanks. I believe with the following fixes, we'll be done :-)
>>>
>>> - Default values should be auto-quoted when necessary (ie. strings, on
>>> a text-based domain).
>>
>> As per our discussion, this should leave unquoted.
>
> And also added a hint to the field stated 'Enter an expression or a value.'
>>
>>
>>> - "System Domain?" should be in the General section, between owner and
>>> comment.
>>
>> Done
>>>
>>> - The switches should use the same colouring/styling as other objects,
>>> e.g.
>>>
>>> options: {
>>> 'onText': 'Yes', 'offText': 'No',
>>> 'onColor': 'success', 'offColor': 'primary',
>>> 'size': 'small'
>>> }
>>
>> Done
>>>
>>> - Please remove the Schema property from the main properties tab (not
>>> the properties dialogue).
>>
>> Done
>>>
>>> - No icon is show for Checks on the Dependents tab for a domain.
>>
>> Done
>>>
>>> - The add button on the Security Labels tab is spelt "Add". Why is
>>> that? Other instances of this grid use "ADD" which is the default in
>>> backform.pgadmin.js.
>>
>> Done
>>>
>>> - Dependencies on domain check constraints are listed as being on a
>>> "Type" not a "Domain".
>>
>> Done
>>>
>>> - If adding a domain constraint using the grid on the Domain dialogue,
>>> I cannot specify "NOT VALID". We need a checkbox for that in a narrow
>>> columns at the end. Unchecking it for an existing constraint should be
>>> the equivalent of doing "ALTER DOMAIN ... VALIDATE CONSTRAINT"
>>
>> Done
>>>
>>> - If I switch the "Don't Validate" switch on a constraint, there are
>>> leading blank lines in the generated SQL. The same occurs when adding
>>> a comment to a constraint.
>>
>> Done
>>>
>>> - I think we need to reverse the meaning of "Don't Validate" and
>>> rename to match the "Valid?" field that's on the properties list.
>>> Otherwise it's not clear they're the same thing.
>>
>> Done
>>>
>>> - s/Not Null/Not Null?/
>>
>> Done
>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>
>>
>
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения
Hi You're going to hate me for this.... - I added an un-validated constraint to a domain, then opened the domain properties and clicked the Validate? option for it. The SQL is generated, but the Save button is not enabled. - If I right-click a domain, I get Create options for "Domain" (with a constraint icon) and "Domain..." with a domain icon. See attached screenshot. Thanks. On Thu, Mar 24, 2016 at 9:54 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: > Hi, > > Please find the attached updated patch for the Domain module. > > Thanks, > Khushboo > > On Wed, Mar 23, 2016 at 6:35 PM, Dave Page <dpage@pgadmin.org> wrote: >> >> Hi >> >> Almost there :-s >> >> - The hint for default should be a placeholder in the textbox itself >> (like combos have "Select from the list" > > Done >> >> - I should be able to check the "Validate?" option on one or more >> constraints from within the Domain dialogue > > Done. > Constraint Name can also be changed through the Domain dialogue. >> >> - Please ensure the capitalisation of all property labels is >> consistent - it should be "Base type" not "Base Type", "System >> domain?" not "System Domain?" etc. > > Done >> >> - The check constraint reverse engineered SQL should include the path >> to the constraint, e.g. >> > Done >> >> -- CHECK: schema.domain.check_at >> >> Once that's done, it can be committed I think. >> >> Thanks. >> >> On Wed, Mar 23, 2016 at 7:27 AM, Khushboo Vashi >> <khushboo.vashi@enterprisedb.com> wrote: >> > Updated one comment. >> > >> > On Wed, Mar 23, 2016 at 12:48 PM, Khushboo Vashi >> > <khushboo.vashi@enterprisedb.com> wrote: >> >> >> >> Hi, >> >> >> >> Please find attached updated patch for the Domains module. >> >> >> >> On Wed, Mar 16, 2016 at 9:40 PM, Dave Page <dpage@pgadmin.org> wrote: >> >>> >> >>> Hi >> >>> >> >>> On Wed, Mar 16, 2016 at 2:03 PM, Khushboo Vashi >> >>> <khushboo.vashi@enterprisedb.com> wrote: >> >>> > Hi, >> >>> > >> >>> > Please find the updated Domain Module Patch. >> >>> > >> >>> > To test this patch, please apply Backgrid Textarea Cell Patch before >> >>> > this. >> >>> >> >>> Thanks. I believe with the following fixes, we'll be done :-) >> >>> >> >>> - Default values should be auto-quoted when necessary (ie. strings, on >> >>> a text-based domain). >> >> >> >> As per our discussion, this should leave unquoted. >> > >> > And also added a hint to the field stated 'Enter an expression or a >> > value.' >> >> >> >> >> >>> - "System Domain?" should be in the General section, between owner and >> >>> comment. >> >> >> >> Done >> >>> >> >>> - The switches should use the same colouring/styling as other objects, >> >>> e.g. >> >>> >> >>> options: { >> >>> 'onText': 'Yes', 'offText': 'No', >> >>> 'onColor': 'success', 'offColor': 'primary', >> >>> 'size': 'small' >> >>> } >> >> >> >> Done >> >>> >> >>> - Please remove the Schema property from the main properties tab (not >> >>> the properties dialogue). >> >> >> >> Done >> >>> >> >>> - No icon is show for Checks on the Dependents tab for a domain. >> >> >> >> Done >> >>> >> >>> - The add button on the Security Labels tab is spelt "Add". Why is >> >>> that? Other instances of this grid use "ADD" which is the default in >> >>> backform.pgadmin.js. >> >> >> >> Done >> >>> >> >>> - Dependencies on domain check constraints are listed as being on a >> >>> "Type" not a "Domain". >> >> >> >> Done >> >>> >> >>> - If adding a domain constraint using the grid on the Domain dialogue, >> >>> I cannot specify "NOT VALID". We need a checkbox for that in a narrow >> >>> columns at the end. Unchecking it for an existing constraint should be >> >>> the equivalent of doing "ALTER DOMAIN ... VALIDATE CONSTRAINT" >> >> >> >> Done >> >>> >> >>> - If I switch the "Don't Validate" switch on a constraint, there are >> >>> leading blank lines in the generated SQL. The same occurs when adding >> >>> a comment to a constraint. >> >> >> >> Done >> >>> >> >>> - I think we need to reverse the meaning of "Don't Validate" and >> >>> rename to match the "Valid?" field that's on the properties list. >> >>> Otherwise it's not clear they're the same thing. >> >> >> >> Done >> >>> >> >>> - s/Not Null/Not Null?/ >> >> >> >> Done >> >>> >> >>> >> >>> >> >>> -- >> >>> Dave Page >> >>> Blog: http://pgsnake.blogspot.com >> >>> Twitter: @pgsnake >> >>> >> >>> EnterpriseDB UK: http://www.enterprisedb.com >> >>> The Enterprise PostgreSQL Company >> >> >> >> >> > >> >> >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company > > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Вложения
Hi,
Please find the attached updated patch for the Domains Module.
Thanks,
Khushboo
On Thu, Mar 24, 2016 at 5:29 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi
You're going to hate me for this....
- I added an un-validated constraint to a domain, then opened the
domain properties and clicked the Validate? option for it. The SQL is
generated, but the Save button is not enabled.
Done.
- If I right-click a domain, I get Create options for "Domain" (with a
constraint icon) and "Domain..." with a domain icon.
As per the discussion with Ashesh, He has updated the context menu JS file with the new version and that is causing the issue.
He is going to fix this issue as this is generic for the all context menu.
I will create a new task for this in kanban and assign it to Ashesh.
See attached screenshot.
Thanks.
On Thu, Mar 24, 2016 at 9:54 AM, Khushboo Vashi<khushboo.vashi@enterprisedb.com> wrote:
> Hi,
>
> Please find the attached updated patch for the Domain module.
>
> Thanks,
> Khushboo
>
> On Wed, Mar 23, 2016 at 6:35 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> Almost there :-s
>>
>> - The hint for default should be a placeholder in the textbox itself
>> (like combos have "Select from the list"
>
> Done
>>
>> - I should be able to check the "Validate?" option on one or more
>> constraints from within the Domain dialogue
>
> Done.
> Constraint Name can also be changed through the Domain dialogue.
>>
>> - Please ensure the capitalisation of all property labels is
>> consistent - it should be "Base type" not "Base Type", "System
>> domain?" not "System Domain?" etc.
>
> Done
>>
>> - The check constraint reverse engineered SQL should include the path
>> to the constraint, e.g.
>>
> Done
>>
>> -- CHECK: schema.domain.check_at
>>
>> Once that's done, it can be committed I think.
>>
>> Thanks.
>>
>> On Wed, Mar 23, 2016 at 7:27 AM, Khushboo Vashi
>> <khushboo.vashi@enterprisedb.com> wrote:
>> > Updated one comment.
>> >
>> > On Wed, Mar 23, 2016 at 12:48 PM, Khushboo Vashi
>> > <khushboo.vashi@enterprisedb.com> wrote:
>> >>
>> >> Hi,
>> >>
>> >> Please find attached updated patch for the Domains module.
>> >>
>> >> On Wed, Mar 16, 2016 at 9:40 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >>>
>> >>> Hi
>> >>>
>> >>> On Wed, Mar 16, 2016 at 2:03 PM, Khushboo Vashi
>> >>> <khushboo.vashi@enterprisedb.com> wrote:
>> >>> > Hi,
>> >>> >
>> >>> > Please find the updated Domain Module Patch.
>> >>> >
>> >>> > To test this patch, please apply Backgrid Textarea Cell Patch before
>> >>> > this.
>> >>>
>> >>> Thanks. I believe with the following fixes, we'll be done :-)
>> >>>
>> >>> - Default values should be auto-quoted when necessary (ie. strings, on
>> >>> a text-based domain).
>> >>
>> >> As per our discussion, this should leave unquoted.
>> >
>> > And also added a hint to the field stated 'Enter an expression or a
>> > value.'
>> >>
>> >>
>> >>> - "System Domain?" should be in the General section, between owner and
>> >>> comment.
>> >>
>> >> Done
>> >>>
>> >>> - The switches should use the same colouring/styling as other objects,
>> >>> e.g.
>> >>>
>> >>> options: {
>> >>> 'onText': 'Yes', 'offText': 'No',
>> >>> 'onColor': 'success', 'offColor': 'primary',
>> >>> 'size': 'small'
>> >>> }
>> >>
>> >> Done
>> >>>
>> >>> - Please remove the Schema property from the main properties tab (not
>> >>> the properties dialogue).
>> >>
>> >> Done
>> >>>
>> >>> - No icon is show for Checks on the Dependents tab for a domain.
>> >>
>> >> Done
>> >>>
>> >>> - The add button on the Security Labels tab is spelt "Add". Why is
>> >>> that? Other instances of this grid use "ADD" which is the default in
>> >>> backform.pgadmin.js.
>> >>
>> >> Done
>> >>>
>> >>> - Dependencies on domain check constraints are listed as being on a
>> >>> "Type" not a "Domain".
>> >>
>> >> Done
>> >>>
>> >>> - If adding a domain constraint using the grid on the Domain dialogue,
>> >>> I cannot specify "NOT VALID". We need a checkbox for that in a narrow
>> >>> columns at the end. Unchecking it for an existing constraint should be
>> >>> the equivalent of doing "ALTER DOMAIN ... VALIDATE CONSTRAINT"
>> >>
>> >> Done
>> >>>
>> >>> - If I switch the "Don't Validate" switch on a constraint, there are
>> >>> leading blank lines in the generated SQL. The same occurs when adding
>> >>> a comment to a constraint.
>> >>
>> >> Done
>> >>>
>> >>> - I think we need to reverse the meaning of "Don't Validate" and
>> >>> rename to match the "Valid?" field that's on the properties list.
>> >>> Otherwise it's not clear they're the same thing.
>> >>
>> >> Done
>> >>>
>> >>> - s/Not Null/Not Null?/
>> >>
>> >> Done
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> Dave Page
>> >>> Blog: http://pgsnake.blogspot.com
>> >>> Twitter: @pgsnake
>> >>>
>> >>> EnterpriseDB UK: http://www.enterprisedb.com
>> >>> The Enterprise PostgreSQL Company
>> >>
>> >>
>> >
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>
>
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения
Thanks - committed. On Thu, Mar 24, 2016 at 3:25 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote: > Hi, > > Please find the attached updated patch for the Domains Module. > > Thanks, > Khushboo > > On Thu, Mar 24, 2016 at 5:29 PM, Dave Page <dpage@pgadmin.org> wrote: >> >> Hi >> >> You're going to hate me for this.... >> >> - I added an un-validated constraint to a domain, then opened the >> domain properties and clicked the Validate? option for it. The SQL is >> generated, but the Save button is not enabled. > > Done. >> >> >> - If I right-click a domain, I get Create options for "Domain" (with a >> constraint icon) and "Domain..." with a domain icon. >> > As per the discussion with Ashesh, He has updated the context menu JS file > with the new version and that is causing the issue. > He is going to fix this issue as this is generic for the all context menu. > I will create a new task for this in kanban and assign it to Ashesh. > >> See attached screenshot. >> >> Thanks. >> >> On Thu, Mar 24, 2016 at 9:54 AM, Khushboo Vashi >> <khushboo.vashi@enterprisedb.com> wrote: >> > Hi, >> > >> > Please find the attached updated patch for the Domain module. >> > >> > Thanks, >> > Khushboo >> > >> > On Wed, Mar 23, 2016 at 6:35 PM, Dave Page <dpage@pgadmin.org> wrote: >> >> >> >> Hi >> >> >> >> Almost there :-s >> >> >> >> - The hint for default should be a placeholder in the textbox itself >> >> (like combos have "Select from the list" >> > >> > Done >> >> >> >> - I should be able to check the "Validate?" option on one or more >> >> constraints from within the Domain dialogue >> > >> > Done. >> > Constraint Name can also be changed through the Domain dialogue. >> >> >> >> - Please ensure the capitalisation of all property labels is >> >> consistent - it should be "Base type" not "Base Type", "System >> >> domain?" not "System Domain?" etc. >> > >> > Done >> >> >> >> - The check constraint reverse engineered SQL should include the path >> >> to the constraint, e.g. >> >> >> > Done >> >> >> >> -- CHECK: schema.domain.check_at >> >> >> >> Once that's done, it can be committed I think. >> >> >> >> Thanks. >> >> >> >> On Wed, Mar 23, 2016 at 7:27 AM, Khushboo Vashi >> >> <khushboo.vashi@enterprisedb.com> wrote: >> >> > Updated one comment. >> >> > >> >> > On Wed, Mar 23, 2016 at 12:48 PM, Khushboo Vashi >> >> > <khushboo.vashi@enterprisedb.com> wrote: >> >> >> >> >> >> Hi, >> >> >> >> >> >> Please find attached updated patch for the Domains module. >> >> >> >> >> >> On Wed, Mar 16, 2016 at 9:40 PM, Dave Page <dpage@pgadmin.org> >> >> >> wrote: >> >> >>> >> >> >>> Hi >> >> >>> >> >> >>> On Wed, Mar 16, 2016 at 2:03 PM, Khushboo Vashi >> >> >>> <khushboo.vashi@enterprisedb.com> wrote: >> >> >>> > Hi, >> >> >>> > >> >> >>> > Please find the updated Domain Module Patch. >> >> >>> > >> >> >>> > To test this patch, please apply Backgrid Textarea Cell Patch >> >> >>> > before >> >> >>> > this. >> >> >>> >> >> >>> Thanks. I believe with the following fixes, we'll be done :-) >> >> >>> >> >> >>> - Default values should be auto-quoted when necessary (ie. strings, >> >> >>> on >> >> >>> a text-based domain). >> >> >> >> >> >> As per our discussion, this should leave unquoted. >> >> > >> >> > And also added a hint to the field stated 'Enter an expression or a >> >> > value.' >> >> >> >> >> >> >> >> >>> - "System Domain?" should be in the General section, between owner >> >> >>> and >> >> >>> comment. >> >> >> >> >> >> Done >> >> >>> >> >> >>> - The switches should use the same colouring/styling as other >> >> >>> objects, >> >> >>> e.g. >> >> >>> >> >> >>> options: { >> >> >>> 'onText': 'Yes', 'offText': 'No', >> >> >>> 'onColor': 'success', 'offColor': 'primary', >> >> >>> 'size': 'small' >> >> >>> } >> >> >> >> >> >> Done >> >> >>> >> >> >>> - Please remove the Schema property from the main properties tab >> >> >>> (not >> >> >>> the properties dialogue). >> >> >> >> >> >> Done >> >> >>> >> >> >>> - No icon is show for Checks on the Dependents tab for a domain. >> >> >> >> >> >> Done >> >> >>> >> >> >>> - The add button on the Security Labels tab is spelt "Add". Why is >> >> >>> that? Other instances of this grid use "ADD" which is the default >> >> >>> in >> >> >>> backform.pgadmin.js. >> >> >> >> >> >> Done >> >> >>> >> >> >>> - Dependencies on domain check constraints are listed as being on a >> >> >>> "Type" not a "Domain". >> >> >> >> >> >> Done >> >> >>> >> >> >>> - If adding a domain constraint using the grid on the Domain >> >> >>> dialogue, >> >> >>> I cannot specify "NOT VALID". We need a checkbox for that in a >> >> >>> narrow >> >> >>> columns at the end. Unchecking it for an existing constraint should >> >> >>> be >> >> >>> the equivalent of doing "ALTER DOMAIN ... VALIDATE CONSTRAINT" >> >> >> >> >> >> Done >> >> >>> >> >> >>> - If I switch the "Don't Validate" switch on a constraint, there >> >> >>> are >> >> >>> leading blank lines in the generated SQL. The same occurs when >> >> >>> adding >> >> >>> a comment to a constraint. >> >> >> >> >> >> Done >> >> >>> >> >> >>> - I think we need to reverse the meaning of "Don't Validate" and >> >> >>> rename to match the "Valid?" field that's on the properties list. >> >> >>> Otherwise it's not clear they're the same thing. >> >> >> >> >> >> Done >> >> >>> >> >> >>> - s/Not Null/Not Null?/ >> >> >> >> >> >> Done >> >> >>> >> >> >>> >> >> >>> >> >> >>> -- >> >> >>> Dave Page >> >> >>> Blog: http://pgsnake.blogspot.com >> >> >>> Twitter: @pgsnake >> >> >>> >> >> >>> EnterpriseDB UK: http://www.enterprisedb.com >> >> >>> The Enterprise PostgreSQL Company >> >> >> >> >> >> >> >> > >> >> >> >> >> >> >> >> -- >> >> Dave Page >> >> Blog: http://pgsnake.blogspot.com >> >> Twitter: @pgsnake >> >> >> >> EnterpriseDB UK: http://www.enterprisedb.com >> >> The Enterprise PostgreSQL Company >> > >> > >> >> >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company > > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company