Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d9pGk-0007GA-D7 for pgadmin-hackers@arkaria.postgresql.org; Sun, 14 May 2017 08:52:50 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1d9pGj-0000jP-Dh for pgadmin-hackers@arkaria.postgresql.org; Sun, 14 May 2017 08:52:49 +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 1d9pGh-0000jA-4N for pgadmin-hackers@postgresql.org; Sun, 14 May 2017 08:52:47 +0000 Received: from mail-pg0-x22d.google.com ([2607:f8b0:400e:c05::22d]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1d9pGc-000766-U7 for pgadmin-hackers@postgresql.org; Sun, 14 May 2017 08:52:45 +0000 Received: by mail-pg0-x22d.google.com with SMTP id u28so47223216pgn.1 for ; Sun, 14 May 2017 01:52:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=R64NkqMIKo4aj+3DIlK5RQByeEggxg8f4Vn1l/sLXnY=; b=rgM2y2rUlB0ym7df09nM+EhUHUkXxxPf7x/mzjiO8tcBcPzndq2PHRpW/wsuIMpu85 MqphnljTu/MSYGjCzdXJDE2u8PYKYTHxc/0Z0UWHQWNzNIwFx2cW87jNlhPBnJjJxRIi jwU9DlLlOo0dGFi4mUMxQ2Sopqrich9semSZMc+H30oWSxuqD73giDKWX8T9Jit3i68+ NvLDP/7nZYazTCJKO+HYfBKxTsK8BjSivzqymvtlm4Y8y0qWwR1s9WZqNDR5njaCiYmS ySnOlFZdSTqzwsL41fq7m6fomwhLy2J+1+/EiXXbjecy3KSj7ZKdOftmqPJ3fEzzv2xA hzag== 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=R64NkqMIKo4aj+3DIlK5RQByeEggxg8f4Vn1l/sLXnY=; b=FJaceWovwC4/TeS5eys/SAtW6APo5ixlC7EBJ62ze+gEojh042tkaxcWTMhKsxNjpd cm3b6/gKzZ4gTnKUFgd3DXZc0k15Zu341YM8BhBS+KYCI0f49cb88/3byd1jJtcdpD+r tv7GFrNSHm+zsBkdNZwKHgjXOic/m6OV2FpN1ZvuMxrOBfNrlbbGzH04+lB5eUsEBfju 5IzJGvLffDvrRb5ndSmfTP1uH3PxA4p1cJhphTHLSYXyXzMtD+KEWWijzdzNdRarSeo2 AdiKnbdCVdP/eX2TSmh6zMw6VK6J3eeDE98IUdhUlWKACZJ5p5gyHFVIwM+7CvHickFb GTXQ== X-Gm-Message-State: AODbwcC5XNZssjUmxIYspx86+sger4gVv3yCIv4JEI6ZUg4iJ9ftiJ3r nq1zOQG6O45dQZKmovJaxoFCiECTcgCg X-Received: by 10.84.229.72 with SMTP id d8mr690771pln.159.1494751961747; Sun, 14 May 2017 01:52:41 -0700 (PDT) MIME-Version: 1.0 Received: by 10.100.170.75 with HTTP; Sun, 14 May 2017 01:52:41 -0700 (PDT) In-Reply-To: References: <19B13ECE-CA6A-48E7-BF70-264F490F40B4@pgadmin.org> From: Akshay Joshi Date: Sun, 14 May 2017 14:22:41 +0530 Message-ID: Subject: Re: [patch] Dependents and Dependencies in GreenPlum To: Joao Pedro De Almeida Pereira Cc: pgadmin-hackers , George Gelashvili Content-Type: multipart/alternative; boundary="94eb2c180d0ab52b6a054f780fbf" 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 --94eb2c180d0ab52b6a054f780fbf Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Joao On Fri, May 12, 2017 at 11:23 PM, Joao Pedro De Almeida Pereira < jdealmeidapereira@pivotal.io> wrote: > Hello Akshay, > > You can find attached a new version of the patch that changes the test in > question. > This new version instead of requiring a creation of a new Role now does > the following: > 1 - Creates a new Role > 2 - Creates a table with that Role > 3 - Checks for dependencies > Test case still failing on my machine because when we create new Role it is without password and when we try to create table with that role first we will try to connect the database using that new role with servers['db_password'] which is wrong. I have modified setup function of " *test_role_dependencies_sql.py*" and it works, we can create new role using database server's password Old setup function: *def *setUp(self): create_role(self.server, *"testpgadmin"*) self.server_with_modified_user =3D self.server.copy() self.server_with_modified_user[*'username'*] =3D *"testpgadmin"* Modified setup function: def setUp(self): with test_utils.Database(self.server) as (connection, database_name): cursor =3D connection.cursor() try: cursor.execute( "CREATE ROLE testpgadmin LOGIN PASSWORD '%s'" % self.server['db_password']) except Exception as exception: print(exception) connection.commit() self.server_with_modified_user =3D self.server.copy() self.server_with_modified_user['username'] =3D "testpgadmin" If the above logic looks good to you then I'll commit the code. > > Thanks > Joao & George > > On Thu, May 11, 2017 at 7:33 AM, Joao Pedro De Almeida Pereira < > jdealmeidapereira@pivotal.io> wrote: > >> Hello all, >> This test is trying to check if the sql to retrieve the role dependencie= s >> of a object in the database is correct. Using a different user to connec= t >> to the database and create tables there is no need for extra setup becau= se >> the role depedencies table entry is added automatically. Also we talked >> with some internal users and they told us they always used a different u= ser >> 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 tab= le? >> >> 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 >>>>>> self.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 su= per >>>>> user (the db owner user), which did not create the role we are expect= ing >>>>> when a table 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, the 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 does= n'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 ou= r >>>>>> 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 t= he 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 trigg= ers. >>>>>>>> >>>>>>>> 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 >>>>>>> failed 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-9517 <+91%2020%203058%209517>Mobile: +91 >>>> 976-788-8246 <+91%2097678%2088246>* >>>> >>>> > --=20 *Akshay Joshi* *Principal Software Engineer * *Phone: +91 20-3058-9517Mobile: +91 976-788-8246* --94eb2c180d0ab52b6a054f780fbf Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Joao

On Fri, May 12, 2017 at 11:23 PM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello Akshay,

You can find attache= d a new version of the patch that changes the test in question.=C2=A0
=
This new version instead of requiring a creation of a new Role now doe= s the following:
1 - Creates a new Role
2 - Creates a t= able with that Role
3 - Checks for dependencies

=C2=A0 =C2=A0Test case still failing on my machi= ne because when we create new Role it is without password and when we try t= o create table with that role first we will try to connect the database usi= ng that new role with servers['db_password'] which is wrong. I have= modified setup function of " test_role_dependencies_sql.py"=C2=A0and it works, we can cre= ate new role using database server's password=C2=A0

Old setup function:
=C2=A0 =C2=A0 =C2=A0=C2=A0def setUp(self):=C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0
<= p style=3D"margin:0px;font-size:12px;line-height:normal;font-family:menlo"> =C2=A0 =C2=A0 create_role(self= .server, "testpgadmin")
=C2=A0 =C2=A0 self.server_with= _modified_user =3D self.server= .copy()
=C2=A0 =C2=A0 self.server_with= _modified_user['username'] =3D "testpgadmin&qu= ot;


Modified setup function:=


=C2=A0 =C2=A0def setUp(self<= span style=3D"color:rgb(0,0,0);font-family:menlo;font-size:9pt">):

    with test_utils.Datab=
ase(self.server) as (connection, database_na=
me):
        cursor =3D connection.cursor()
        try:
            cursor.execut=
e(
                "CREATE ROLE testpgadmin LOGIN PASSWORD '%s'"
=
                % self.server['db_password'])
 =
       except Exception as exception:
            print(exception)
        connection.commi=
t()

    self.server_wit=
h_modified_user =3D self.serve=
r.copy()
    self.server_wi=
th_modified_user['=
;username'] =3D "testpgadmin"

=C2=A0 =C2=A0= If the above logic looks good to you then I'll commit the code.

Thanks
Joao &= amp; George
=

On Thu, May 11, 2= 017 at 7:33 AM, Joao Pedro De Almeida Pereira <jdealmeidapereir= a@pivotal.io> wrote:

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 <khushboo.vashi@enterprisedb.com> wrote:
On Thu, May 11, 2017 at 12:03 PM, Dave Page <dpage@pgadmi= n.org> wrote:


On 11 May 2017, at 07:11, Akshay Joshi <akshay.joshi@enterpri= sedb.com> wrote:

=
Hi Sarah

On Thu, May 11, 2017 at 2:30 AM, Sarah McAl= ear <smcalear@pivotal.io> wrote:
Hi Ashkay!
TestTablesNodeSql hasn't failed but TestR= oleDependenciesSql failed with below error:
=C2=A0 =C2=A0 .....\test_role_dependencies_sql.py\",= line 41, in assertions\n=C2=A0self.assertE= qual(1, len(fetch_result))\nAssertionE= rror: 1 !=3D 0

We = experienced a similar problem when we started developing this patch. We not= iced that we were connecting to the database with the super user (the db ow= ner user), which did not create the role we are expecting when a table is c= reated. To ensure that this role is created, we had to create a new user an= d use it to execute all the tests. After we made this change, the tests pas= sed.=C2=A0

Can you try to create a new user and us= e it to execute your tests?

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

Agreed. That suggest= s 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 ta= ble 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.
<= div>
Thanks,
Jo=C3=A3o & Sarah

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


On May 10, 2017 19:07, "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.

=C2=A0 =C2=A0 On both 9.6 and 1= 0.0. Query returned 0 rows which is compared against 1 in assert statement.= =C2=A0

Thanks!
J= o=C3=A3o & Sarah

On Wed, May 10, 2017 at 2:29 AM, Ak= shay Joshi <akshay.joshi@enterprisedb.com> = wrote:
Hi Sarah

On Tue, May 9, 2017 at 9:03 PM, Sa= rah McAlear <smcalear@pivotal.io> wrote:
Hi Akshay!
=C2=A0
Some test file na= mes ended with "_sql_te= mplate.py" do we need t= o add that string ?=C2=A0
we added this suff= ix after moving the tests up a level to tables/tests to clarify what subjec= t they applied to. we changed the suffix to "_sql.py"
<= br>
- 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.
- Fil= es "test_trigger_get_oid_sql_tem= plate.py" and "= ;test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->= tests. As its related to triggers.
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.=C2=A0

Following test cases are failing
Thank you, fixed, see new patch. Can you confirm that TestTabl= esNodeSql doesn't fail in your environment?
=C2=A0 =C2=A0
=C2=A0 =C2=A0 TestTablesNodeSql hasn't= failed but TestRoleDependenciesSql failed with below error:
=C2= =A0 =C2=A0 .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\n<= span style=3D"font-family:menlo;font-size:9pt;color:rgb(0,128,0);font-weigh= t:bold">AssertionError: 1 !=3D 0

=

Thanks,
George & Sarah




--
Akshay Josh= i
Principal Software Engineer=C2=A0






--
=
Akshay Joshi<= /b>
= Principal Software Engineer=C2=A0


<= b>Phone: +91 20-3058-9517
Mobile: <= a href=3D"tel:+91%2097678%2088246" value=3D"+919767888246" target=3D"_blank= ">+91 976-788-8246




--
=
Akshay Joshi
Principal Software Engi= neer=C2=A0

=

Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
--94eb2c180d0ab52b6a054f780fbf--