Re: [pgAdmin4][Patch][RM2257]: Query tool - Insert row doesn't use default values

Hi Dave,

Please find updated patch for RM case and a separate patch for Feature
tests.

*Python:*

- Added [default] label for cells with default values while inserting a new
row.

- Introduced a FieldValidator function for cells that don't allow null
values. If user tries to insert null value, field with be highlighted with
red borders around.

​-
If a cell contains blank string('') and when we set it to null, the change
into the cell is not detected. It was because the comparison
for (defaultValue == null) return true if defaultValue is undefined. Hence
_.isNull(value) is used to fix this.

*Feature Test cases:*

- Introduced a new method create_table_with_query(server, db_name, query)
in test_utils.py which executes the given query on connected server.

- Added a new file test_data.json that has test data for test cases.

On Fri, Apr 7, 2017 at 2:21 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Sat, Apr 1, 2017 at 12:45 PM, Surinder Kumar
> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
> > Hi
> >
> > Issues fixed:
> >
> > 1. If a column is defined with a default modifier, there is now way to
> > insert the row with those defaults.
> > The column will be left blank and it will take default value
> automatically.
> >
> > 2. If a column has a not-null constraint then an error is returned and
> the
> > row is not inserted.
> > The column will be left blank
> >
> > The default values for new added rows will be displayed on
> refresh/execute.
> >
> > Please find attached patch and review.
>
> This largely works as expected, but there is some weirdness. I have a
> test table that looks like this:
>
> CREATE TABLE public.defaults
> (
> id bigint NOT NULL DEFAULT nextval('defaults_id_seq'::regclass),
> data_default_nulls text COLLATE pg_catalog."default" DEFAULT
> 'abc123'::text,
> data_default_no_nulls text COLLATE pg_catalog."default" NOT NULL
> DEFAULT 'def456'::text,
> data_nulls text COLLATE pg_catalog."default",
> data_no_nulls text COLLATE pg_catalog."default" NOT NULL,
> CONSTRAINT defaults_pkey PRIMARY KEY (id)
> )
>
> Remember that the expected behaviour is:
>
> - Set a value to empty to update the column to null.
> - Set a value to '' to update the column to an empty string
> - Set a value to anything else to update the column to that value
>
> 1) In a row with values in each column, if I try to set the value of
> data_default_nulls to null, the query executed is:
>
> UPDATE public.defaults SET
> data_default_nulls = '' WHERE
> id = '2';
>
> 2) If I do the same in the data_nulls column, the value is immediately
> shown as [null] and the query executed is:
>
> UPDATE public.defaults SET
> data_nulls = NULL WHERE
> id = '2';
>
> 3) If I then edit the value in data_default_nulls, it shows the
> current value as ''. Removing the quotes (to set it to null) doesn't
> get detected as a change.
>
​​Taken care.

>
> 4) When I manually executed "update defaults set data_default_nulls =
> null where id = 2" in a query tool window, I got:
>
> 2017-04-07 09:43:02,987: INFO werkzeug: 127.0.0.1 - - [07/Apr/2017
> 09:43:02] "GET /sqleditor/columns/8745675 HTTP/1.1" 500 -
> Traceback (most recent call last):
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/flask/app.py",
> line 2000, in __call__
> return self.wsgi_app(environ, start_response)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/flask/app.py",
> line 1991, 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 1567, in handle_exception
> reraise(exc_type, exc_value, tb)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/flask/app.py",
> line 1988, in wsgi_app
> response = self.full_dispatch_request()
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/flask/app.py",
> line 1641, in full_dispatch_request
> rv = self.handle_user_exception(e)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/flask/app.py",
> line 1544, in handle_user_exception
> reraise(exc_type, exc_value, tb)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/flask/app.py",
> line 1639, in full_dispatch_request
> rv = self.dispatch_request()
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/flask/app.py",
> line 1625, in dispatch_request
> return self.view_functions[rule.endpoint](**req.view_args)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/flask_login.py",
> line 792, in decorated_view
> return func(*args, **kwargs)
> File "/Users/dpage/git/pgadmin4/web/pgadmin/tools/sqleditor/__
> init__.py",
> line 452, in get_columns
> tid=command_obj.obj_id)
> AttributeError: 'QueryToolCommand' object has no attribute 'obj_id'
>
​Fixed.​

>
> 5) When I run the query again in pgAdmin III, then refresh the data in
> pgAdmin 4, the data_default_nulls column is displayed without the
> [null] marker (despite having a null value, which I confirmed in
> pgAdmin 3).
>
​Fixed.​

>
> I'm sure there are other combinations of issues here. Please fix and
> thoroughly re-test to ensure behaviour is consistent - and to avoid
> future issues, please add some appropriate feature tests to check
> nulls, defaults and empty strings are properly handled in view, insert
> and updates. Murtuza recently wrote some feature tests for the query
> tool - you should be able to use those as a starting point.
>
​Added feature tests​

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