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

Hello Josh,

>> 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:
 - without "AS EXPLICIT": 5286408 bytes                  gram.o:  904936 bytes              keywords.o:   20392 bytes
 - with "AS EXPLICIT"   : 5282312 bytes                  gram.o:  901632 bytes              keywords.o:   20440 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.

(2) user annoyance vs cycles

I strongly disagree that user annoyance is of little value. This is one of 
the reason why I cannot stand MySQL (oops, EXCEPT is not implemented, 
casts are not for all types, the default engine is not ACID, the standard 
information_schema implementation does *NOT* implement the standard...).
I've made an extensive argument earlier in the thread.

> Where are we with this patch?  Fabien, are you going to submit an
> updated version which addresses the objections, or should I mark it
> Returned With Feedback?

There is no need for an updated patch. I addressed the objections with 
words, not code:-)

I've stronly argued against the premises of Robert & Tom objections. 
Moreover, it happens that the patch does reduce, because of some flucke, 
the executable size, which is one of the motivation for the objections.

I have no doubt that the potential cache misses induced by the 8 bytes
extension of the keyword table are of less value that user time and
annoyance.

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.

-- 
Fabien.