Re: Converting tab-complete.c's else-if chain to a switch

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: Converting tab-complete.c's else-if chain to a switch
Date: 2024-07-13 17:16:14
Message-ID: 2632864.1720890974@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> Per discussion elsewhere [1], I've been looking at $SUBJECT.

Here's a finished patchset to perform this change. I don't
believe it has any material effect on runtime behavior or
performance, but it does achieve what Andres anticipated
would happen: the compile time for tab-complete.c drops
considerably. With gcc 8.5.0 I see the time drop from
about 3 seconds to about 0.7. The win is less noticeable
with clang version 15.0 on a Mac M1: from about 0.7s to 0.45s.
Of course the main point is the hope that it will work at all
with MSVC, but I'm not in a position to test that.

The most painful part of this is all the changes like

COMPLETE_WITH("ADD", "DROP", "OWNER TO", "RENAME", "SET",
"VALIDATE CONSTRAINT");
+ break;
/* ALTER DOMAIN <sth> DROP */
- else if (Matches("ALTER", "DOMAIN", MatchAny, "DROP"))
+ case Matches("ALTER", "DOMAIN", MatchAny, "DROP"):
COMPLETE_WITH("CONSTRAINT", "DEFAULT", "NOT NULL");
+ break;
/* ALTER DOMAIN <sth> DROP|RENAME|VALIDATE CONSTRAINT */

I despaired of doing that accurately by hand, so I wrote
a Perl script to do it. The script can cope with all but
about three of the existing tests; those have to be manually
modified before running the script, and then the actual
"switch()" has to be inserted afterwards. There won't be
any need to commit the script, since it's a one-time tool,
but I've included it for documentation's sake, along with
the "pre-convert.diff" and "post-convert.diff" manual patch
steps that are required to construct the complete 0004 patch.

I'm going to add this to the September CF, but I'd really
rather get it reviewed and committed pretty quickly.
Even with the helper script, I'm not eager to have to
rebase this a bunch of times.

regards, tom lane

Attachment Content-Type Size
v1-0001-Invent-MatchAnyN-option-for-tab-complete.c-s-Matc.patch text/x-diff 31.6 KB
v1-0002-Split-out-most-of-psql_completion-into-a-separate.patch text/x-diff 7.3 KB
v1-0003-Install-preprocessor-infrastructure-for-tab-compl.patch text/x-diff 8.7 KB
v1-0004-Convert-tab-complete-s-long-else-if-chain-to-a-sw.patch text/x-diff 199.6 KB
v1-0005-Run-pgindent-on-tab-complete.in.c.patch text/x-diff 221.9 KB
convert_tabcomplete.pl text/plain 5.1 KB
pre-convert.diff.txt text/x-diff 4.1 KB
post-convert.diff.txt text/x-diff 4.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2024-07-13 21:47:48 Re: MAINTAIN privilege -- what do we need to un-revert it?
Previous Message Arjan Marku 2024-07-13 17:03:37 [PATCH v1] Fix parsing of a complex type that has an array of complex types