Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d5WL4-00064Z-6L for pgadmin-hackers@arkaria.postgresql.org; Tue, 02 May 2017 11:51:30 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1d5WL3-0006mf-0j for pgadmin-hackers@arkaria.postgresql.org; Tue, 02 May 2017 11:51:29 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1d5WL1-0006lH-9k for pgadmin-hackers@postgresql.org; Tue, 02 May 2017 11:51:27 +0000 Received: from mail-it0-x233.google.com ([2607:f8b0:4001:c0b::233]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1d5WKs-0003oy-2M for pgadmin-hackers@postgresql.org; Tue, 02 May 2017 11:51:26 +0000 Received: by mail-it0-x233.google.com with SMTP id x188so8549917itb.0 for ; Tue, 02 May 2017 04:51:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin-org.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=T+Zhj34qlek9I8R/2y0iXHeeaxpeEWclKiDe33T61B8=; b=ZI6tvL4aVF4YrSM/w6CGn/rSXJMi+GiCTXrsU4dAFvX57KK59iuVZ5aLwqEVAmm6HZ M2RKaXdh1xkL5KC3JkoU9F7FaduyQPMGFYSyYINcKGng1cB7t5i9Iew5e16Ml38GQ3X3 jVjqo/nQFvWJQOPhR8oFQ4cbgmCcuEMU7QcRQX3NJaUOVsdMcRMP/BqAVpGh8cA6kkHe O168/C1ndozWcXJ3xUgjQenN1w0kZQHZ6vUc68OAhWHrUHUL0YKz/B5pAHTnqQ9peiFA N0ZPRkPRcN8SZEoSoOujfM9qRpIIiiajtUBo1nNDP1fuOwlSfE2yRTVBrn790xYO1tzC 9jaA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=T+Zhj34qlek9I8R/2y0iXHeeaxpeEWclKiDe33T61B8=; b=TgPHQF3KDcPjI2Br+mf3f5nxgwx2DEt5kYtHwdDd9TgVx0Oa3nOv/T0g0zvnpdqk6C KUGRBRJV7cCSId0qCUqFwQORI/Vx1supQv3MPvGb6o56HB5YBQfFhgUiFpUaPcSmwcex 1C6t0f2gWnKwQEklv8Q0+oyRgRT8UKnUMEU9xYODWOUla2frMbqSQLuOnA8aaf+xVS6K YDfW4zAmqn+n3TSzl8Oz258muez9ZoKg1CsaqgXtqhFgp/zlthJbDBE0JJcrJvgIelT6 1i8+XgmrhTIl8Lq6rWEzKfo5iOCoyinCY8Yh9y//D1v3LMFRW4p53QUrUXDcxeEDUVur +aLA== X-Gm-Message-State: AN3rC/6b0uSyg24GwdrDACL27ziRr9kmBn32aVqSldcq2oI+LSdW2nqt qvLWKrxsvqqqUyxl0Tww9EtznrQJlHsjJ74= X-Received: by 10.36.31.200 with SMTP id d191mr2582551itd.85.1493725874426; Tue, 02 May 2017 04:51:14 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.174.167 with HTTP; Tue, 2 May 2017 04:51:13 -0700 (PDT) In-Reply-To: References: From: Dave Page Date: Tue, 2 May 2017 12:51:13 +0100 Message-ID: Subject: Re: [pgAdmin4][Patch][RM2257]: Query tool - Insert row doesn't use default values To: Surinder Kumar Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary=001a11446e86232db3054e8928b8 X-Pg-Spam-Score: -2.3 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --001a11446e86232db3054e8928b8 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hi, This is looking much better now :-). Couple of thoughts and a bug: - Only the TextFormatter seems to handle both [null] and [default] values. Shouldn't all formatters do so (including Json and Checkmark)? For example, "serial" columns currently get displayed as [null] when left blank, but I would expect to see [default]. - I would suggest we put [null] and [default] in a lighter colour - #999999= . - With the feature test patch added, I seem to be consistently getting the following failure (immediately after your new tests run): =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D ERROR: runTest (pgadmin.feature_tests.xss_checks_panels_and_query_tool_test.CheckForXssFea= tureTest) Test XSS check for panels and query tool ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/base_feature_test.p= y", line 42, in setUp self._screenshot() File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/base_feature_test.p= y", line 92, in _screenshot python_version)) File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/we= bdriver/remote/webdriver.py", line 802, in get_screenshot_as_file png =3D self.get_screenshot_as_png() File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/we= bdriver/remote/webdriver.py", line 821, in get_screenshot_as_png return base64.b64decode(self.get_screenshot_as_base64().encode('ascii')= ) File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/we= bdriver/remote/webdriver.py", line 831, in get_screenshot_as_base64 return self.execute(Command.SCREENSHOT)['value'] File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/we= bdriver/remote/webdriver.py", line 238, in execute self.error_handler.check_response(response) File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/we= bdriver/remote/errorhandler.py", line 193, in check_response raise exception_class(message, screen, stacktrace) UnexpectedAlertPresentException: Alert Text: None Message: unexpected alert open: {Alert text : Are you sure you wish to close the pgAdmin 4 browser?} (Session info: chrome=3D58.0.3029.81) (Driver info: chromedriver=3D2.29.461585 (0be2cd95f834e9ee7c46bcc7cf405b483f5ae83b),platform=3DMac OS X 10.12.3 x86_= 64) =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D ERROR: runTest (pgadmin.feature_tests.xss_checks_pgadmin_debugger_test.CheckDebuggerForXss= FeatureTest) Test table DDL generation ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/base_feature_test.p= y", line 42, in setUp self._screenshot() File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/base_feature_test.p= y", line 92, in _screenshot python_version)) File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/we= bdriver/remote/webdriver.py", line 802, in get_screenshot_as_file png =3D self.get_screenshot_as_png() File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/we= bdriver/remote/webdriver.py", line 821, in get_screenshot_as_png return base64.b64decode(self.get_screenshot_as_base64().encode('ascii')= ) File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/we= bdriver/remote/webdriver.py", line 831, in get_screenshot_as_base64 return self.execute(Command.SCREENSHOT)['value'] File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/we= bdriver/remote/webdriver.py", line 238, in execute self.error_handler.check_response(response) File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/we= bdriver/remote/errorhandler.py", line 193, in check_response raise exception_class(message, screen, stacktrace) UnexpectedAlertPresentException: Alert Text: None Message: unexpected alert open: {Alert text : Are you sure you wish to close the pgAdmin 4 browser?} (Session info: chrome=3D58.0.3029.81) (Driver info: chromedriver=3D2.29.461585 (0be2cd95f834e9ee7c46bcc7cf405b483f5ae83b),platform=3DMac OS X 10.12.3 x86_= 64) Thanks! On Fri, Apr 28, 2017 at 10:19 AM, Surinder Kumar < surinder.kumar@enterprisedb.com> wrote: > 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 wit= h > red borders around. > > =E2=80=8B- > If a cell contains blank string('') and when we set it to null, the chang= e > into the cell is not detected. It was because the comparison > for (defaultValue =3D=3D 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 wrote: > >> Hi >> >> On Sat, Apr 1, 2017 at 12:45 PM, Surinder Kumar >> 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 =3D '' WHERE >> id =3D '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 =3D NULL WHERE >> id =3D '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. >> > =E2=80=8B=E2=80=8BTaken care. > >> >> 4) When I manually executed "update defaults set data_default_nulls =3D >> null where id =3D 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-packa >> ges/flask/app.py", >> line 2000, in __call__ >> return self.wsgi_app(environ, start_response) >> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >> ges/flask/app.py", >> line 1991, in wsgi_app >> response =3D self.make_response(self.handle_exception(e)) >> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >> ges/flask/app.py", >> line 1567, in handle_exception >> reraise(exc_type, exc_value, tb) >> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >> ges/flask/app.py", >> line 1988, in wsgi_app >> response =3D self.full_dispatch_request() >> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >> ges/flask/app.py", >> line 1641, in full_dispatch_request >> rv =3D self.handle_user_exception(e) >> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >> ges/flask/app.py", >> line 1544, in handle_user_exception >> reraise(exc_type, exc_value, tb) >> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >> ges/flask/app.py", >> line 1639, in full_dispatch_request >> rv =3D self.dispatch_request() >> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >> ges/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-packa >> ges/flask_login.py", >> line 792, in decorated_view >> return func(*args, **kwargs) >> File "/Users/dpage/git/pgadmin4/web/pgadmin/tools/sqleditor/__ini >> t__.py", >> line 452, in get_columns >> tid=3Dcommand_obj.obj_id) >> AttributeError: 'QueryToolCommand' object has no attribute 'obj_id' >> > =E2=80=8BFixed.=E2=80=8B > >> >> 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). >> > =E2=80=8BFixed.=E2=80=8B > >> >> 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. >> > =E2=80=8BAdded feature tests=E2=80=8B > >> >> Thanks. >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> > > --=20 Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --001a11446e86232db3054e8928b8 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi,

This is looking much better now :-)= . Couple of thoughts and a bug:

- Only the TextFor= matter seems to handle both [null] and [default] values. Shouldn't all = formatters do so (including Json and Checkmark)? For example, "serial&= quot; columns currently get displayed as [null] when left blank, but I woul= d expect to see [default].=C2=A0

- I would suggest= we put [null] and [default] in a lighter colour - #999999.

<= /div>
- With the feature test patch added, I seem to be consistently ge= tting the following failure (immediately after your new tests run):

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D
ERROR: runTest (pgadmin.feature_tests.xss_checks_panels= _and_query_tool_test.CheckForXssFeatureTest)
Test XSS check for p= anels and query tool
--------------------------------------------= --------------------------
Traceback (most recent call last):
=C2=A0 File "/Users/dpage/git/pgadmin4/web/regression/feature_u= tils/base_feature_test.py", line 42, in setUp
=C2=A0 =C2=A0 = self._screenshot()
=C2=A0 File "/Users/dpage/git/pgadmin4/we= b/regression/feature_utils/base_feature_test.py", line 92, in _screens= hot
=C2=A0 =C2=A0 python_version))
=C2=A0 File "/U= sers/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdr= iver/remote/webdriver.py", line 802, in get_screenshot_as_file
=C2=A0 =C2=A0 png =3D self.get_screenshot_as_png()
=C2=A0 File= "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selen= ium/webdriver/remote/webdriver.py", line 821, in get_screenshot_as_png=
=C2=A0 =C2=A0 return base64.b64decode(self.get_screenshot_as_bas= e64().encode('ascii'))
=C2=A0 File "/Users/dpage/.vi= rtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/remote/we= bdriver.py", line 831, in get_screenshot_as_base64
=C2=A0 = =C2=A0 return self.execute(Command.SCREENSHOT)['value']
= =C2=A0 File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-pac= kages/selenium/webdriver/remote/webdriver.py", line 238, in execute
=C2=A0 =C2=A0 self.error_handler.check_response(response)
=C2=A0 File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-pa= ckages/selenium/webdriver/remote/errorhandler.py", line 193, in check_= response
=C2=A0 =C2=A0 raise exception_class(message, screen, sta= cktrace)
UnexpectedAlertPresentException: Alert Text: None
<= div>Message: unexpected alert open: {Alert text : Are you sure you wish to = close the pgAdmin 4 browser?}
=C2=A0 (Session info: chrome=3D58.0= .3029.81)
=C2=A0 (Driver info: chromedriver=3D2.29.461585 (0be2cd= 95f834e9ee7c46bcc7cf405b483f5ae83b),platform=3DMac OS X 10.12.3 x86_64)


