Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d8mLn-0006YM-QG for pgadmin-hackers@arkaria.postgresql.org; Thu, 11 May 2017 11:33:44 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1d8mLm-0007ld-AI for pgadmin-hackers@arkaria.postgresql.org; Thu, 11 May 2017 11:33:42 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1d8mLX-0007N9-3Z for pgadmin-hackers@postgresql.org; Thu, 11 May 2017 11:33:27 +0000 Received: from mail-ua0-x229.google.com ([2607:f8b0:400c:c08::229]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1d8mLO-0006Rf-Vw for pgadmin-hackers@postgresql.org; Thu, 11 May 2017 11:33:25 +0000 Received: by mail-ua0-x229.google.com with SMTP id g49so26292518uaa.1 for ; Thu, 11 May 2017 04:33:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pivotal-io.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=pLfgqO6POVcrJjydxekp6qKHuGYLBZpxyQDtnO8qHCk=; b=fMeCTHTL+Oye8MMtCrFIVc0sXo4eaq3ozi83Rlt0mWPEstLNi9hkH4N9u46gJFmdYm 2h3Iw0fHPaz8wBlfurv64ybX9G6tt6XycGrePI5/UZPkmEIobdTB9XrS9A1tRJn/ZGRG Wyh11wIFqvCn11oWH0kOiVYAzhM/OZmfF/w5SOBMBDz51FgelziNPzvzTHbFQaJOBGMO bcVrlP31EmiHn/yU68nsv8UwaAnXrK1xasxPqiuKtu0D9julxGPxBacOY1vZkRRpasmF d8KMqiuWMo8Fio6vxDjj+OJxh0rZP6LxOpohTdFIvj/Wkb7oP+ewe03hizAVp/PHT/YT J7Iw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=pLfgqO6POVcrJjydxekp6qKHuGYLBZpxyQDtnO8qHCk=; b=a7CM4IncPvib0CehyaCL+FPls1dwYIPcXwAInHC5obLzGXvQJWga67Q0MCZfy0kwY/ fmxTqCJ0vei86RIGZmSUoRzaWF0qIiSPmax+Y3EbPiaaFdbT6T+Uwlf27Lpm4Z+KoMMq fGudAbq40Ug/WG+fvAZyHHreaIRkWbL/zmK5kZTe9+iyYVGiHj9h5p2kiiV4Yg+FqH82 5lAUfxFDk8cKLSVsrODvUeYXRYqoh/4eNk3oKFvKT6VAGoNAjF+JtZACKKd2G3XWwVJ0 +/NZEUi2rhFIfH9rVW3fvnxAtGGKbAuHOblRz1aejyNe5n4xIgPeXFPVlVH4IDXHRcCj vDqg== X-Gm-Message-State: AODbwcBrfERYxn0dlBKf4UtA2Xh6k4LNKvdTpPr9E6p7MteJjrVGZYLV uTEoEcHlq47TWWRRchn87xQYDtnWoemh X-Received: by 10.159.34.145 with SMTP id 17mr5538662uan.42.1494502396645; Thu, 11 May 2017 04:33:16 -0700 (PDT) MIME-Version: 1.0 References: <19B13ECE-CA6A-48E7-BF70-264F490F40B4@pgadmin.org> In-Reply-To: From: Joao Pedro De Almeida Pereira Date: Thu, 11 May 2017 11:33:06 +0000 Message-ID: Subject: Re: [patch] Dependents and Dependencies in GreenPlum To: Khushboo Vashi , Dave Page Cc: Akshay Joshi , Sarah McAlear , Robert Eckhardt , Matthew Kleiman , Ashesh Vashi , pgadmin-hackers Content-Type: multipart/alternative; boundary=94eb2c04c98877eda1054f3df45b X-Pg-Spam-Score: -1.9 (-) 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 --94eb2c04c98877eda1054f3df45b Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hello all, This test is trying to check if the sql to retrieve the role dependencies of a object in the database is correct. Using a different user to connect to the database and create tables there is no need for extra setup because the role depedencies table entry is added automatically. Also we talked with some internal users and they told us they always used a different user to connect to the database with pgadmin. After your comments we realize that the test need to have a different setup in order to become more readable and resilient. What do we need to do in the test setup to ensure an entry is added to the role dependency table? Also the test need to be a little bit more resilient to work independently of the user that is used on the test. Thanks Joao On Thu, May 11, 2017, 3:20 AM Khushboo Vashi < khushboo.vashi@enterprisedb.com> wrote: > On Thu, May 11, 2017 at 12:03 PM, Dave Page wrote: > >> >> >> On 11 May 2017, at 07:11, Akshay Joshi >> wrote: >> >> Hi Sarah >> >> On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear >> wrote: >> >>> Hi Ashkay! >>> >>> TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with >>>> below error: >>>> .....\test_role_dependencies_sql.py\", line 41, in assertions\n se= lf.assertEqual(1, >>>> len(fetch_result))\nAssertionError: 1 !=3D 0 >>> >>> >>> We experienced a similar problem when we started developing this patch. >>> We noticed that we were connecting to the database with the super user = (the >>> db owner user), which did not create the role we are expecting when a t= able >>> is created. To ensure that this role is created, we had to create a new >>> user and use it to execute all the tests. After we made this change, th= e >>> tests passed. >>> >>> Can you try to create a new user and use it to execute your tests? >>> >> >> I did that and it works. I have created new user 'test' with >> superuser privileges and update parameter db_username=3D"test" in >> test_config.json file. But still I am unable to understand why it doesn'= t >> work with 'postgres' (default) user? >> >> >> Agreed. That suggests something fishy is going on that should be >> understood. >> >> > We don't check role dependencies for every object. As per the code, the > role dependencies are executed for columns not for the table itself. > And in the test cases, the table object is trying to execute the role > dependencies, so I think this test case is not correct. > >> >>> Thanks, >>> Jo=C3=A3o & Sarah >>> >>> On Wed, May 10, 2017 at 1:56 PM, Akshay Joshi < >>> akshay.joshi@enterprisedb.com> wrote: >>> >>>> >>>> >>>> On May 10, 2017 19:07, "Sarah McAlear" wrote: >>>> >>>> Hi Akshay! >>>> >>>> Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our >>>> tests pass. >>>> >>>> >>>> On both 9.6 and 10.0. Query returned 0 rows which is compared >>>> against 1 in assert statement. >>>> >>>> >>>> Thanks! >>>> Jo=C3=A3o & Sarah >>>> >>>> On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi < >>>> akshay.joshi@enterprisedb.com> wrote: >>>> >>>>> Hi Sarah >>>>> >>>>> On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear >>>>> wrote: >>>>> >>>>>> Hi Akshay! >>>>>> >>>>>> >>>>>>> Some test file names ended with "*_sql_template.py*" do we need to >>>>>>> add that string ? >>>>>> >>>>>> we added this suffix after moving the tests up a level to >>>>>> tables/tests to clarify what subject they applied to. we changed the= suffix >>>>>> to "_sql.py" >>>>>> >>>>>> - Files "test_column_acl_sql_template.py" and " >>>>>>> test_column_properties_sql_template.py" should be moved from >>>>>>> tables->tests to tables->column->tests. As it's related to column. >>>>>>> - Files "test_trigger_get_oid_sql_template.py" and " >>>>>>> test_trigger_nodes_sql_template.py" should be moved from >>>>>>> tables->tests to tables->triggers->tests. As its related to trigger= s. >>>>>> >>>>>> these tests are related to the sql files in tables/templates/column, >>>>>> not tables/column, so moving them into tables/column would be more >>>>>> confusing. >>>>>> >>>>>> Following test cases are failing >>>>>> >>>>>> Thank you, fixed, see new patch. Can you confirm that >>>>>> TestTablesNodeSql doesn't fail in your environment? >>>>>> >>>>> >>>>> TestTablesNodeSql hasn't failed but TestRoleDependenciesSql faile= d >>>>> with below error: >>>>> .....\test_role_dependencies_sql.py\", line 41, in assertions\n >>>>> self.assertEqual(1, len(fetch_result))\nAssertionError: 1 !=3D 0 >>>>> >>>>> >>>>>> Thanks, >>>>>> George & Sarah >>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> *Akshay Joshi* >>>>> *Principal Software Engineer * >>>>> >>>>> >>>>> >>>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 >>>>> 976-788-8246 <+91%2097678%2088246>* >>>>> >>>> >>>> >>>> >>> >> >> >> -- >> *Akshay Joshi* >> *Principal Software Engineer * >> >> >> >> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246* >> >> --94eb2c04c98877eda1054f3df45b Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

Hello all,
This test is trying to check if the sql to retrieve the role dependencies o= f a object in the database is correct. Using a different user to connect to= the database and create tables there is no need for extra setup because th= e role depedencies table entry is added automatically. Also we talked with = some internal users and they told us they always used a different user to c= onnect to the database with pgadmin.

After your comments we realize that the test need to have a = different setup in order to become more readable and resilient. What do we = need to do in the test setup to ensure an entry is added to the role depend= ency table?

Also the test need to be a little bit more resilient to work= independently of the user that is used on the test.

Thanks
Joao


On Thu, May 11, 2017, 3:20 = AM Khushboo Vashi <kh= ushboo.vashi@enterprisedb.com> wrote:
On Thu, May 11, 2017 at 12:03 PM, Dave Page <dpage@pgadmin.org&g= t; wrote:


On 11 M= ay 2017, at 07:11, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Sarah<= div class=3D"gmail_extra">
=
On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear <= span dir=3D"ltr"><smcalear@pivotal.io> wrote:
Hi Ashkay!

TestTablesNodeSql hasn't fail= ed but TestRoleDependenciesSql failed with below error:
=C2=A0 =C2=A0 ..= ...\test_role_dependencies_sql.py\", line 41, in assertions\n=C2=A0self.= assertEqual(1, len(fetch_result))\nAssertio= nError: 1 !=3D 0

We exp= erienced a similar problem when we started developing this patch. We notice= d that we were connecting to the database with the super user (the db owner= user), which did not create the role we are expecting when a table is crea= ted. To ensure that this role is created, we had to create a new user and u= se it to execute all the tests. After we made this change, the tests passed= .=C2=A0

Can you try to create a new user and use i= t to execute your tests?

=C2=A0= =C2=A0 I did that and it works. I have created new user 'test' wit= h superuser privileges and update parameter db_username=3D"test" = in test_config.json file. But still I am unable to understand why it doesn&= #39;t work with 'postgres' (default) user? =C2=A0

Agreed. That suggests s= omething fishy is going on that should be understood.


We don't check role dependenci= es for every object. As per the code, the role dependencies are executed fo= r columns not for the table itself.
And in the test cases, the ta= ble object is trying to execute the role dependencies, so I think this test= case is not correct.

Thanks,
Jo=C3=A3o & Sarah

On Wed, May 10, 2017 at 1:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:


On May 10, 2017 19:07, &= quot;Sarah McAlear" <smcalear@pivotal.io> wrote:
Hi Akshay!

Does this error occur on 9= .6 or 10.0? We tested it in 9.6 and all our tests pass.
<= /blockquote>

=C2=A0 =C2=A0 On both 9.6 and 10.0. Query returned 0 rows which is compare= d against 1 in assert statement.=C2=A0

Thanks!
Jo=C3=A3o & Sarah

On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <akshay.jos= hi@enterprisedb.com> wrote:
Hi Sarah
On Tue, May 9, 2017 at 9:03 PM, Sarah McA= lear <smcalear@pivotal.io> wrote:
Hi Akshay!
=C2=A0
Some te= st file names ended with "_sql_template.py" do we need to add that st= ring ?=C2=A0
we added this suffix after movi= ng the tests up a level to tables/tests to clarify what subject they applie= d to. we changed the suffix to "_sql.py"

- Files "test_column_acl_sql_template.py" and "test_column_= properties_sql_template.py" should be moved from tables->tests to tables-= >column->tests. As it's related to column.
- Files "= test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables-&= gt;tests to tables->triggers->tests. As its related to triggers.
these tests are related to the sql files in tables/temp= lates/column, not tables/column, so moving them into tables/column would be= more confusing.=C2=A0

Following test cases are failin= g
Thank you, fixed, see new patch. Can you c= onfirm that TestTablesNodeSql doesn't fail in your environment?
=C2=A0 =C2=A0
=C2=A0 =C2=A0 TestTabl= esNodeSql hasn't failed but TestRoleDependenciesSql failed with below e= rror:
=C2=A0 =C2=A0 .....\test_role_dependencies_sql.py<= /span>\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 !=3D 0

Thanks,
George & Sarah




--
Akshay Joshi
Principal Software Engineer=C2=A0
<= font color=3D"#3333FF">
=






--
=
Akshay Joshi
<= span style=3D"color:rgb(0,0,0);font-family:arial,sans-serif;font-size:13px;= border-collapse:collapse">Principal Software Engineer=C2=A0


Phone: +91 20-3058-9= 517
Mobile: +91 976-788-8246
--94eb2c04c98877eda1054f3df45b--