From 0a9f469a4f9710ed2ced1bde2a76653f507c097e Mon Sep 17 00:00:00 2001 From: Jingyi Mei Date: Mon, 24 Sep 2018 11:48:51 -0700 Subject: [PATCH 1/2] Build: Remove primary key constraint in IC/DC Some of the install-check/dev-check tests were setting primary key constraints. If the user sets the GUC gp_default_storage_options='appendonly=true', IC/DC will fail while table creation because appendonly tables don't support primary key. This commit removes these constrains since they are unnecessary for those test cases. Co-authored-by: Nikhil Kak --- src/ports/postgres/modules/kmeans/test/kmeans.sql_in | 3 +-- src/ports/postgres/modules/regress/test/linear.ic.sql_in | 1 - src/ports/postgres/modules/regress/test/linear.sql_in | 3 --- src/ports/postgres/modules/regress/test/logistic.ic.sql_in | 1 - src/ports/postgres/modules/regress/test/logistic.sql_in | 3 --- src/ports/postgres/modules/regress/test/marginal.ic.sql_in | 1 - src/ports/postgres/modules/regress/test/marginal.sql_in | 1 - .../postgres/modules/regress/test/multilogistic.ic.sql_in | 1 - src/ports/postgres/modules/regress/test/multilogistic.sql_in | 3 --- src/ports/postgres/modules/regress/test/robust.ic.sql_in | 1 - src/ports/postgres/modules/regress/test/robust.sql_in | 2 -- .../postgres/modules/stats/test/cox_prop_hazards.ic.sql_in | 1 - src/ports/postgres/modules/stats/test/cox_prop_hazards.sql_in | 2 -- 13 files changed, 1 insertion(+), 22 deletions(-) diff --git a/src/ports/postgres/modules/kmeans/test/kmeans.sql_in b/src/ports/postgres/modules/kmeans/test/kmeans.sql_in index b91998fed..8d790fa09 100644 --- a/src/ports/postgres/modules/kmeans/test/kmeans.sql_in +++ b/src/ports/postgres/modules/kmeans/test/kmeans.sql_in @@ -9,8 +9,7 @@ CREATE TABLE kmeans_2d( id SERIAL, x DOUBLE PRECISION, y DOUBLE PRECISION, - position DOUBLE PRECISION[], - PRIMARY KEY (id) + position DOUBLE PRECISION[] ) m4_ifdef(`__POSTGRESQL__', `', `DISTRIBUTED BY (id)'); INSERT INTO kmeans_2d(x, y, position) diff --git a/src/ports/postgres/modules/regress/test/linear.ic.sql_in b/src/ports/postgres/modules/regress/test/linear.ic.sql_in index 4fb539848..e1945dc8e 100644 --- a/src/ports/postgres/modules/regress/test/linear.ic.sql_in +++ b/src/ports/postgres/modules/regress/test/linear.ic.sql_in @@ -35,7 +35,6 @@ CREATE TABLE weibull ( x1 DOUBLE PRECISION, x2 DOUBLE PRECISION, y DOUBLE PRECISION - , CONSTRAINT pk_weibull PRIMARY KEY (id) ); /* diff --git a/src/ports/postgres/modules/regress/test/linear.sql_in b/src/ports/postgres/modules/regress/test/linear.sql_in index 20ab7e9a9..90131083e 100644 --- a/src/ports/postgres/modules/regress/test/linear.sql_in +++ b/src/ports/postgres/modules/regress/test/linear.sql_in @@ -15,7 +15,6 @@ CREATE TABLE weibull ( x1 DOUBLE PRECISION, x2 DOUBLE PRECISION, y DOUBLE PRECISION - , CONSTRAINT pk_weibull PRIMARY KEY (id) ) m4_ifdef(`__POSTGRESQL__', `', `DISTRIBUTED BY (id)'); /* @@ -68,7 +67,6 @@ CREATE TABLE unm ( x1 DOUBLE PRECISION, x2 DOUBLE PRECISION, y DOUBLE PRECISION - , CONSTRAINT pk_unm PRIMARY KEY (id) ); INSERT INTO unm(id, x1, x2, y) VALUES @@ -106,7 +104,6 @@ CREATE TABLE houses ( price INTEGER, size INTEGER, lot INTEGER - , CONSTRAINT pk_houses PRIMARY KEY (id) ); INSERT INTO houses(tax, bedroom, bath, price, size, lot) VALUES diff --git a/src/ports/postgres/modules/regress/test/logistic.ic.sql_in b/src/ports/postgres/modules/regress/test/logistic.ic.sql_in index bd7ed3aca..1373be14d 100644 --- a/src/ports/postgres/modules/regress/test/logistic.ic.sql_in +++ b/src/ports/postgres/modules/regress/test/logistic.ic.sql_in @@ -35,7 +35,6 @@ CREATE TABLE patients ( second_attack INTEGER, treatment INTEGER, trait_anxiety INTEGER - , CONSTRAINT pk_patient PRIMARY key (id) ); INSERT INTO patients(ID, second_attack, treatment, trait_anxiety) VALUES diff --git a/src/ports/postgres/modules/regress/test/logistic.sql_in b/src/ports/postgres/modules/regress/test/logistic.sql_in index 697dce488..fa6fc7dd2 100644 --- a/src/ports/postgres/modules/regress/test/logistic.sql_in +++ b/src/ports/postgres/modules/regress/test/logistic.sql_in @@ -15,7 +15,6 @@ CREATE TABLE patients ( second_attack INTEGER, treatment INTEGER, trait_anxiety INTEGER - , CONSTRAINT pk_patient PRIMARY key (id) )m4_ifdef(`__POSTGRESQL__', `', `DISTRIBUTED BY (id)'); INSERT INTO patients(ID, second_attack, treatment, trait_anxiety) VALUES @@ -177,7 +176,6 @@ CREATE TABLE crime ( Unemp2 INTEGER, Median INTEGER, BelowMed INTEGER - , CONSTRAINT pk_crime PRIMARY key (id) )m4_ifdef(`__POSTGRESQL__', `', `DISTRIBUTED BY (id)'); INSERT INTO crime( @@ -271,7 +269,6 @@ CREATE TABLE grad_school ( gre INTEGER, gpa DOUBLE PRECISION, rank INTEGER - , CONSTRAINT pk_grad_school PRIMARY key (id) )m4_ifdef(`__POSTGRESQL__', `', `DISTRIBUTED BY (id)'); COPY grad_school (admit, gre, gpa, rank) FROM STDIN; diff --git a/src/ports/postgres/modules/regress/test/marginal.ic.sql_in b/src/ports/postgres/modules/regress/test/marginal.ic.sql_in index 02796506c..9922342ea 100644 --- a/src/ports/postgres/modules/regress/test/marginal.ic.sql_in +++ b/src/ports/postgres/modules/regress/test/marginal.ic.sql_in @@ -29,7 +29,6 @@ CREATE TABLE patients ( second_attack INTEGER, treatment INTEGER, trait_anxiety INTEGER - , CONSTRAINT pk_patient PRIMARY key (id) ); INSERT INTO patients(ID, second_attack, treatment, trait_anxiety) VALUES ( 1, 1, 1, 70), diff --git a/src/ports/postgres/modules/regress/test/marginal.sql_in b/src/ports/postgres/modules/regress/test/marginal.sql_in index cc67b5012..4d037dadf 100644 --- a/src/ports/postgres/modules/regress/test/marginal.sql_in +++ b/src/ports/postgres/modules/regress/test/marginal.sql_in @@ -10,7 +10,6 @@ CREATE TABLE patients ( second_attack INTEGER, treatment INTEGER, trait_anxiety INTEGER - , CONSTRAINT pk_patient PRIMARY key (id) )m4_ifdef(`__POSTGRESQL__', `', `DISTRIBUTED BY (id)'); INSERT INTO patients(ID, second_attack, treatment, trait_anxiety) VALUES ( 1, 1, 1, 70), diff --git a/src/ports/postgres/modules/regress/test/multilogistic.ic.sql_in b/src/ports/postgres/modules/regress/test/multilogistic.ic.sql_in index 0c5a75976..dd2925af2 100644 --- a/src/ports/postgres/modules/regress/test/multilogistic.ic.sql_in +++ b/src/ports/postgres/modules/regress/test/multilogistic.ic.sql_in @@ -35,7 +35,6 @@ CREATE TABLE patients ( "SECOND_ATTACK" INTEGER, treatment INTEGER, trait_anxiety INTEGER - ,CONSTRAINT pk_patient PRIMARY key (id) ); INSERT INTO patients(id, "SECOND_ATTACK", treatment, trait_anxiety) VALUES diff --git a/src/ports/postgres/modules/regress/test/multilogistic.sql_in b/src/ports/postgres/modules/regress/test/multilogistic.sql_in index a97d6b46d..21bce4218 100644 --- a/src/ports/postgres/modules/regress/test/multilogistic.sql_in +++ b/src/ports/postgres/modules/regress/test/multilogistic.sql_in @@ -16,7 +16,6 @@ CREATE TABLE patients ( "SECOND_ATTACK" INTEGER, treatment INTEGER, trait_anxiety INTEGER - ,CONSTRAINT pk_patient PRIMARY key (id) ) m4_ifdef(`__POSTGRESQL__', `', `DISTRIBUTED BY (id)'); INSERT INTO patients(id, "SECOND_ATTACK", treatment, trait_anxiety) VALUES @@ -380,7 +379,6 @@ CREATE TABLE patients_with_null ( second_attack INTEGER, treatment INTEGER, trait_anxiety INTEGER - ,CONSTRAINT pk_patient_null PRIMARY key (id) ) m4_ifdef(`__POSTGRESQL__', `', `DISTRIBUTED BY (id)'); INSERT INTO patients_with_null(ID, second_attack, treatment, trait_anxiety) VALUES ( 1, 1, 1, 70), @@ -439,7 +437,6 @@ CREATE TABLE patients_all_null ( second_attack INTEGER, treatment INTEGER, trait_anxiety INTEGER - ,CONSTRAINT pk_patient_all_null PRIMARY key (id) )m4_ifdef(`__POSTGRESQL__', `', `DISTRIBUTED BY (id)'); INSERT INTO patients_all_null(ID, second_attack, treatment, trait_anxiety) VALUES ( 1, NULL, 1, 70), diff --git a/src/ports/postgres/modules/regress/test/robust.ic.sql_in b/src/ports/postgres/modules/regress/test/robust.ic.sql_in index 54a26ff8f..129fae8b6 100644 --- a/src/ports/postgres/modules/regress/test/robust.ic.sql_in +++ b/src/ports/postgres/modules/regress/test/robust.ic.sql_in @@ -34,7 +34,6 @@ CREATE TABLE weibull ( x1 DOUBLE PRECISION, x2 DOUBLE PRECISION, y DOUBLE PRECISION - , CONSTRAINT pk_weibull PRIMARY KEY (id) ); /* diff --git a/src/ports/postgres/modules/regress/test/robust.sql_in b/src/ports/postgres/modules/regress/test/robust.sql_in index c210fd67c..8f23e2280 100644 --- a/src/ports/postgres/modules/regress/test/robust.sql_in +++ b/src/ports/postgres/modules/regress/test/robust.sql_in @@ -15,7 +15,6 @@ CREATE TABLE weibull ( x1 DOUBLE PRECISION, x2 DOUBLE PRECISION, y DOUBLE PRECISION - , CONSTRAINT pk_weibull PRIMARY KEY (id) ) m4_ifdef(`__POSTGRESQL__', `', `DISTRIBUTED BY (id)'); /* @@ -102,7 +101,6 @@ CREATE TABLE patients ( second_attack INTEGER, treatment INTEGER, trait_anxiety INTEGER - , CONSTRAINT pk_patient PRIMARY key (id) ) m4_ifdef(`__POSTGRESQL__', `', `DISTRIBUTED BY (id)'); INSERT INTO patients(ID, second_attack, treatment, trait_anxiety) VALUES ( 1, 1, 1, 70), diff --git a/src/ports/postgres/modules/stats/test/cox_prop_hazards.ic.sql_in b/src/ports/postgres/modules/stats/test/cox_prop_hazards.ic.sql_in index 30854ee49..596a3208f 100644 --- a/src/ports/postgres/modules/stats/test/cox_prop_hazards.ic.sql_in +++ b/src/ports/postgres/modules/stats/test/cox_prop_hazards.ic.sql_in @@ -33,7 +33,6 @@ CREATE TABLE leukemia ( wbc DOUBLE PRECISION, timedeath INTEGER, status BOOLEAN - ,CONSTRAINT pk_leukemia PRIMARY key (id) ); diff --git a/src/ports/postgres/modules/stats/test/cox_prop_hazards.sql_in b/src/ports/postgres/modules/stats/test/cox_prop_hazards.sql_in index 7571e7f69..8b294aacf 100644 --- a/src/ports/postgres/modules/stats/test/cox_prop_hazards.sql_in +++ b/src/ports/postgres/modules/stats/test/cox_prop_hazards.sql_in @@ -16,7 +16,6 @@ CREATE TABLE leukemia ( wbc DOUBLE PRECISION, timedeath INTEGER, status BOOLEAN - ,CONSTRAINT pk_leukemia PRIMARY key (id) )m4_ifdef(, , ); @@ -101,7 +100,6 @@ CREATE TABLE leukemia ( wbc DOUBLE PRECISION, timedeath INTEGER, status BOOLEAN - ,CONSTRAINT pk_leukemia PRIMARY key (id) )m4_ifdef(, , ); INSERT INTO leukemia(id, grp, wbc, timedeath, status) VALUES From 495943059e4f55342fee70d4c3f868ed738a1a27 Mon Sep 17 00:00:00 2001 From: Jingyi Mei Date: Wed, 26 Sep 2018 13:56:04 -0700 Subject: [PATCH 2/2] Build: Add single quote while setting AppendOnly guc JIRA: MADLIB-1273 Commit 3db98babe3326fb5e2cd16d0639a2bef264f4b04 added a context manager for setting appendonly to false for all madlib modules. The commit was missing a quote around the `gp_default_storage_options` guc because of which the set command always failed. This commit adds a quote while setting the guc. Co-authored-by: Nikhil Kak --- .../postgres/modules/utilities/control.py_in | 20 ++++++++++++------- .../test/unit_tests/test_control.py_in | 8 ++++---- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/ports/postgres/modules/utilities/control.py_in b/src/ports/postgres/modules/utilities/control.py_in index d147103e7..13b22f0b1 100644 --- a/src/ports/postgres/modules/utilities/control.py_in +++ b/src/ports/postgres/modules/utilities/control.py_in @@ -199,25 +199,31 @@ class AOControl(ContextDecorator): for k, v in self.storage_options_dict.iteritems()]) def __enter__(self): + # We first check if we can get the guc value from the database using the + # show command. If this fails, then we assume that the guc doesn't exist + # and we ignore the error and return. This can happen when platform is + # postgres, or platform is gpdb but the guc doesn't exist anymore. try: _storage_options_str = plpy.execute( "show gp_default_storage_options")[0]["gp_default_storage_options"] - self._parse_gp_default_storage_options(_storage_options_str) + except plpy.SPIError: + self.guc_exists = False + return self + if self.guc_exists: + self._parse_gp_default_storage_options(_storage_options_str) # Set APPENDONLY= after backing up existing value self.was_ao_enabled = self.storage_options_dict['appendonly'] self.storage_options_dict['appendonly'] = self.to_enable - plpy.execute("set gp_default_storage_options={0}". + plpy.execute("set gp_default_storage_options='{0}'". format(self._gp_default_storage_options)) - except plpy.SPIError: - self.guc_exists = False - finally: - return self + + return self def __exit__(self, *args): if self.guc_exists: self.storage_options_dict['appendonly'] = self.was_ao_enabled - plpy.execute("set gp_default_storage_options={0}". + plpy.execute("set gp_default_storage_options='{0}'". format(self._gp_default_storage_options)) if args and args[0]: # an exception was raised in code. We return False so that any diff --git a/src/ports/postgres/modules/utilities/test/unit_tests/test_control.py_in b/src/ports/postgres/modules/utilities/test/unit_tests/test_control.py_in index 55bbd0674..66a429e10 100644 --- a/src/ports/postgres/modules/utilities/test/unit_tests/test_control.py_in +++ b/src/ports/postgres/modules/utilities/test/unit_tests/test_control.py_in @@ -55,8 +55,8 @@ class ControlTestCase(unittest.TestCase): with self.subject.AOControl(False) as C: self.assertFalse(C.storage_options_dict['appendonly']) self.plpy_mock_execute.assert_called_with( - "set gp_default_storage_options=compresstype=none,blocksize=32768" - ",appendonly=True,orientation=row,checksum=true") + "set gp_default_storage_options='compresstype=none,blocksize=32768" + ",appendonly=True,orientation=row,checksum=true'") def test_ao_control_true(self): option = ('appendonly=true,blocksize=32768,compresstype=none,' @@ -65,8 +65,8 @@ class ControlTestCase(unittest.TestCase): with self.subject.AOControl(True) as C: self.assertTrue(C.storage_options_dict['appendonly']) self.plpy_mock_execute.assert_called_with( - "set gp_default_storage_options=compresstype=none,blocksize=32768" - ",appendonly=True,orientation=row,checksum=true") + "set gp_default_storage_options='compresstype=none,blocksize=32768" + ",appendonly=True,orientation=row,checksum=true'") def test_ao_control_missing(self): option = ('appendonly=true,blocksize=32768,compresstype=none,'