=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D
ERROR: runTest (pgadmin.feature_tests.xs= s_checks_pgadmin_debugger_test.CheckDebuggerForXssFeatureTest)
Te= st table DDL generation
-----------------------------------------= -----------------------------
Traceback (most recent call last):<= /div>
=C2=A0 File "/Users/dpage/git/pgadmin4/web/regression/featur= e_utils/base_feature_test.py", line 42, in setUp
=C2=A0 =C2= =A0 self._screenshot()
=C2=A0 File "/Users/dpage/git/pgadmin= 4/web/regression/feature_utils/base_feature_test.py", line 92, in _scr= eenshot
=C2=A0 =C2=A0 python_version))
=C2=A0 File &quo= t;/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/w= ebdriver/remote/webdriver.py", line 802, in get_screenshot_as_file
=C2=A0 =C2=A0 png =3D self.get_screenshot_as_png()
=C2=A0 = File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/s= elenium/webdriver/remote/webdriver.py", line 821, in get_screenshot_as= _png
=C2=A0 =C2=A0 return base64.b64decode(self.get_screenshot_as= _base64().encode('ascii'))
=C2=A0 File "/Users/dpage= /.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/remot= e/webdriver.py", line 831, in get_screenshot_as_base64
=C2= =A0 =C2=A0 return self.execute(Command.SCREENSHOT)['value']
=C2=A0 File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-= packages/selenium/webdriver/remote/webdriver.py", line 238, in execute=
=C2=A0 =C2=A0 self.error_handler.check_response(response)
<= div>=C2=A0 File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site= -packages/selenium/webdriver/remote/errorhandler.py", line 193, in che= ck_response
=C2=A0 =C2=A0 raise exception_class(message, screen, = stacktrace)
UnexpectedAlertPresentException: Alert Text: None
Message: unexpected alert open: {Alert text : Are you sure you wish = to close the pgAdmin 4 browser?}
=C2=A0 (Session info: chrome=3D5= 8.0.3029.81)
=C2=A0 (Driver info: chromedriver=3D2.29.461585 (0be= 2cd95f834e9ee7c46bcc7cf405b483f5ae83b),platform=3DMac OS X 10.12.3 x86_64)<= /div>

Thanks!


On Fri, Apr 28, 2017 at 10:19 AM,= Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:
Hi Dave,

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

Python:

- Add= ed=C2=A0[default] label for cells with defau= lt values while inserting a new row.

- Introduced a FieldValidator= function for cells that don't allow null values. If user tries to inse= rt null value, field with be highlighted with red =C2=A0 =C2=A0borders arou= nd.

=E2=80=8B-=C2=A0
If a cell contains blank string(= '') and when we set it to null, the change into the cell is not det= ected. It was because the comparison
for (defaultValue =3D=3D null) retu= rn true if defaultValue is undefined. Hence _.isNull(value) is used to fix = this.

Feature Test cases:

<= /div>
=C2=A0- Introduced a new method=C2=A0create_table_with_<= wbr>query(server, db_name, query)=C2=A0 in test_utils.py which execu= tes the given query on connected server.

=C2=A0- A= dded a new file test_data.json that has test data for test cases.


<= div class=3D"gmail-h5">On Fri, Apr 7, 2017 at 2:21 PM, Dave Page <dpage@p= gadmin.org> wrote:
Hi

On Sat, Apr 1, 2017 at 12:45 PM, Surinder Kumar
<su= rinder.kumar@enterprisedb.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 automatic= ally.
>
> 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/exe= cute.
>
> 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
(
=C2=A0 =C2=A0 id bigint NOT NULL DEFAULT nextval('defaults_id_seq':= :regclass),
=C2=A0 =C2=A0 data_default_nulls text COLLATE pg_catalog."default"= ; DEFAULT 'abc123'::text,
=C2=A0 =C2=A0 data_default_no_nulls text COLLATE pg_catalog."default&q= uot; NOT NULL
DEFAULT 'def456'::text,
=C2=A0 =C2=A0 data_nulls text COLLATE pg_catalog."default",
=C2=A0 =C2=A0 data_no_nulls text COLLATE pg_catalog."default" NOT= NULL,
=C2=A0 =C2=A0 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 =3D '' WHERE
id =3D '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 =3D NULL WHERE
id =3D '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&= #39;t
get detected as a change.
=E2=80=8B=E2=80=8BTaken care.=C2=A0

4) When I manually executed "update defaults set data_default_nulls = =3D
null where id =3D 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):
=C2=A0 File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/sit= e-packages/flask/app.py",
line 2000, in __call__
=C2=A0 =C2=A0 return self.wsgi_app(environ, start_response)
=C2=A0 File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/sit= e-packages/flask/app.py",
line 1991, in wsgi_app
=C2=A0 =C2=A0 response =3D self.make_response(self.handle_exception(e)= )
=C2=A0 File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/sit= e-packages/flask/app.py",
line 1567, in handle_exception
=C2=A0 =C2=A0 reraise(exc_type, exc_value, tb)
=C2=A0 File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/sit= e-packages/flask/app.py",
line 1988, in wsgi_app
=C2=A0 =C2=A0 response =3D self.full_dispatch_request()
=C2=A0 File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/sit= e-packages/flask/app.py",
line 1641, in full_dispatch_request
=C2=A0 =C2=A0 rv =3D self.handle_user_exception(e)
=C2=A0 File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/sit= e-packages/flask/app.py",
line 1544, in handle_user_exception
=C2=A0 =C2=A0 reraise(exc_type, exc_value, tb)
=C2=A0 File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/sit= e-packages/flask/app.py",
line 1639, in full_dispatch_request
=C2=A0 =C2=A0 rv =3D self.dispatch_request()
=C2=A0 File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/sit= e-packages/flask/app.py",
line 1625, in dispatch_request
=C2=A0 =C2=A0 return self.view_functions[rule.endpoint](**req.view_arg= s)
=C2=A0 File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/sit= e-packages/flask_login.py",
line 792, in decorated_view
=C2=A0 =C2=A0 return func(*args, **kwargs)
=C2=A0 File "/Users/dpage/git/pgadmin4/web/pgadmin/tools/sqledito= r/__init__.py",
line 452, in get_columns
=C2=A0 =C2=A0 tid=3Dcommand_obj.obj_id)
AttributeError: 'QueryToolCommand' object has no attribute 'obj= _id'
=E2=80= =8BFixed.=E2=80=8B

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).
=E2=80=8B= Fixed.=E2=80=8B

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.
=E2=80=8BAdded feature tests=E2=80= =8B

Thanks.

--
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 Compan= y
--001a11446e86232db3054e8928b8--