[YSQL] ysql_dump should consistently use quotes on column names

[ddorian]

On 2.3.2.0:
1.

kirankg=# create table chat_user("chatID" text NOT NULL, PRIMARY KEY("chatID")); 
kirankg=# \d+ chat_user
 Column | Type | Collation | Nullable | Default | Storage  | Stats target | Description 
--------+------+-----------+----------+---------+----------+--------------+-------------
 chatID | text |           | not null |         | extended |              | 
Indexes:
    "chat_user_pkey" PRIMARY KEY, lsm ("chatID" HASH)

2. Export with ysql_dump:

./postgres/bin/ysql_dump kirankg -f my_db.sql

3. Output is:

CREATE TABLE public.chat_user (
    "chatID" text NOT NULL,
    PRIMARY KEY((chatID) HASH)
);

Which is not valid even in normal PostgreSQL:

kirankg=# CREATE TABLE public.chat_user ("chatID" text NOT NULL, PRIMARY KEY((chatID) HASH) );
ERROR:  column "chatid" named in key does not exist
LINE 1: ...E TABLE public.chat_user ("chatID" text NOT NULL, PRIMARY KE...

The column names have to be inside/outside quotes in both cases.

[tedyu]

We can utilize --quote-all-identifiers option. Though there is code change required.

Or, calling fmtId() is simpler fix.
The generated script becomes valid SQL.

[ddorian]

@tedyu --quote-all-identifiers doesn't work:

$ ./postgres/bin/ysql_dump --quote-all-identifiers kirankg
.....
CREATE TABLE "public"."chat_user" (
    "chatID" "text" NOT NULL,
    PRIMARY KEY((chatID) HASH)
);


ALTER TABLE "public"."chat_user" OWNER TO "yugabyte";

--
-- Data for Name: chat_user; Type: TABLE DATA; Schema: public; Owner: yugabyte
--

COPY "public"."chat_user" ("chatID") FROM stdin;
\.
.....

It used quotes on ALTER TABLE & COPY but not in the PRIMARY KEY().

[tedyu]

There used to be code change shown in my previous comment.

See the above PR for the current proposal.

[ddorian]

@tedyu note that this isn't still available in 2.5.0.0 (just tested). Tried with --quote-all-identifiers too.
Can you recheck ?

[ddorian]

This is fixed but didn't enter 2.5.0.0 because of a mistaken commit tagging. It will be in the release. (verified that it works on master).