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
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 |