Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve DDL replication related regression tests. #360

Merged
merged 2 commits into from
Mar 17, 2016

Conversation

anarazel
Copy link
Contributor

The previous form of the test, utilizing DEBUG2, included too much
output dependent on the specifc system and version. Reformulate it to
explicitly connect to workers and show the schema there, when necessary.

@sumedhpathak I guess @samay-sharma can be the reviewer here?

@anarazel anarazel force-pushed the ddl-rep-regressiontest-improvements branch 2 times, most recently from d494fdb to 721d816 Compare February 24, 2016 23:23
@samay-sharma samay-sharma self-assigned this Feb 25, 2016
@samay-sharma
Copy link
Contributor

Could you check if we need the multi_alter_table_statements_0.source file?

I think the primary reason for this file was the differing DEBUG output across 9.4 / 9.5. Now that you've removed the DEBUG statements, I think it should be safe to remove the file.

@samay-sharma
Copy link
Contributor

@anarazel : This looks good, other than the few minor comments I added above.

I also ran check-full and all tests pass, so should be good to go once you've looked at the above.

@anarazel
Copy link
Contributor Author

anarazel commented Mar 2, 2016

WRT hash indexes: In theory I agree, but given that adding a gin/gist index for a similar test would be just as complicated as the hash index test (due to them only being available for few datatypes without extension), I'm inclined to leave it as is.

@anarazel anarazel force-pushed the ddl-rep-regressiontest-improvements branch 2 times, most recently from 5af6f99 to 048a6ee Compare March 2, 2016 01:08
@ozgune ozgune added this to the 5.1 Release milestone Mar 2, 2016
@onderkalaci
Copy link
Member

@samay-sharma There are two PRs (#385 and #388) which are changing the outputs of multi_alter_table_statements.

So, are there any blockers to check-in this?

Thanks!

@anarazel anarazel force-pushed the ddl-rep-regressiontest-improvements branch from 048a6ee to 52c48fe Compare March 17, 2016 22:47
There already exist tests that locally embed knowledge about port
numbers, and there's more tests requiring that. Instead of copying
\set's to several tests, make these port number variables available to
all tests.
The previous form of the test, utilizing DEBUG2, included too much
output dependent on the specifc system and version. Reformulate it to
explicitly connect to workers and show the schema there, when necessary.

The only remaining difference in some of the remaining alternate
regression test files was due to an older minor version release
change. Remove those as well.
@anarazel anarazel force-pushed the ddl-rep-regressiontest-improvements branch from 52c48fe to 5330946 Compare March 17, 2016 23:06
anarazel added a commit that referenced this pull request Mar 17, 2016
…ments

Improve DDL replication related regression tests.

Reviewed-By: Samay Sharma
@anarazel anarazel merged commit 3e512db into master Mar 17, 2016
@anarazel anarazel deleted the ddl-rep-regressiontest-improvements branch March 17, 2016 23:16
@onderkalaci
Copy link
Member

Hi @anarazel, @samay-sharma

After this checked-in, the regression tests fail on PG 9.4.5 on my MacOS box on the master branch. I also tested this on an UBUNTU 14.04, the same result.

I don't understand how it passed on travis. The diff output is:

*** /Users/onderkalaci/Documents/citus_code/citus/src/test/regress/expected/multi_master_protocol.out   Thu Feb 11 19:35:17 2016
--- /Users/onderkalaci/Documents/citus_code/citus/src/test/regress/results/multi_master_protocol.out    Fri Mar 18 14:02:37 2016
***************
*** 14,20 ****
  ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
   CREATE TABLE lineitem (l_orderkey bigint NOT NULL, l_partkey integer NOT NULL, l_suppkey integer NOT NULL, l_linenumber integer NOT NULL, l_quantity numeric(15,2) NOT NULL, l_extendedprice numeric(15,2) NOT NULL, l_discount numeric(15,2) NOT NULL, l_tax numeric(15,2) NOT NULL, l_returnflag character(1) NOT NULL, l_linestatus character(1) NOT NULL, l_shipdate date NOT NULL, l_commitdate date NOT NULL, l_receiptdate date NOT NULL, l_shipinstruct character(25) NOT NULL, l_shipmode character(10) NOT NULL, l_comment character varying(44) NOT NULL)
   CREATE INDEX lineitem_time_index ON lineitem USING btree (l_shipdate)
!  ALTER TABLE public.lineitem ADD CONSTRAINT lineitem_pkey PRIMARY KEY (l_orderkey, l_linenumber)
  (3 rows)

  SELECT * FROM master_get_new_shardid();
--- 14,20 ----
  ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
   CREATE TABLE lineitem (l_orderkey bigint NOT NULL, l_partkey integer NOT NULL, l_suppkey integer NOT NULL, l_linenumber integer NOT NULL, l_quantity numeric(15,2) NOT NULL, l_extendedprice numeric(15,2) NOT NULL, l_discount numeric(15,2) NOT NULL, l_tax numeric(15,2) NOT NULL, l_returnflag character(1) NOT NULL, l_linestatus character(1) NOT NULL, l_shipdate date NOT NULL, l_commitdate date NOT NULL, l_receiptdate date NOT NULL, l_shipinstruct character(25) NOT NULL, l_shipmode character(10) NOT NULL, l_comment character varying(44) NOT NULL)
   CREATE INDEX lineitem_time_index ON lineitem USING btree (l_shipdate)
!  ALTER TABLE ONLY lineitem ADD CONSTRAINT lineitem_pkey PRIMARY KEY (l_orderkey, l_linenumber)
  (3 rows)

  SELECT * FROM master_get_new_shardid();

======================================================================

/Users/onderkalaci/Documents/citus_code/citus/src/test/regress/regression.diffs (END)

How should we tackle this?

@anarazel
Copy link
Contributor Author

Update to a recent 9.4 release (i.e. 9.4.6), that's a difference between
the minor versions.

On Fri, Mar 18, 2016 at 5:33 AM, Önder Kalacı [email protected]
wrote:

Hi @anarazel https://github.com/anarazel, @samay-sharma
https://github.com/samay-sharma

After this checked-in, the regression tests fail on PG 9.4.5 on my MacOS
box on the master branch. I also tested this on an UBUNTU 14.04, the same
result.

I don't understand how it passed on travis. The diff output is:

*** /Users/onderkalaci/Documents/citus_code/citus/src/test/regress/expected/multi_master_protocol.out Thu Feb 11 19:35:17 2016
--- /Users/onderkalaci/Documents/citus_code/citus/src/test/regress/results/multi_master_protocol.out Fri Mar 18 14:02:37 2016


*** 14,20 ****


CREATE TABLE lineitem (l_orderkey bigint NOT NULL, l_partkey integer NOT NULL, l_suppkey integer NOT NULL, l_linenumber integer NOT NULL, l_quantity numeric(15,2) NOT NULL, l_extendedprice numeric(15,2) NOT NULL, l_discount numeric(15,2) NOT NULL, l_tax numeric(15,2) NOT NULL, l_returnflag character(1) NOT NULL, l_linestatus character(1) NOT NULL, l_shipdate date NOT NULL, l_commitdate date NOT NULL, l_receiptdate date NOT NULL, l_shipinstruct character(25) NOT NULL, l_shipmode character(10) NOT NULL, l_comment character varying(44) NOT NULL)
CREATE INDEX lineitem_time_index ON lineitem USING btree (l_shipdate)
! ALTER TABLE public.lineitem ADD CONSTRAINT lineitem_pkey PRIMARY KEY (l_orderkey, l_linenumber)
(3 rows)

SELECT * FROM master_get_new_shardid();
--- 14,20 ----


CREATE TABLE lineitem (l_orderkey bigint NOT NULL, l_partkey integer NOT NULL, l_suppkey integer NOT NULL, l_linenumber integer NOT NULL, l_quantity numeric(15,2) NOT NULL, l_extendedprice numeric(15,2) NOT NULL, l_discount numeric(15,2) NOT NULL, l_tax numeric(15,2) NOT NULL, l_returnflag character(1) NOT NULL, l_linestatus character(1) NOT NULL, l_shipdate date NOT NULL, l_commitdate date NOT NULL, l_receiptdate date NOT NULL, l_shipinstruct character(25) NOT NULL, l_shipmode character(10) NOT NULL, l_comment character varying(44) NOT NULL)
CREATE INDEX lineitem_time_index ON lineitem USING btree (l_shipdate)
! ALTER TABLE ONLY lineitem ADD CONSTRAINT lineitem_pkey PRIMARY KEY (l_orderkey, l_linenumber)
(3 rows)

SELECT * FROM master_get_new_shardid();

/Users/onderkalaci/Documents/citus_code/citus/src/test/regress/regression.diffs (END)

How should we tackle this?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#360 (comment)

@anarazel
Copy link
Contributor Author

@onderkalaci Have you verified that the above this indeed the cause of your problem?

@onderkalaci
Copy link
Member

Yes @anarazel, I verified that is a difference between minor versions (i.e., all tests pass on 9.4.6 whereas multi_master_protocol fails on 9.4.5 and below).

@onderkalaci onderkalaci mentioned this pull request Mar 23, 2016
DimCitus pushed a commit that referenced this pull request Jan 10, 2018
…ments

Improve DDL replication related regression tests.

Reviewed-By: Samay Sharma
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants