Re: [Review] Re: minor patch submission: CREATE CAST ... AS EXPLICIT

From: Cédric Villemain <cedric(at)2ndquadrant(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Cc: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, Josh Berkus <josh(at)agliodbs(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Subject: Re: [Review] Re: minor patch submission: CREATE CAST ... AS EXPLICIT
Date: 2013-07-13 10:52:11
Message-ID: 201307131252.14949.cedric@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> >> Generally speaking, I agree with Robert's objection. The patch in
> >> itself adds only one unnecessary keyword, which probably wouldn't be
> >> noticeable, but the argument for it implies that we should be willing
> >> to add a lot more equally-unnecessary keywords, which I'm not. gram.o
> >> is already about 10% of the entire postgres executable, which probably
> >> goes far towards explaining why its inner loop always shows up high in
> >> profiling: cache misses are routine. And the size of those tables is
> >> at least linear in the number of keywords --- perhaps worse than linear,
> >> I'm not sure. Adding a bunch of keywords *will* cost us in performance.
> >> I'm not willing to pay that cost for something that adds neither
> >> features nor spec compliance.
>
> (1) Here are objective measures of the postgres stripped binary size
> compiled from scratch on my laptop this morning:

amd64, gcc version 4.7.3 (Debian 4.7.3-4)
then gcc version 4.8.1 (Debian 4.8.1-6)

with no option to configure, I got:

> - without "AS EXPLICIT": 5286408 bytes
> gram.o: 904936 bytes
> keywords.o: 20392 bytes

keywords.o : 20376 bytes
gram.o: 909240 bytes

keywords.o : 20376 bytes
gram.o: 900504 bytes

>
> - with "AS EXPLICIT" : 5282312 bytes
> gram.o: 901632 bytes
> keywords.o: 20440 bytes

keywords.o : 20424 bytes
gram.o: 905904 bytes

keywords.o : 20424 bytes
gram.o: 897168 bytes

> The executable binary is reduced by 4 KB with AS EXPLICIT, which
> seems to come from some "ld" flucke really, because the only difference
> I've found are the 8 bytes added to "keywords.o". This must be very
> specific to the version and options used with gcc & ld on my laptop.

same here, amd64. gcc to more impact, I didn't tryed with clang.

> As for the general issue with the parser size: I work with languages and
> compilers as a researcher. We had issues at one point with a scanner
> because of too many keywords, and we solved it by removing keywords from
> the scanner and checking them semantically in the parser with a hash
> table, basically as suggested by Andres. The SQL syntax is pretty
> redundant so that there are little choices at each point. Some tools can
> generate perfect hash functions that can help (e.g. gperf). However I'm
> not sure of the actual impact in size and time by following this path, I'm
> just sure that there would be less keywords. IMHO, this issue is
> orthogonal & independent from this patch.
>
> Finally, to answer your question directly, I'm really a nobody here, so
> you can do whatever pleases you with the patch.

I have no strong objection to the patch. It is only decoration and should not
hurt.
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2013-07-13 12:02:59 Re: proposal: simple date constructor from numeric values
Previous Message Abhijit Menon-Sen 2013-07-13 10:21:14 Re: Patch to add support of "IF NOT EXISTS" to others "CREATE" statements