public inbox for [email protected]
help / color / mirror / Atom feed[pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view
10+ messages / 2 participants
[nested] [flat]
* [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view
@ 2020-01-14 04:57 Khushboo Vashi <[email protected]>
2020-01-14 06:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Khushboo Vashi @ 2020-01-14 04:57 UTC (permalink / raw)
To: pgadmin-hackers
Hi,
Please find the attached patch for RM #5053 - Getting an error while
changing the columns in the existing view.
PostgreSQL doesn't allow to change the view columns. So, while performing
this task the existing view should be dropped first and then recreate it
and also user will get a warning first.
Thanks,
Khushboo
Attachments:
[application/octet-stream] RM_5053.patch (8.2K, 3-RM_5053.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py
index b03136891..1b34b9715 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py
@@ -10,6 +10,7 @@
"""Implements View and Materialized View Node"""
import copy
+import re
from functools import wraps
import simplejson as json
@@ -496,7 +497,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
)
)
try:
- SQL, nameOrError = self.getSQL(gid, sid, did, data)
+ SQL, nameOrError = self.getSQL(gid, sid, did, scid, data)
if SQL is None:
return nameOrError
SQL = SQL.strip('\n').strip(' ')
@@ -541,7 +542,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
request.data, encoding='utf-8'
)
try:
- SQL, name = self.getSQL(gid, sid, did, data, vid)
+ SQL, name = self.getSQL(gid, sid, did, scid, data, vid)
if SQL is None:
return name
SQL = SQL.strip('\n').strip(' ')
@@ -678,7 +679,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
except ValueError:
data[k] = v
- sql, nameOrError = self.getSQL(gid, sid, did, data, vid)
+ sql, nameOrError = self.getSQL(gid, sid, did, scid, data, vid)
if sql is None:
return nameOrError
@@ -692,7 +693,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
status=200
)
- def getSQL(self, gid, sid, did, data, vid=None):
+ def getSQL(self, gid, sid, did, scid, data, vid=None):
"""
This function will generate sql from model data
"""
@@ -716,7 +717,21 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
if 'schema' not in data:
data['schema'] = res['rows'][0]['schema']
- acls = []
+
+ DEL_SQL = None
+ if 'definition' in data:
+ new_def = re.sub(r"\W", "", data['definition']).split('FROM')
+ old_def = re.sub(r"\W", "", res['rows'][0]['definition']
+ ).split('FROM')
+ if 'definition' in data and (
+ old_def[0] != new_def[0]
+ and old_def[0] not in new_def[0]
+ ):
+ DEL_SQL = self.delete(gid=gid, sid=sid, did=did,
+ scid=scid,
+ vid=vid, only_sql=True
+ )
+
try:
acls = render_template(
"/".join([self.template_path, 'sql/allowed_privs.json'])
@@ -740,6 +755,9 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
SQL = render_template("/".join(
[self.template_path, 'sql/update.sql']), data=data,
o_data=old_data, conn=self.conn)
+
+ if DEL_SQL:
+ SQL = DEL_SQL + SQL
except Exception as e:
current_app.logger.exception(e)
return None, internal_server_error(errormsg=str(e))
@@ -1436,7 +1454,14 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
if data:
if diff_schema:
data['schema'] = diff_schema
- sql, nameOrError = self.getSQL(gid, sid, did, data, oid)
+ sql, nameOrError = self.getSQL(gid, sid, did, scid, data, oid)
+ if sql.find('DROP VIEW') != -1:
+ sql = gettext("""
+-- Changing the columns in a view requires dropping and re-creating the view.
+-- This may fail if other objects are dependent upon this view,
+-- or may cause procedural functions to fail if they are not modified to
+-- take account of the changes.
+""") + sql
else:
if drop_sql:
sql = self.delete(gid=gid, sid=sid, did=did,
@@ -1544,8 +1569,8 @@ class MViewNode(ViewNode, VacuumSettings):
data['vacuum_data']['reset'] = []
# table vacuum: separate list of changed and reset data for
- if ('vacuum_table' in data):
- if ('changed' in data['vacuum_table']):
+ if 'vacuum_table' in data:
+ if 'changed' in data['vacuum_table']:
for item in data['vacuum_table']['changed']:
if 'value' in item.keys():
if item['value'] is None:
@@ -1578,8 +1603,8 @@ class MViewNode(ViewNode, VacuumSettings):
'value': data['autovacuum_enabled']})
# toast autovacuum: separate list of changed and reset data
- if ('vacuum_toast' in data):
- if ('changed' in data['vacuum_toast']):
+ if 'vacuum_toast' in data:
+ if 'changed' in data['vacuum_toast']:
for item in data['vacuum_toast']['changed']:
if 'value' in item.keys():
toast_key = 'toast_' + item['name']
@@ -1668,14 +1693,14 @@ class MViewNode(ViewNode, VacuumSettings):
if 'value' in item.keys() and item['value'] is not None]
# add table_enabled & toast_enabled settings
- if ('autovacuum_custom' in data and data['autovacuum_custom']):
+ if 'autovacuum_custom' in data and data['autovacuum_custom']:
vacuum_table.append(
{
'name': 'autovacuum_enabled',
'value': str(data['autovacuum_enabled'])
}
)
- if ('toast_autovacuum' in data and data['toast_autovacuum']):
+ if 'toast_autovacuum' in data and data['toast_autovacuum']:
vacuum_table.append(
{
'name': 'toast.autovacuum_enabled',
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js
index e85bd7f15..42f47f192 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js
@@ -154,8 +154,29 @@ define('pgadmin.node.view', [
id: 'definition', label: gettext('Code'), cell: 'string',
type: 'text', mode: ['create', 'edit'], group: gettext('Code'),
tabPanelCodeClass: 'sql-code-control',
- control: Backform.SqlCodeControl,
disabled: 'notInSchema',
+ control: Backform.SqlCodeControl.extend({
+ onChange: function() {
+ Backform.SqlCodeControl.prototype.onChange.apply(this, arguments);
+ if(this.model && this.model.changed) {
+ if(this.model.origSessAttrs && (this.model.changed.definition != this.model.origSessAttrs.definition)) {
+ let old_def = this.model.origSessAttrs.definition.replace(/\s/gi, '').split('FROM'),
+ new_def = this.model.changed.definition.replace(/\s/gi, '').split('FROM');
+ if (old_def[0] != new_def[0]) {
+ this.model.warn_text = gettext('Changing the columns in a view requires dropping and re-creating the view. This may fail if other objects are dependent upon this view, or may cause procedural functions to fail if they are not modified to take account of the changes. Do you wish to continue?');
+ } else {
+ this.model.warn_text = undefined;
+ }
+ }
+ else {
+ this.model.warn_text = undefined;
+ }
+ }
+ else {
+ this.model.warn_text = undefined;
+ }
+ },
+ }),
}, pgBrowser.SecurityGroupSchema, {
// Add Privilege Control
id: 'datacl', label: gettext('Privileges'), type: 'collection',
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view
2020-01-14 04:57 [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
@ 2020-01-14 06:17 ` Akshay Joshi <[email protected]>
2020-03-24 08:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Akshay Joshi @ 2020-01-14 06:17 UTC (permalink / raw)
To: Khushboo Vashi <[email protected]>; +Cc: pgadmin-hackers
Hi Khushboo
Following are the review comments:
- Fix the PEP8 issue.
- Drop query should be part of the jinja template for consistency.
Currently, it is added through the python file.
- Any changes in the view code should not warn the user "Changing the
columns in a view requires dropping...." and we should not drop the view.
For example, I have only change the WHERE clause or added 'ORDER BY'.
On Tue, Jan 14, 2020 at 10:27 AM Khushboo Vashi <
[email protected]> wrote:
> Hi,
>
> Please find the attached patch for RM #5053 - Getting an error while
> changing the columns in the existing view.
>
> PostgreSQL doesn't allow to change the view columns. So, while performing
> this task the existing view should be dropped first and then recreate it
> and also user will get a warning first.
>
> Thanks,
> Khushboo
>
--
*Thanks & Regards*
*Akshay Joshi*
*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view
2020-01-14 04:57 [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-01-14 06:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
@ 2020-03-24 08:17 ` Khushboo Vashi <[email protected]>
2020-03-24 09:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Khushboo Vashi @ 2020-03-24 08:17 UTC (permalink / raw)
To: Akshay Joshi <[email protected]>; +Cc: pgadmin-hackers
Hi Akshay,
On Tue, Jan 14, 2020 at 11:47 AM Akshay Joshi <[email protected]>
wrote:
> Hi Khushboo
>
> Following are the review comments:
>
> - Fix the PEP8 issue.
> - Drop query should be part of the jinja template for consistency.
> Currently, it is added through the python file.
>
> The Delete query is already in the template file, I have just reused the
delete call and merged the SQL queries in the python file.
>
> - Any changes in the view code should not warn the user "Changing the
> columns in a view requires dropping...." and we should not drop the view.
> For example, I have only change the WHERE clause or added 'ORDER BY'.
>
> I have tested but couldn't reproduce this issue. Can you please let me
know the proper use case?
Thanks,
Khushboo
>
>
> On Tue, Jan 14, 2020 at 10:27 AM Khushboo Vashi <
> [email protected]> wrote:
>
>> Hi,
>>
>> Please find the attached patch for RM #5053 - Getting an error while
>> changing the columns in the existing view.
>>
>> PostgreSQL doesn't allow to change the view columns. So, while performing
>> this task the existing view should be dropped first and then recreate it
>> and also user will get a warning first.
>>
>> Thanks,
>> Khushboo
>>
>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
>
> *Sr. Software Architect*
> *EnterpriseDB Software India Private Limited*
> *Mobile: +91 976-788-8246*
>
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view
2020-01-14 04:57 [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-01-14 06:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-03-24 08:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
@ 2020-03-24 09:17 ` Akshay Joshi <[email protected]>
2020-04-07 04:30 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Akshay Joshi @ 2020-03-24 09:17 UTC (permalink / raw)
To: Khushboo Vashi <[email protected]>; +Cc: pgadmin-hackers
Hi Khushboo
On Tue, Mar 24, 2020 at 1:47 PM Khushboo Vashi <
[email protected]> wrote:
> Hi Akshay,
>
> On Tue, Jan 14, 2020 at 11:47 AM Akshay Joshi <
> [email protected]> wrote:
>
>> Hi Khushboo
>>
>> Following are the review comments:
>>
>> - Fix the PEP8 issue.
>> - Drop query should be part of the jinja template for consistency.
>> Currently, it is added through the python file.
>>
>> The Delete query is already in the template file, I have just reused the
> delete call and merged the SQL queries in the python file.
>
>>
>> - Any changes in the view code should not warn the user "Changing the
>> columns in a view requires dropping...." and we should not drop the view.
>> For example, I have only change the WHERE clause or added 'ORDER BY'.
>>
>> I have tested but couldn't reproduce this issue. Can you please let me
> know the proper use case?
>
Create a view with 'SELECT 1;' as code. Then change the code to 'SELECT
1234;' and click on the Save button.
Warning popup is displayed "Changing the columns in a view....". Click
on the 'Yes' button and check the OID of the view. You will get the same
OID, it means view is not recreated.
I have observed below error in the browser while changing the code:
view.js:241 Uncaught TypeError: Cannot read property 'replace'
of undefined
at child.onChange (view.js:241)
at HTMLDivElement.dispatch (jquery.js:5237)
at HTMLDivElement.elemData.handle (jquery.js:5044)
> Thanks,
> Khushboo
>
>>
>>
>
>> On Tue, Jan 14, 2020 at 10:27 AM Khushboo Vashi <
>> [email protected]> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached patch for RM #5053 - Getting an error while
>>> changing the columns in the existing view.
>>>
>>> PostgreSQL doesn't allow to change the view columns. So, while
>>> performing this task the existing view should be dropped first and then
>>> recreate it and also user will get a warning first.
>>>
>>> Thanks,
>>> Khushboo
>>>
>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>>
>> *Sr. Software Architect*
>> *EnterpriseDB Software India Private Limited*
>> *Mobile: +91 976-788-8246*
>>
>
--
*Thanks & Regards*
*Akshay Joshi*
*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view
2020-01-14 04:57 [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-01-14 06:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-03-24 08:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-03-24 09:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
@ 2020-04-07 04:30 ` Khushboo Vashi <[email protected]>
2020-04-07 06:12 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Khushboo Vashi @ 2020-04-07 04:30 UTC (permalink / raw)
To: Akshay Joshi <[email protected]>; +Cc: pgadmin-hackers
Hi Akshay,
Please find the attached updated patch.
On Tue, Mar 24, 2020 at 2:47 PM Akshay Joshi <[email protected]>
wrote:
> Hi Khushboo
>
> On Tue, Mar 24, 2020 at 1:47 PM Khushboo Vashi <
> [email protected]> wrote:
>
>> Hi Akshay,
>>
>> On Tue, Jan 14, 2020 at 11:47 AM Akshay Joshi <
>> [email protected]> wrote:
>>
>>> Hi Khushboo
>>>
>>> Following are the review comments:
>>>
>>> - Fix the PEP8 issue.
>>> - Drop query should be part of the jinja template for consistency.
>>> Currently, it is added through the python file.
>>>
>>> The Delete query is already in the template file, I have just reused the
>> delete call and merged the SQL queries in the python file.
>>
>>>
>>> - Any changes in the view code should not warn the user "Changing
>>> the columns in a view requires dropping...." and we should not drop the
>>> view. For example, I have only change the WHERE clause or added 'ORDER BY'.
>>>
>>> I have tested but couldn't reproduce this issue. Can you please let me
>> know the proper use case?
>>
>
> Create a view with 'SELECT 1;' as code. Then change the code to 'SELECT
> 1234;' and click on the Save button.
> Warning popup is displayed "Changing the columns in a view....". Click
> on the 'Yes' button and check the OID of the view. You will get the same
> OID, it means view is not recreated.
>
>
I can reproduce this issue with the given SQL but the problem is as per the
PostgreSQL documentation, (Ref:
https://www.postgresql.org/docs/12/sql-createview.html)
"CREATE OR REPLACE VIEW is similar, but if a view of the same name already
exists, it is replaced. The new query must generate the same columns that
were generated by the existing view query (that is, the same column names
in the same order and with the same data types), but it may add additional
columns to the end of the list. The calculations giving rise to the output
columns may be completely different."
So, I put a check on the columns and if the column is changed, the message
will popup.
In case of the example given by you, the column name is not changed as if
you don't give the column name it will be default and I think view would
have the column names properly.
> I have observed below error in the browser while changing the code:
> view.js:241 Uncaught TypeError: Cannot read property 'replace'
> of undefined
> at child.onChange (view.js:241)
> at HTMLDivElement.dispatch (jquery.js:5237)
> at HTMLDivElement.elemData.handle (jquery.js:5044)
>
> Fixed.
Thanks,
Khushboo
>
>> Thanks,
>> Khushboo
>>
>>>
>>>
>>
>>> On Tue, Jan 14, 2020 at 10:27 AM Khushboo Vashi <
>>> [email protected]> wrote:
>>>
>>>> Hi,
>>>>
>>>> Please find the attached patch for RM #5053 - Getting an error while
>>>> changing the columns in the existing view.
>>>>
>>>> PostgreSQL doesn't allow to change the view columns. So, while
>>>> performing this task the existing view should be dropped first and then
>>>> recreate it and also user will get a warning first.
>>>>
>>>> Thanks,
>>>> Khushboo
>>>>
>>>
>>>
>>> --
>>> *Thanks & Regards*
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect*
>>> *EnterpriseDB Software India Private Limited*
>>> *Mobile: +91 976-788-8246*
>>>
>>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
>
> *Sr. Software Architect*
> *EnterpriseDB Software India Private Limited*
> *Mobile: +91 976-788-8246*
>
Attachments:
[application/octet-stream] RM_5053_v1.patch (8.3K, 3-RM_5053_v1.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py
index fb1ad76ac..d502b29aa 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py
@@ -10,6 +10,7 @@
"""Implements View and Materialized View Node"""
import copy
+import re
from functools import wraps
import simplejson as json
@@ -496,7 +497,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
)
)
try:
- SQL, nameOrError = self.getSQL(gid, sid, did, data)
+ SQL, nameOrError = self.getSQL(gid, sid, did, scid, data)
if SQL is None:
return nameOrError
SQL = SQL.strip('\n').strip(' ')
@@ -541,7 +542,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
request.data, encoding='utf-8'
)
try:
- SQL, name = self.getSQL(gid, sid, did, data, vid)
+ SQL, name = self.getSQL(gid, sid, did, scid, data, vid)
if SQL is None:
return name
SQL = SQL.strip('\n').strip(' ')
@@ -678,7 +679,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
except ValueError:
data[k] = v
- sql, nameOrError = self.getSQL(gid, sid, did, data, vid)
+ sql, nameOrError = self.getSQL(gid, sid, did, scid, data, vid)
if sql is None:
return nameOrError
@@ -692,7 +693,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
status=200
)
- def getSQL(self, gid, sid, did, data, vid=None):
+ def getSQL(self, gid, sid, did, scid, data, vid=None):
"""
This function will generate sql from model data
"""
@@ -716,7 +717,20 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
if 'schema' not in data:
data['schema'] = res['rows'][0]['schema']
- acls = []
+ DEL_SQL = None
+ if 'definition' in data:
+ new_def = re.sub(r"\W", "", data['definition']).split('FROM')
+ old_def = re.sub(r"\W", "", res['rows'][0]['definition']
+ ).split('FROM')
+ if 'definition' in data and (
+ old_def[0] != new_def[0] and
+ old_def[0] not in new_def[0]
+ ):
+ DEL_SQL = self.delete(gid=gid, sid=sid, did=did,
+ scid=scid,
+ vid=vid, only_sql=True
+ )
+
try:
acls = render_template(
"/".join([self.template_path, 'sql/allowed_privs.json'])
@@ -740,6 +754,9 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
SQL = render_template("/".join(
[self.template_path, 'sql/update.sql']), data=data,
o_data=old_data, conn=self.conn)
+
+ if DEL_SQL:
+ SQL = DEL_SQL + SQL
except Exception as e:
current_app.logger.exception(e)
return None, internal_server_error(errormsg=str(e))
@@ -1439,7 +1456,14 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
if data:
if diff_schema:
data['schema'] = diff_schema
- sql, nameOrError = self.getSQL(gid, sid, did, data, oid)
+ sql, nameOrError = self.getSQL(gid, sid, did, scid, data, oid)
+ if sql.find('DROP VIEW') != -1:
+ sql = gettext("""
+-- Changing the columns in a view requires dropping and re-creating the view.
+-- This may fail if other objects are dependent upon this view,
+-- or may cause procedural functions to fail if they are not modified to
+-- take account of the changes.
+""") + sql
else:
if drop_sql:
sql = self.delete(gid=gid, sid=sid, did=did,
@@ -1547,8 +1571,8 @@ class MViewNode(ViewNode, VacuumSettings):
data['vacuum_data']['reset'] = []
# table vacuum: separate list of changed and reset data for
- if ('vacuum_table' in data):
- if ('changed' in data['vacuum_table']):
+ if 'vacuum_table' in data:
+ if 'changed' in data['vacuum_table']:
for item in data['vacuum_table']['changed']:
if 'value' in item.keys():
if item['value'] is None:
@@ -1581,8 +1605,8 @@ class MViewNode(ViewNode, VacuumSettings):
'value': data['autovacuum_enabled']})
# toast autovacuum: separate list of changed and reset data
- if ('vacuum_toast' in data):
- if ('changed' in data['vacuum_toast']):
+ if 'vacuum_toast' in data:
+ if 'changed' in data['vacuum_toast']:
for item in data['vacuum_toast']['changed']:
if 'value' in item.keys():
toast_key = 'toast_' + item['name']
@@ -1671,14 +1695,14 @@ class MViewNode(ViewNode, VacuumSettings):
if 'value' in item.keys() and item['value'] is not None]
# add table_enabled & toast_enabled settings
- if ('autovacuum_custom' in data and data['autovacuum_custom']):
+ if 'autovacuum_custom' in data and data['autovacuum_custom']:
vacuum_table.append(
{
'name': 'autovacuum_enabled',
'value': str(data['autovacuum_enabled'])
}
)
- if ('toast_autovacuum' in data and data['toast_autovacuum']):
+ if 'toast_autovacuum' in data and data['toast_autovacuum']:
vacuum_table.append(
{
'name': 'toast.autovacuum_enabled',
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js
index 72ea59c13..49237089a 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js
@@ -154,8 +154,29 @@ define('pgadmin.node.view', [
id: 'definition', label: gettext('Code'), cell: 'string',
type: 'text', mode: ['create', 'edit'], group: gettext('Code'),
tabPanelCodeClass: 'sql-code-control',
- control: Backform.SqlCodeControl,
disabled: 'notInSchema',
+ control: Backform.SqlCodeControl.extend({
+ onChange: function() {
+ Backform.SqlCodeControl.prototype.onChange.apply(this, arguments);
+ if(this.model && this.model.changed) {
+ if(this.model.origSessAttrs && this.model.changed.definition !== undefined && (this.model.changed.definition != this.model.origSessAttrs.definition)) {
+ let old_def = this.model.origSessAttrs.definition.replace(/\s/gi, '').split('FROM'),
+ new_def = this.model.changed.definition.replace(/\s/gi, '').split('FROM');
+ if (old_def[0] != new_def[0]) {
+ this.model.warn_text = gettext('Changing the columns in a view requires dropping and re-creating the view. This may fail if other objects are dependent upon this view, or may cause procedural functions to fail if they are not modified to take account of the changes. Do you wish to continue?');
+ } else {
+ this.model.warn_text = undefined;
+ }
+ }
+ else {
+ this.model.warn_text = undefined;
+ }
+ }
+ else {
+ this.model.warn_text = undefined;
+ }
+ },
+ }),
}, pgBrowser.SecurityGroupSchema, {
// Add Privilege Control
id: 'datacl', label: gettext('Privileges'), type: 'collection',
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view
2020-01-14 04:57 [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-01-14 06:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-03-24 08:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-03-24 09:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-04-07 04:30 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
@ 2020-04-07 06:12 ` Akshay Joshi <[email protected]>
2020-04-08 05:28 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Akshay Joshi @ 2020-04-07 06:12 UTC (permalink / raw)
To: Khushboo Vashi <[email protected]>; +Cc: pgadmin-hackers
Hi Khushboo
The warning message is not showing up. Please fix and resend the patch.
On Tue, Apr 7, 2020 at 10:00 AM Khushboo Vashi <
[email protected]> wrote:
> Hi Akshay,
>
> Please find the attached updated patch.
>
> On Tue, Mar 24, 2020 at 2:47 PM Akshay Joshi <
> [email protected]> wrote:
>
>> Hi Khushboo
>>
>> On Tue, Mar 24, 2020 at 1:47 PM Khushboo Vashi <
>> [email protected]> wrote:
>>
>>> Hi Akshay,
>>>
>>> On Tue, Jan 14, 2020 at 11:47 AM Akshay Joshi <
>>> [email protected]> wrote:
>>>
>>>> Hi Khushboo
>>>>
>>>> Following are the review comments:
>>>>
>>>> - Fix the PEP8 issue.
>>>> - Drop query should be part of the jinja template for consistency.
>>>> Currently, it is added through the python file.
>>>>
>>>> The Delete query is already in the template file, I have just reused
>>> the delete call and merged the SQL queries in the python file.
>>>
>>>>
>>>> - Any changes in the view code should not warn the user "Changing
>>>> the columns in a view requires dropping...." and we should not drop the
>>>> view. For example, I have only change the WHERE clause or added 'ORDER BY'.
>>>>
>>>> I have tested but couldn't reproduce this issue. Can you please let
>>> me know the proper use case?
>>>
>>
>> Create a view with 'SELECT 1;' as code. Then change the code to
>> 'SELECT 1234;' and click on the Save button.
>> Warning popup is displayed "Changing the columns in a view....". Click
>> on the 'Yes' button and check the OID of the view. You will get the same
>> OID, it means view is not recreated.
>>
>>
> I can reproduce this issue with the given SQL but the problem is as per
> the PostgreSQL documentation, (Ref:
> https://www.postgresql.org/docs/12/sql-createview.html)
>
> "CREATE OR REPLACE VIEW is similar, but if a view of the same name
> already exists, it is replaced. The new query must generate the same
> columns that were generated by the existing view query (that is, the same
> column names in the same order and with the same data types), but it may
> add additional columns to the end of the list. The calculations giving rise
> to the output columns may be completely different."
>
> So, I put a check on the columns and if the column is changed, the message
> will popup.
>
> In case of the example given by you, the column name is not changed as if
> you don't give the column name it will be default and I think view would
> have the column names properly.
>
>
>> I have observed below error in the browser while changing the code:
>> view.js:241 Uncaught TypeError: Cannot read property
>> 'replace' of undefined
>> at child.onChange (view.js:241)
>> at HTMLDivElement.dispatch (jquery.js:5237)
>> at HTMLDivElement.elemData.handle (jquery.js:5044)
>>
>> Fixed.
>
> Thanks,
> Khushboo
>
>>
>>> Thanks,
>>> Khushboo
>>>
>>>>
>>>>
>>>
>>>> On Tue, Jan 14, 2020 at 10:27 AM Khushboo Vashi <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Please find the attached patch for RM #5053 - Getting an error while
>>>>> changing the columns in the existing view.
>>>>>
>>>>> PostgreSQL doesn't allow to change the view columns. So, while
>>>>> performing this task the existing view should be dropped first and then
>>>>> recreate it and also user will get a warning first.
>>>>>
>>>>> Thanks,
>>>>> Khushboo
>>>>>
>>>>
>>>>
>>>> --
>>>> *Thanks & Regards*
>>>> *Akshay Joshi*
>>>>
>>>> *Sr. Software Architect*
>>>> *EnterpriseDB Software India Private Limited*
>>>> *Mobile: +91 976-788-8246*
>>>>
>>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>>
>> *Sr. Software Architect*
>> *EnterpriseDB Software India Private Limited*
>> *Mobile: +91 976-788-8246*
>>
>
--
*Thanks & Regards*
*Akshay Joshi*
*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view
2020-01-14 04:57 [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-01-14 06:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-03-24 08:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-03-24 09:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-04-07 04:30 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-04-07 06:12 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
@ 2020-04-08 05:28 ` Khushboo Vashi <[email protected]>
2020-04-08 06:12 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Khushboo Vashi @ 2020-04-08 05:28 UTC (permalink / raw)
To: Akshay Joshi <[email protected]>; +Cc: pgadmin-hackers
Hi Akshay,
Please find the attached updated patch.
Thanks,
Khushboo
On Tue, Apr 7, 2020 at 11:43 AM Akshay Joshi <[email protected]>
wrote:
> Hi Khushboo
>
> The warning message is not showing up. Please fix and resend the patch.
>
> On Tue, Apr 7, 2020 at 10:00 AM Khushboo Vashi <
> [email protected]> wrote:
>
>> Hi Akshay,
>>
>> Please find the attached updated patch.
>>
>> On Tue, Mar 24, 2020 at 2:47 PM Akshay Joshi <
>> [email protected]> wrote:
>>
>>> Hi Khushboo
>>>
>>> On Tue, Mar 24, 2020 at 1:47 PM Khushboo Vashi <
>>> [email protected]> wrote:
>>>
>>>> Hi Akshay,
>>>>
>>>> On Tue, Jan 14, 2020 at 11:47 AM Akshay Joshi <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi Khushboo
>>>>>
>>>>> Following are the review comments:
>>>>>
>>>>> - Fix the PEP8 issue.
>>>>> - Drop query should be part of the jinja template for consistency.
>>>>> Currently, it is added through the python file.
>>>>>
>>>>> The Delete query is already in the template file, I have just reused
>>>> the delete call and merged the SQL queries in the python file.
>>>>
>>>>>
>>>>> - Any changes in the view code should not warn the user "Changing
>>>>> the columns in a view requires dropping...." and we should not drop the
>>>>> view. For example, I have only change the WHERE clause or added 'ORDER BY'.
>>>>>
>>>>> I have tested but couldn't reproduce this issue. Can you please let
>>>> me know the proper use case?
>>>>
>>>
>>> Create a view with 'SELECT 1;' as code. Then change the code to
>>> 'SELECT 1234;' and click on the Save button.
>>> Warning popup is displayed "Changing the columns in a view....".
>>> Click on the 'Yes' button and check the OID of the view. You will get the
>>> same OID, it means view is not recreated.
>>>
>>>
>> I can reproduce this issue with the given SQL but the problem is as per
>> the PostgreSQL documentation, (Ref:
>> https://www.postgresql.org/docs/12/sql-createview.html)
>>
>> "CREATE OR REPLACE VIEW is similar, but if a view of the same name
>> already exists, it is replaced. The new query must generate the same
>> columns that were generated by the existing view query (that is, the same
>> column names in the same order and with the same data types), but it may
>> add additional columns to the end of the list. The calculations giving rise
>> to the output columns may be completely different."
>>
>> So, I put a check on the columns and if the column is changed, the
>> message will popup.
>>
>> In case of the example given by you, the column name is not changed as if
>> you don't give the column name it will be default and I think view would
>> have the column names properly.
>>
>>
>>> I have observed below error in the browser while changing the code:
>>> view.js:241 Uncaught TypeError: Cannot read property
>>> 'replace' of undefined
>>> at child.onChange (view.js:241)
>>> at HTMLDivElement.dispatch (jquery.js:5237)
>>> at HTMLDivElement.elemData.handle (jquery.js:5044)
>>>
>>> Fixed.
>>
>> Thanks,
>> Khushboo
>>
>>>
>>>> Thanks,
>>>> Khushboo
>>>>
>>>>>
>>>>>
>>>>
>>>>> On Tue, Jan 14, 2020 at 10:27 AM Khushboo Vashi <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Please find the attached patch for RM #5053 - Getting an error while
>>>>>> changing the columns in the existing view.
>>>>>>
>>>>>> PostgreSQL doesn't allow to change the view columns. So, while
>>>>>> performing this task the existing view should be dropped first and then
>>>>>> recreate it and also user will get a warning first.
>>>>>>
>>>>>> Thanks,
>>>>>> Khushboo
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> *Thanks & Regards*
>>>>> *Akshay Joshi*
>>>>>
>>>>> *Sr. Software Architect*
>>>>> *EnterpriseDB Software India Private Limited*
>>>>> *Mobile: +91 976-788-8246*
>>>>>
>>>>
>>>
>>> --
>>> *Thanks & Regards*
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect*
>>> *EnterpriseDB Software India Private Limited*
>>> *Mobile: +91 976-788-8246*
>>>
>>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
>
> *Sr. Software Architect*
> *EnterpriseDB Software India Private Limited*
> *Mobile: +91 976-788-8246*
>
Attachments:
[application/octet-stream] RM_5053_v2.patch (8.5K, 3-RM_5053_v2.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py
index fb1ad76ac..e9a1f38fe 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py
@@ -10,6 +10,7 @@
"""Implements View and Materialized View Node"""
import copy
+import re
from functools import wraps
import simplejson as json
@@ -496,7 +497,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
)
)
try:
- SQL, nameOrError = self.getSQL(gid, sid, did, data)
+ SQL, nameOrError = self.getSQL(gid, sid, did, scid, data)
if SQL is None:
return nameOrError
SQL = SQL.strip('\n').strip(' ')
@@ -541,7 +542,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
request.data, encoding='utf-8'
)
try:
- SQL, name = self.getSQL(gid, sid, did, data, vid)
+ SQL, name = self.getSQL(gid, sid, did, scid, data, vid)
if SQL is None:
return name
SQL = SQL.strip('\n').strip(' ')
@@ -678,7 +679,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
except ValueError:
data[k] = v
- sql, nameOrError = self.getSQL(gid, sid, did, data, vid)
+ sql, nameOrError = self.getSQL(gid, sid, did, scid, data, vid)
if sql is None:
return nameOrError
@@ -692,7 +693,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
status=200
)
- def getSQL(self, gid, sid, did, data, vid=None):
+ def getSQL(self, gid, sid, did, scid, data, vid=None):
"""
This function will generate sql from model data
"""
@@ -716,7 +717,22 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
if 'schema' not in data:
data['schema'] = res['rows'][0]['schema']
- acls = []
+ DEL_SQL = None
+ if 'definition' in data:
+ new_def = re.sub(r"\W", "", data['definition']).split('FROM')
+ old_def = re.sub(r"\W", "", res['rows'][0]['definition']
+ ).split('FROM')
+ if 'definition' in data and (
+ len(old_def) > 1 or len(new_def) > 1
+ ) and(
+ old_def[0] != new_def[0] and
+ old_def[0] not in new_def[0]
+ ):
+ DEL_SQL = self.delete(gid=gid, sid=sid, did=did,
+ scid=scid,
+ vid=vid, only_sql=True
+ )
+
try:
acls = render_template(
"/".join([self.template_path, 'sql/allowed_privs.json'])
@@ -740,6 +756,9 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
SQL = render_template("/".join(
[self.template_path, 'sql/update.sql']), data=data,
o_data=old_data, conn=self.conn)
+
+ if DEL_SQL:
+ SQL = DEL_SQL + SQL
except Exception as e:
current_app.logger.exception(e)
return None, internal_server_error(errormsg=str(e))
@@ -1439,7 +1458,14 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
if data:
if diff_schema:
data['schema'] = diff_schema
- sql, nameOrError = self.getSQL(gid, sid, did, data, oid)
+ sql, nameOrError = self.getSQL(gid, sid, did, scid, data, oid)
+ if sql.find('DROP VIEW') != -1:
+ sql = gettext("""
+-- Changing the columns in a view requires dropping and re-creating the view.
+-- This may fail if other objects are dependent upon this view,
+-- or may cause procedural functions to fail if they are not modified to
+-- take account of the changes.
+""") + sql
else:
if drop_sql:
sql = self.delete(gid=gid, sid=sid, did=did,
@@ -1547,8 +1573,8 @@ class MViewNode(ViewNode, VacuumSettings):
data['vacuum_data']['reset'] = []
# table vacuum: separate list of changed and reset data for
- if ('vacuum_table' in data):
- if ('changed' in data['vacuum_table']):
+ if 'vacuum_table' in data:
+ if 'changed' in data['vacuum_table']:
for item in data['vacuum_table']['changed']:
if 'value' in item.keys():
if item['value'] is None:
@@ -1581,8 +1607,8 @@ class MViewNode(ViewNode, VacuumSettings):
'value': data['autovacuum_enabled']})
# toast autovacuum: separate list of changed and reset data
- if ('vacuum_toast' in data):
- if ('changed' in data['vacuum_toast']):
+ if 'vacuum_toast' in data:
+ if 'changed' in data['vacuum_toast']:
for item in data['vacuum_toast']['changed']:
if 'value' in item.keys():
toast_key = 'toast_' + item['name']
@@ -1671,14 +1697,14 @@ class MViewNode(ViewNode, VacuumSettings):
if 'value' in item.keys() and item['value'] is not None]
# add table_enabled & toast_enabled settings
- if ('autovacuum_custom' in data and data['autovacuum_custom']):
+ if 'autovacuum_custom' in data and data['autovacuum_custom']:
vacuum_table.append(
{
'name': 'autovacuum_enabled',
'value': str(data['autovacuum_enabled'])
}
)
- if ('toast_autovacuum' in data and data['toast_autovacuum']):
+ if 'toast_autovacuum' in data and data['toast_autovacuum']:
vacuum_table.append(
{
'name': 'toast.autovacuum_enabled',
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js
index 72ea59c13..ef8ac74cb 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js
@@ -154,8 +154,33 @@ define('pgadmin.node.view', [
id: 'definition', label: gettext('Code'), cell: 'string',
type: 'text', mode: ['create', 'edit'], group: gettext('Code'),
tabPanelCodeClass: 'sql-code-control',
- control: Backform.SqlCodeControl,
disabled: 'notInSchema',
+ control: Backform.SqlCodeControl.extend({
+ onChange: function() {
+ Backform.SqlCodeControl.prototype.onChange.apply(this, arguments);
+ if(this.model && this.model.changed) {
+ if(this.model.origSessAttrs && (this.model.changed.definition != this.model.origSessAttrs.definition)) {
+ let old_def = this.model.origSessAttrs.definition.replace(/\s/gi, '').split('FROM'),
+ new_def = [];
+
+ if(this.model.changed.definition !== undefined) {
+ new_def = this.model.changed.definition.replace(/\s/gi, '').split('FROM');
+ }
+ if ((old_def.length > 1 || new_def.length > 1) && old_def[0] != new_def[0] && !new_def[0].includes(old_def[0])) {
+ this.model.warn_text = gettext('Changing the columns in a view requires dropping and re-creating the view. This may fail if other objects are dependent upon this view, or may cause procedural functions to fail if they are not modified to take account of the changes. Do you wish to continue?');
+ } else {
+ this.model.warn_text = undefined;
+ }
+ }
+ else {
+ this.model.warn_text = undefined;
+ }
+ }
+ else {
+ this.model.warn_text = undefined;
+ }
+ },
+ }),
}, pgBrowser.SecurityGroupSchema, {
// Add Privilege Control
id: 'datacl', label: gettext('Privileges'), type: 'collection',
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view
2020-01-14 04:57 [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-01-14 06:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-03-24 08:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-03-24 09:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-04-07 04:30 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-04-07 06:12 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-04-08 05:28 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
@ 2020-04-08 06:12 ` Akshay Joshi <[email protected]>
2020-04-13 08:59 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Akshay Joshi @ 2020-04-08 06:12 UTC (permalink / raw)
To: Khushboo Vashi <[email protected]>; +Cc: pgadmin-hackers
Thanks, patch applied.
On Wed, Apr 8, 2020 at 10:58 AM Khushboo Vashi <
[email protected]> wrote:
> Hi Akshay,
>
> Please find the attached updated patch.
>
> Thanks,
> Khushboo
>
> On Tue, Apr 7, 2020 at 11:43 AM Akshay Joshi <
> [email protected]> wrote:
>
>> Hi Khushboo
>>
>> The warning message is not showing up. Please fix and resend the patch.
>>
>> On Tue, Apr 7, 2020 at 10:00 AM Khushboo Vashi <
>> [email protected]> wrote:
>>
>>> Hi Akshay,
>>>
>>> Please find the attached updated patch.
>>>
>>> On Tue, Mar 24, 2020 at 2:47 PM Akshay Joshi <
>>> [email protected]> wrote:
>>>
>>>> Hi Khushboo
>>>>
>>>> On Tue, Mar 24, 2020 at 1:47 PM Khushboo Vashi <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi Akshay,
>>>>>
>>>>> On Tue, Jan 14, 2020 at 11:47 AM Akshay Joshi <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Hi Khushboo
>>>>>>
>>>>>> Following are the review comments:
>>>>>>
>>>>>> - Fix the PEP8 issue.
>>>>>> - Drop query should be part of the jinja template for
>>>>>> consistency. Currently, it is added through the python file.
>>>>>>
>>>>>> The Delete query is already in the template file, I have just reused
>>>>> the delete call and merged the SQL queries in the python file.
>>>>>
>>>>>>
>>>>>> - Any changes in the view code should not warn the user "Changing
>>>>>> the columns in a view requires dropping...." and we should not drop the
>>>>>> view. For example, I have only change the WHERE clause or added 'ORDER BY'.
>>>>>>
>>>>>> I have tested but couldn't reproduce this issue. Can you please let
>>>>> me know the proper use case?
>>>>>
>>>>
>>>> Create a view with 'SELECT 1;' as code. Then change the code to
>>>> 'SELECT 1234;' and click on the Save button.
>>>> Warning popup is displayed "Changing the columns in a view....".
>>>> Click on the 'Yes' button and check the OID of the view. You will get the
>>>> same OID, it means view is not recreated.
>>>>
>>>>
>>> I can reproduce this issue with the given SQL but the problem is as per
>>> the PostgreSQL documentation, (Ref:
>>> https://www.postgresql.org/docs/12/sql-createview.html)
>>>
>>> "CREATE OR REPLACE VIEW is similar, but if a view of the same name
>>> already exists, it is replaced. The new query must generate the same
>>> columns that were generated by the existing view query (that is, the same
>>> column names in the same order and with the same data types), but it may
>>> add additional columns to the end of the list. The calculations giving rise
>>> to the output columns may be completely different."
>>>
>>> So, I put a check on the columns and if the column is changed, the
>>> message will popup.
>>>
>>> In case of the example given by you, the column name is not changed as
>>> if you don't give the column name it will be default and I think view would
>>> have the column names properly.
>>>
>>>
>>>> I have observed below error in the browser while changing the code:
>>>> view.js:241 Uncaught TypeError: Cannot read property
>>>> 'replace' of undefined
>>>> at child.onChange (view.js:241)
>>>> at HTMLDivElement.dispatch (jquery.js:5237)
>>>> at HTMLDivElement.elemData.handle (jquery.js:5044)
>>>>
>>>> Fixed.
>>>
>>> Thanks,
>>> Khushboo
>>>
>>>>
>>>>> Thanks,
>>>>> Khushboo
>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>> On Tue, Jan 14, 2020 at 10:27 AM Khushboo Vashi <
>>>>>> [email protected]> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Please find the attached patch for RM #5053 - Getting an error while
>>>>>>> changing the columns in the existing view.
>>>>>>>
>>>>>>> PostgreSQL doesn't allow to change the view columns. So, while
>>>>>>> performing this task the existing view should be dropped first and then
>>>>>>> recreate it and also user will get a warning first.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Khushboo
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> *Thanks & Regards*
>>>>>> *Akshay Joshi*
>>>>>>
>>>>>> *Sr. Software Architect*
>>>>>> *EnterpriseDB Software India Private Limited*
>>>>>> *Mobile: +91 976-788-8246*
>>>>>>
>>>>>
>>>>
>>>> --
>>>> *Thanks & Regards*
>>>> *Akshay Joshi*
>>>>
>>>> *Sr. Software Architect*
>>>> *EnterpriseDB Software India Private Limited*
>>>> *Mobile: +91 976-788-8246*
>>>>
>>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>>
>> *Sr. Software Architect*
>> *EnterpriseDB Software India Private Limited*
>> *Mobile: +91 976-788-8246*
>>
>
--
*Thanks & Regards*
*Akshay Joshi*
*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view
2020-01-14 04:57 [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-01-14 06:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-03-24 08:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-03-24 09:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-04-07 04:30 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-04-07 06:12 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-04-08 05:28 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-04-08 06:12 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
@ 2020-04-13 08:59 ` Khushboo Vashi <[email protected]>
2020-04-13 14:55 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Khushboo Vashi @ 2020-04-13 08:59 UTC (permalink / raw)
To: Akshay Joshi <[email protected]>; +Cc: pgadmin-hackers
Hi,
Please find the attached patch to fix the test cases due to this patch.
Also, this functionality will not be applicable on EPAS server as we can
change the view definition without dropping it.
Thanks,
Khushboo
On Wed, Apr 8, 2020 at 11:42 AM Akshay Joshi <[email protected]>
wrote:
> Thanks, patch applied.
>
> On Wed, Apr 8, 2020 at 10:58 AM Khushboo Vashi <
> [email protected]> wrote:
>
>> Hi Akshay,
>>
>> Please find the attached updated patch.
>>
>> Thanks,
>> Khushboo
>>
>> On Tue, Apr 7, 2020 at 11:43 AM Akshay Joshi <
>> [email protected]> wrote:
>>
>>> Hi Khushboo
>>>
>>> The warning message is not showing up. Please fix and resend the patch.
>>>
>>> On Tue, Apr 7, 2020 at 10:00 AM Khushboo Vashi <
>>> [email protected]> wrote:
>>>
>>>> Hi Akshay,
>>>>
>>>> Please find the attached updated patch.
>>>>
>>>> On Tue, Mar 24, 2020 at 2:47 PM Akshay Joshi <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi Khushboo
>>>>>
>>>>> On Tue, Mar 24, 2020 at 1:47 PM Khushboo Vashi <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Hi Akshay,
>>>>>>
>>>>>> On Tue, Jan 14, 2020 at 11:47 AM Akshay Joshi <
>>>>>> [email protected]> wrote:
>>>>>>
>>>>>>> Hi Khushboo
>>>>>>>
>>>>>>> Following are the review comments:
>>>>>>>
>>>>>>> - Fix the PEP8 issue.
>>>>>>> - Drop query should be part of the jinja template for
>>>>>>> consistency. Currently, it is added through the python file.
>>>>>>>
>>>>>>> The Delete query is already in the template file, I have just reused
>>>>>> the delete call and merged the SQL queries in the python file.
>>>>>>
>>>>>>>
>>>>>>> - Any changes in the view code should not warn the user
>>>>>>> "Changing the columns in a view requires dropping...." and we should not
>>>>>>> drop the view. For example, I have only change the WHERE clause or added
>>>>>>> 'ORDER BY'.
>>>>>>>
>>>>>>> I have tested but couldn't reproduce this issue. Can you please
>>>>>> let me know the proper use case?
>>>>>>
>>>>>
>>>>> Create a view with 'SELECT 1;' as code. Then change the code to
>>>>> 'SELECT 1234;' and click on the Save button.
>>>>> Warning popup is displayed "Changing the columns in a view....".
>>>>> Click on the 'Yes' button and check the OID of the view. You will get the
>>>>> same OID, it means view is not recreated.
>>>>>
>>>>>
>>>> I can reproduce this issue with the given SQL but the problem is as per
>>>> the PostgreSQL documentation, (Ref:
>>>> https://www.postgresql.org/docs/12/sql-createview.html)
>>>>
>>>> "CREATE OR REPLACE VIEW is similar, but if a view of the same name
>>>> already exists, it is replaced. The new query must generate the same
>>>> columns that were generated by the existing view query (that is, the same
>>>> column names in the same order and with the same data types), but it may
>>>> add additional columns to the end of the list. The calculations giving rise
>>>> to the output columns may be completely different."
>>>>
>>>> So, I put a check on the columns and if the column is changed, the
>>>> message will popup.
>>>>
>>>> In case of the example given by you, the column name is not changed as
>>>> if you don't give the column name it will be default and I think view would
>>>> have the column names properly.
>>>>
>>>>
>>>>> I have observed below error in the browser while changing the
>>>>> code:
>>>>> view.js:241 Uncaught TypeError: Cannot read property
>>>>> 'replace' of undefined
>>>>> at child.onChange (view.js:241)
>>>>> at HTMLDivElement.dispatch (jquery.js:5237)
>>>>> at HTMLDivElement.elemData.handle (jquery.js:5044)
>>>>>
>>>>> Fixed.
>>>>
>>>> Thanks,
>>>> Khushboo
>>>>
>>>>>
>>>>>> Thanks,
>>>>>> Khushboo
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>> On Tue, Jan 14, 2020 at 10:27 AM Khushboo Vashi <
>>>>>>> [email protected]> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Please find the attached patch for RM #5053 - Getting an error
>>>>>>>> while changing the columns in the existing view.
>>>>>>>>
>>>>>>>> PostgreSQL doesn't allow to change the view columns. So, while
>>>>>>>> performing this task the existing view should be dropped first and then
>>>>>>>> recreate it and also user will get a warning first.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Khushboo
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> *Thanks & Regards*
>>>>>>> *Akshay Joshi*
>>>>>>>
>>>>>>> *Sr. Software Architect*
>>>>>>> *EnterpriseDB Software India Private Limited*
>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> *Thanks & Regards*
>>>>> *Akshay Joshi*
>>>>>
>>>>> *Sr. Software Architect*
>>>>> *EnterpriseDB Software India Private Limited*
>>>>> *Mobile: +91 976-788-8246*
>>>>>
>>>>
>>>
>>> --
>>> *Thanks & Regards*
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect*
>>> *EnterpriseDB Software India Private Limited*
>>> *Mobile: +91 976-788-8246*
>>>
>>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
>
> *Sr. Software Architect*
> *EnterpriseDB Software India Private Limited*
> *Mobile: +91 976-788-8246*
>
Attachments:
[application/octet-stream] RM_5053_test_case_fixes.patch (13.5K, 3-RM_5053_test_case_fixes.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py
index aa03b02b0..955af8bc1 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py
@@ -496,7 +496,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
"Could not find the required parameter (%s).") % arg
)
try:
- SQL, nameOrError = self.getSQL(gid, sid, did, scid, data)
+ SQL, nameOrError = self.getSQL(gid, sid, did, data)
if SQL is None:
return nameOrError
SQL = SQL.strip('\n').strip(' ')
@@ -541,7 +541,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
request.data, encoding='utf-8'
)
try:
- SQL, name = self.getSQL(gid, sid, did, scid, data, vid)
+ SQL, name = self.getSQL(gid, sid, did, data, vid)
if SQL is None:
return name
SQL = SQL.strip('\n').strip(' ')
@@ -678,7 +678,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
except ValueError:
data[k] = v
- sql, nameOrError = self.getSQL(gid, sid, did, scid, data, vid)
+ sql, nameOrError = self.getSQL(gid, sid, did, data, vid)
if sql is None:
return nameOrError
@@ -692,7 +692,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
status=200
)
- def getSQL(self, gid, sid, did, scid, data, vid=None):
+ def getSQL(self, gid, sid, did, data, vid=None):
"""
This function will generate sql from model data
"""
@@ -716,22 +716,6 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
if 'schema' not in data:
data['schema'] = res['rows'][0]['schema']
- DEL_SQL = None
- if 'definition' in data:
- new_def = re.sub(r"\W", "", data['definition']).split('FROM')
- old_def = re.sub(r"\W", "", res['rows'][0]['definition']
- ).split('FROM')
- if 'definition' in data and (
- len(old_def) > 1 or len(new_def) > 1
- ) and(
- old_def[0] != new_def[0] and
- old_def[0] not in new_def[0]
- ):
- DEL_SQL = self.delete(gid=gid, sid=sid, did=did,
- scid=scid,
- vid=vid, only_sql=True
- )
-
try:
acls = render_template(
"/".join([self.template_path, 'sql/allowed_privs.json'])
@@ -750,14 +734,53 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
data[aclcol][key] = parse_priv_to_db(
data[aclcol][key], allowedacl['acl']
)
+ data['del_sql'] = False
+ old_data['acl_sql'] = ''
+
+ if 'definition' in data and self.manager.server_type == 'pg':
+ new_def = re.sub(r"\W", "", data['definition']).split('FROM')
+ old_def = re.sub(r"\W", "", res['rows'][0]['definition']
+ ).split('FROM')
+ if 'definition' in data and (
+ len(old_def) > 1 or len(new_def) > 1
+ ) and(
+ old_def[0] != new_def[0] and
+ old_def[0] not in new_def[0]
+ ):
+ data['del_sql'] = True
+
+ # If we drop and recreate the view, the
+ # privileges must be restored
+
+ # Fetch all privileges for view
+ sql_acl = render_template("/".join(
+ [self.template_path, 'sql/acl.sql']), vid=vid)
+ status, dataclres = self.conn.execute_dict(sql_acl)
+ if not status:
+ return internal_server_error(errormsg=res)
+
+ for row in dataclres['rows']:
+ priv = parse_priv_from_db(row)
+ res['rows'][0].setdefault(row['deftype'], []
+ ).append(priv)
+
+ old_data.update(res['rows'][0])
+
+ # Privileges
+ for aclcol in acls:
+ if aclcol in old_data:
+ allowedacl = acls[aclcol]
+ old_data[aclcol] = parse_priv_to_db(
+ old_data[aclcol], allowedacl['acl'])
+
+ old_data['acl_sql'] = render_template("/".join(
+ [self.template_path, 'sql/grant.sql']), data=old_data)
try:
SQL = render_template("/".join(
[self.template_path, 'sql/update.sql']), data=data,
o_data=old_data, conn=self.conn)
- if DEL_SQL:
- SQL = DEL_SQL + SQL
except Exception as e:
current_app.logger.exception(e)
return None, internal_server_error(errormsg=str(e))
@@ -1457,7 +1480,7 @@ class ViewNode(PGChildNodeView, VacuumSettings, SchemaDiffObjectCompare):
if data:
if diff_schema:
data['schema'] = diff_schema
- sql, nameOrError = self.getSQL(gid, sid, did, scid, data, oid)
+ sql, nameOrError = self.getSQL(gid, sid, did, data, oid)
if sql.find('DROP VIEW') != -1:
sql = gettext("""
-- Changing the columns in a view requires dropping and re-creating the view.
@@ -1539,7 +1562,7 @@ class MViewNode(ViewNode, VacuumSettings):
'9.3_plus'
)
- def getSQL(self, gid, sid, did, scid, data, vid=None):
+ def getSQL(self, gid, sid, did, data, vid=None):
"""
This function will generate sql from model data
"""
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js
index ef8ac74cb..e7dacef47 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.js
@@ -158,7 +158,7 @@ define('pgadmin.node.view', [
control: Backform.SqlCodeControl.extend({
onChange: function() {
Backform.SqlCodeControl.prototype.onChange.apply(this, arguments);
- if(this.model && this.model.changed) {
+ if(this.model && this.model.changed && this.model.node_info.server.server_type == 'pg') {
if(this.model.origSessAttrs && (this.model.changed.definition != this.model.origSessAttrs.definition)) {
let old_def = this.model.origSessAttrs.definition.replace(/\s/gi, '').split('FROM'),
new_def = [];
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/views/pg/9.4_plus/sql/delete.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/views/pg/9.4_plus/sql/delete.sql
index b1c173f98..788a0c2a6 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/views/pg/9.4_plus/sql/delete.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/views/pg/9.4_plus/sql/delete.sql
@@ -9,5 +9,5 @@ LEFT JOIN pg_namespace nsp ON c.relnamespace = nsp.oid
WHERE
c.relfilenode = {{ vid }};
{% elif (name and nspname) %}
-DROP VIEW {{ conn|qtIdent(nspname, name) }} {% if cascade %} CASCADE {% endif %};
+DROP VIEW {{ conn|qtIdent(nspname, name) }}{% if cascade %} CASCADE {% endif %};
{% endif %}
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/views/pg/9.4_plus/sql/update.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/views/pg/9.4_plus/sql/update.sql
index 7576600c9..e328ad3cf 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/views/pg/9.4_plus/sql/update.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/views/pg/9.4_plus/sql/update.sql
@@ -13,11 +13,10 @@ ALTER VIEW {{ conn|qtIdent(o_data.schema, o_data.name) }}
ALTER VIEW {{ conn|qtIdent(o_data.schema, view_name ) }}
SET SCHEMA {{ conn|qtIdent(data.schema) }};
{% endif %}
-{% if data.owner and data.owner != o_data.owner %}
-ALTER TABLE {{ conn|qtIdent(view_schema, view_name) }}
- OWNER TO {{ conn|qtIdent(data.owner) }};
-{% endif %}
{% if def and def != o_data.definition.rstrip(';') %}
+{% if data.del_sql %}
+DROP VIEW {{ conn|qtIdent(view_schema, view_name) }};
+{% endif %}
CREATE OR REPLACE VIEW {{ conn|qtIdent(view_schema, view_name) }}
{% if ((data.check_option and data.check_option.lower() != 'no') or data.security_barrier) %}
WITH ({% if (data.check_option or o_data.check_option) %}check_option={{ data.check_option if data.check_option else o_data.check_option }}{{', ' }}{% endif %}security_barrier={{ data.security_barrier|lower if data.security_barrier is defined else o_data.security_barrier|default('false', 'true')|lower }})
@@ -36,13 +35,23 @@ ALTER VIEW {{ conn|qtIdent(view_schema, view_name) }}
ALTER VIEW {{ conn|qtIdent(view_schema, view_name) }} RESET (check_option);
{% endif %}
{% endif %}
+{% if data.owner and data.owner != o_data.owner %}
+ALTER TABLE {{ conn|qtIdent(view_schema, view_name) }}
+ OWNER TO {{ conn|qtIdent(data.owner) }};
+{% endif %}
{% set old_comment = o_data.comment|default('', true) %}
{% if (data.comment is defined and (data.comment != old_comment)) %}
COMMENT ON VIEW {{ conn|qtIdent(view_schema, view_name) }}
IS {{ data.comment|qtLiteral }};
+{% elif data.del_sql == True %}
+COMMENT ON VIEW {{ conn|qtIdent(view_schema, view_name) }}
+ IS {{ old_comment|qtLiteral }};
{% endif %}
{# The SQL generated below will change privileges #}
+{% if o_data.acl_sql and o_data.acl_sql != '' %}
+{{o_data['acl_sql']}}
+{% endif %}
{% if data.datacl %}
{% if 'deleted' in data.datacl %}
{% for priv in data.datacl.deleted %}
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/pg/9.4_plus/alter_view_add_some_priv_msql.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/pg/9.4_plus/alter_view_add_some_priv_msql.sql
index 285bb2268..dc05c982c 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/pg/9.4_plus/alter_view_add_some_priv_msql.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/pg/9.4_plus/alter_view_add_some_priv_msql.sql
@@ -1 +1,5 @@
+ALTER VIEW public."testview_$%{}[]()&*^!@""'`\/#"
+ SET (security_barrier=true);
+ALTER VIEW public."testview_$%{}[]()&*^!@""'`\/#"
+ SET (check_option=cascaded);
GRANT SELECT ON TABLE public."testview_$%{}[]()&*^!@""'`\/#" TO PUBLIC;
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/pg/9.4_plus/alter_view_definition_msql.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/pg/9.4_plus/alter_view_definition_msql.sql
index aa577020e..0c13d741f 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/pg/9.4_plus/alter_view_definition_msql.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/pg/9.4_plus/alter_view_definition_msql.sql
@@ -1,3 +1,7 @@
+DROP VIEW public."testview_$%{}[]()&*^!@""'`\/#";
CREATE OR REPLACE VIEW public."testview_$%{}[]()&*^!@""'`\/#"
AS
SELECT * FROM test_view_table;
+COMMENT ON VIEW public."testview_$%{}[]()&*^!@""'`\/#"
+ IS 'Testcomment-updated';
+GRANT ALL ON TABLE public."testview_$%{}[]()&*^!@""'`\/#" TO postgres;
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/pg/9.4_plus/tests.json b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/pg/9.4_plus/tests.json
index 180dbfad8..14e121653 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/pg/9.4_plus/tests.json
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/pg/9.4_plus/tests.json
@@ -57,6 +57,18 @@
"expected_sql_file": "alter_view.sql",
"expected_msql_file": "alter_view_msql.sql"
},
+ {
+ "type": "alter",
+ "name": "Alter View (changing code)",
+ "endpoint": "NODE-view.obj_id",
+ "sql_endpoint": "NODE-view.sql_id",
+ "msql_endpoint": "NODE-view.msql_id",
+ "data": {
+ "definition": "SELECT * FROM test_view_table;"
+ },
+ "expected_sql_file": "alter_view_definition.sql",
+ "expected_msql_file": "alter_view_definition_msql.sql"
+ },
{
"type": "alter",
"name": "Alter View (adding privileges)",
@@ -64,6 +76,12 @@
"sql_endpoint": "NODE-view.sql_id",
"msql_endpoint": "NODE-view.msql_id",
"data": {
+ "name": "testview_$%{}[]()&*^!@\"'`\\/#",
+ "owner": "postgres",
+ "schema": "public",
+ "check_option": "cascaded",
+ "security_barrier": true,
+ "comment":"Testcomment-updated",
"datacl":{
"added":[
{
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view
2020-01-14 04:57 [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-01-14 06:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-03-24 08:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-03-24 09:17 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-04-07 04:30 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-04-07 06:12 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-04-08 05:28 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-04-08 06:12 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Akshay Joshi <[email protected]>
2020-04-13 08:59 ` Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
@ 2020-04-13 14:55 ` Akshay Joshi <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Akshay Joshi @ 2020-04-13 14:55 UTC (permalink / raw)
To: Khushboo Vashi <[email protected]>; +Cc: pgadmin-hackers
Thanks, patch applied.
On Mon, Apr 13, 2020 at 2:29 PM Khushboo Vashi <
[email protected]> wrote:
> Hi,
>
> Please find the attached patch to fix the test cases due to this patch.
> Also, this functionality will not be applicable on EPAS server as we can
> change the view definition without dropping it.
>
> Thanks,
> Khushboo
>
> On Wed, Apr 8, 2020 at 11:42 AM Akshay Joshi <
> [email protected]> wrote:
>
>> Thanks, patch applied.
>>
>> On Wed, Apr 8, 2020 at 10:58 AM Khushboo Vashi <
>> [email protected]> wrote:
>>
>>> Hi Akshay,
>>>
>>> Please find the attached updated patch.
>>>
>>> Thanks,
>>> Khushboo
>>>
>>> On Tue, Apr 7, 2020 at 11:43 AM Akshay Joshi <
>>> [email protected]> wrote:
>>>
>>>> Hi Khushboo
>>>>
>>>> The warning message is not showing up. Please fix and resend the patch.
>>>>
>>>> On Tue, Apr 7, 2020 at 10:00 AM Khushboo Vashi <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi Akshay,
>>>>>
>>>>> Please find the attached updated patch.
>>>>>
>>>>> On Tue, Mar 24, 2020 at 2:47 PM Akshay Joshi <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Hi Khushboo
>>>>>>
>>>>>> On Tue, Mar 24, 2020 at 1:47 PM Khushboo Vashi <
>>>>>> [email protected]> wrote:
>>>>>>
>>>>>>> Hi Akshay,
>>>>>>>
>>>>>>> On Tue, Jan 14, 2020 at 11:47 AM Akshay Joshi <
>>>>>>> [email protected]> wrote:
>>>>>>>
>>>>>>>> Hi Khushboo
>>>>>>>>
>>>>>>>> Following are the review comments:
>>>>>>>>
>>>>>>>> - Fix the PEP8 issue.
>>>>>>>> - Drop query should be part of the jinja template for
>>>>>>>> consistency. Currently, it is added through the python file.
>>>>>>>>
>>>>>>>> The Delete query is already in the template file, I have just
>>>>>>> reused the delete call and merged the SQL queries in the python file.
>>>>>>>
>>>>>>>>
>>>>>>>> - Any changes in the view code should not warn the user
>>>>>>>> "Changing the columns in a view requires dropping...." and we should not
>>>>>>>> drop the view. For example, I have only change the WHERE clause or added
>>>>>>>> 'ORDER BY'.
>>>>>>>>
>>>>>>>> I have tested but couldn't reproduce this issue. Can you please
>>>>>>> let me know the proper use case?
>>>>>>>
>>>>>>
>>>>>> Create a view with 'SELECT 1;' as code. Then change the code to
>>>>>> 'SELECT 1234;' and click on the Save button.
>>>>>> Warning popup is displayed "Changing the columns in a view....".
>>>>>> Click on the 'Yes' button and check the OID of the view. You will get the
>>>>>> same OID, it means view is not recreated.
>>>>>>
>>>>>>
>>>>> I can reproduce this issue with the given SQL but the problem is as
>>>>> per the PostgreSQL documentation, (Ref:
>>>>> https://www.postgresql.org/docs/12/sql-createview.html)
>>>>>
>>>>> "CREATE OR REPLACE VIEW is similar, but if a view of the same name
>>>>> already exists, it is replaced. The new query must generate the same
>>>>> columns that were generated by the existing view query (that is, the same
>>>>> column names in the same order and with the same data types), but it may
>>>>> add additional columns to the end of the list. The calculations giving rise
>>>>> to the output columns may be completely different."
>>>>>
>>>>> So, I put a check on the columns and if the column is changed, the
>>>>> message will popup.
>>>>>
>>>>> In case of the example given by you, the column name is not changed as
>>>>> if you don't give the column name it will be default and I think view would
>>>>> have the column names properly.
>>>>>
>>>>>
>>>>>> I have observed below error in the browser while changing the
>>>>>> code:
>>>>>> view.js:241 Uncaught TypeError: Cannot read property
>>>>>> 'replace' of undefined
>>>>>> at child.onChange (view.js:241)
>>>>>> at HTMLDivElement.dispatch (jquery.js:5237)
>>>>>> at HTMLDivElement.elemData.handle (jquery.js:5044)
>>>>>>
>>>>>> Fixed.
>>>>>
>>>>> Thanks,
>>>>> Khushboo
>>>>>
>>>>>>
>>>>>>> Thanks,
>>>>>>> Khushboo
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>> On Tue, Jan 14, 2020 at 10:27 AM Khushboo Vashi <
>>>>>>>> [email protected]> wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Please find the attached patch for RM #5053 - Getting an error
>>>>>>>>> while changing the columns in the existing view.
>>>>>>>>>
>>>>>>>>> PostgreSQL doesn't allow to change the view columns. So, while
>>>>>>>>> performing this task the existing view should be dropped first and then
>>>>>>>>> recreate it and also user will get a warning first.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Khushboo
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> *Thanks & Regards*
>>>>>>>> *Akshay Joshi*
>>>>>>>>
>>>>>>>> *Sr. Software Architect*
>>>>>>>> *EnterpriseDB Software India Private Limited*
>>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> *Thanks & Regards*
>>>>>> *Akshay Joshi*
>>>>>>
>>>>>> *Sr. Software Architect*
>>>>>> *EnterpriseDB Software India Private Limited*
>>>>>> *Mobile: +91 976-788-8246*
>>>>>>
>>>>>
>>>>
>>>> --
>>>> *Thanks & Regards*
>>>> *Akshay Joshi*
>>>>
>>>> *Sr. Software Architect*
>>>> *EnterpriseDB Software India Private Limited*
>>>> *Mobile: +91 976-788-8246*
>>>>
>>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>>
>> *Sr. Software Architect*
>> *EnterpriseDB Software India Private Limited*
>> *Mobile: +91 976-788-8246*
>>
>
--
*Thanks & Regards*
*Akshay Joshi*
*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2020-04-13 14:55 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 04:57 [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view Khushboo Vashi <[email protected]>
2020-01-14 06:17 ` Akshay Joshi <[email protected]>
2020-03-24 08:17 ` Khushboo Vashi <[email protected]>
2020-03-24 09:17 ` Akshay Joshi <[email protected]>
2020-04-07 04:30 ` Khushboo Vashi <[email protected]>
2020-04-07 06:12 ` Akshay Joshi <[email protected]>
2020-04-08 05:28 ` Khushboo Vashi <[email protected]>
2020-04-08 06:12 ` Akshay Joshi <[email protected]>
2020-04-13 08:59 ` Khushboo Vashi <[email protected]>
2020-04-13 14:55 ` Akshay Joshi <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox