Re: [Review] Re: minor patch submission: CREATE CAST ... AS EXPLICIT - Mailing list pgsql-hackers

From Cédric Villemain
Subject Re: [Review] Re: minor patch submission: CREATE CAST ... AS EXPLICIT
Date
Msg-id 201307131252.14949.cedric@2ndquadrant.com
Whole thread Raw
In response to Re: [Review] Re: minor patch submission: CREATE CAST ... AS EXPLICIT  (Fabien COELHO <coelho@cri.ensmp.fr>)
List 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

pgsql-hackers by date:

Previous
From: Abhijit Menon-Sen
Date:
Subject: Re: Patch to add support of "IF NOT EXISTS" to others "CREATE" statements
Next
From: Pavel Stehule
Date:
Subject: Re: proposal: simple date constructor from numeric values