[YSQL] pg_dump of sequences has incorrect ordering

[ndeodhar]

When we pg_dump a database containing sequences, pg_dump output has incorrect ordering in which CREATE TABLE uses the sequence before it is created using CREATE SEQUENCE.
Example pg_dump output:

CREATE TABLE public.seq (                                                                            
    a integer NOT NULL,                                                                              
    b integer DEFAULT nextval('public.seq_b_seq'::regclass) NOT NULL,                                
    c integer                                                                                        
);                                                                                                   
                                                                                                     
                                                                                                     
ALTER TABLE public.seq OWNER TO postgres;                                                            
                                                                                                   
--                                                                                                   
-- Name: seq_b_seq; Type: SEQUENCE; Schema: public; Owner: postgres                                  
--                                                                                                   
                                                                                                     
CREATE SEQUENCE public.seq_b_seq                                                                     
    AS integer                                                                                       
    START WITH 1                                                                                     
    INCREMENT BY 1                                                                                   
    NO MINVALUE                                                                                      
    NO MAXVALUE                                                                                      
    CACHE 1;                                                                                         
                                                                                                     
                                                                                                     
ALTER TABLE public.seq_b_seq OWNER TO postgres;                                                      
                                                                                                     
--                                                                                                   
-- Name: seq_b_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: postgres                         
--                                                                                                   
                                                                                                     
ALTER SEQUENCE public.seq_b_seq OWNED BY public.seq.b;

pg_dump on vanilla postgres works as expected though. Here's a sample output:

CREATE TABLE public.seq (                                                                            
    a integer NOT NULL,                                                                              
    b integer NOT NULL,                                                                              
    c integer                                                                                        
);                                                                                                   
                                                                                                     
                                                                                                     
ALTER TABLE public.seq OWNER TO yugabyte;                    
                                                                                                     
--                                                                                                   
-- Name: seq_b_seq; Type: SEQUENCE; Schema: public; Owner: yugabyte                                  
--                                                                                                   
                                                                                                     
CREATE SEQUENCE public.seq_b_seq                                                                     
    AS integer                                                                                       
    START WITH 1                                                                                     
    INCREMENT BY 1                                                                                   
    NO MINVALUE                                                                                      
    NO MAXVALUE                                                                                      
    CACHE 1;                                                                                         
                                                                                                     
                                                                                                     
ALTER TABLE public.seq_b_seq OWNER TO yugabyte;                                                      
                                                                                                     
--                                                                                                   
-- Name: seq_b_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: yugabyte                         
--                                                                                                   
                                                                                                     
ALTER SEQUENCE public.seq_b_seq OWNED BY public.seq.b;                                               
                                                                                                     
                                                                                                     
--                                                                                                   
-- Name: seq b; Type: DEFAULT; Schema: public; Owner: yugabyte                                       
--                                                                                                   
                                                                                                     
ALTER TABLE ONLY public.seq ALTER COLUMN b SET DEFAULT nextval('public.seq_b_seq'::regclass);

[georgeklees]

It should be noted that the issue is on the server (YB implementation of Postgres) side, not the client (pg_dump) side. When the pg_dump built in the YugaByte repository connects to a vanilla Postgres cluster, the correct output as shown in @ndeodhar's second code snippet is generated. Having it connect to a YugaByte cluster, on the other hand, results in the incorrect output.

[georgeklees]

The cause of this issue is a bug (currently being investigated) in how YSQL does system columns. pg_dump does this query of default attributes:

printfPQExpBuffer(q, "SELECT tableoid, oid, adnum, "
							  "pg_catalog.pg_get_expr(adbin, adrelid) AS adsrc "
							  "FROM pg_catalog.pg_attrdef "
							  "WHERE adrelid = '%u'::pg_catalog.oid",
							  tbinfo->dobj.catId.oid);

On a vanilla Postgres cluster it returns something like this:

postgres=# SELECT tableoid, oid, adnum, pg_catalog.pg_get_expr(adbin, adrelid) AS adsrc FROM pg_catalog.pg_attrdef WHERE adrelid = '16387'::pg_catalog.oid;
 tableoid |  oid  | adnum |              adsrc              
----------+-------+-------+---------------------------------
     2604 | 16390 |     1 | nextval('tbl1_a_seq'::regclass)

But on an equivalent YugaByte cluster it returns this:

postgres=# SELECT tableoid, oid, adnum, pg_catalog.pg_get_expr(adbin, adrelid) AS adsrc FROM pg_catalog.pg_attrdef WHERE adrelid = '16410'::pg_catalog.oid;
 tableoid |  oid  | adnum |              adsrc              
----------+-------+-------+---------------------------------
        0 | 16413 |     1 | nextval('tbl1_a_seq'::regclass)

The underlying problem on YugaByte is that the tableoid column, though present, is erroneously returned as 0 when there is a WHERE clause to filter the SELECT operation:

postgres=# SELECT tableoid FROM pg_catalog.pg_attrdef WHERE oid = 16413;
 tableoid 
----------
        0
(1 row)

postgres=# SELECT tableoid, oid FROM pg_catalog.pg_attrdef;
 tableoid |  oid  
----------+-------
     2604 | 16413
     2604 | 16645
(2 rows)

[ndeodhar]

@georgeklees Looks like index lookup does not work as expected here:

postgres=# SELECT tableoid FROM pg_catalog.pg_attrdef WHERE oid = 17472;
 tableoid 
----------
        0
(1 row)

postgres=# explain SELECT tableoid FROM pg_catalog.pg_attrdef WHERE oid = 17472;
                                      QUERY PLAN                                       
---------------------------------------------------------------------------------------
 Index Scan using pg_attrdef_oid_index on pg_attrdef  (cost=0.00..4.11 rows=1 width=4)
   Index Cond: (oid = '17472'::oid)
(2 rows)

postgres=# postgres=# explain SELECT tableoid FROM pg_catalog.pg_attrdef WHERE oid >= 17472;
                            QUERY PLAN                             
-------------------------------------------------------------------
 Foreign Scan on pg_attrdef  (cost=0.00..102.50 rows=1000 width=4)
   Filter: (oid >= '17472'::oid)
(2 rows)

postgres=# SELECT tableoid FROM pg_catalog.pg_attrdef WHERE oid <= 17472;
 tableoid 
----------
     2604
(1 row)

[georgeklees]

The root cause of the incorrect output that YBCFetchTuple() in ybcam.c, which appears to be used for index searches of non-primary keys, did not properly obtain the tableoid field. A solution is to, after this code:

		if (syscols.oid != InvalidOid)
		{
			HeapTupleSetOid(tuple, syscols.oid);
		}
		if (syscols.ybctid != NULL)
		{
			tuple->t_ybctid = PointerGetDatum(syscols.ybctid);
		}

Add this line:

tuple->t_tableOid = RelationGetRelid(relation);

This causes the output to match up with vanilla Postgres. However, we still need ALTER table ALTER column support (#1200) before this issue can be resolved.