Re: multiset patch review

On Sat, Feb 12, 2011 at 00:50, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> Right, but making the parser slower has a cost, too.
>> ScanKeywordLookup() is already a hotspot in some workloads, and
>> there's overhead buried in the bison parser, too.
>
> Yeah.  Keep in mind that a bison parser fundamentally runs off a
> two-dimensional array: one axis is parser state and the other is token
> number.  They have some tricks to compress the array a bit, but adding
> keywords contributes directly to a bigger array, which means slower
> parsing (more L1 cache misses).  The parser's inner loop frequently
> shows up as a hotspot in profiles I do, and I think that has to be more
> about the amount of data it's touching than the cost of the loop per se.

Did you measure the actual cost in the real world? If we are using
such a sensitive parser, it should be a problem even without the patch.

> Adding unnecessary keywords is something to be avoided.

Our conclusion is "we never support multiset syntax in the SQL standard",
right? If we think they are unnecessary, we cannot support it.

I will remove parser changes from the patch; it will add only a few array
functions. Then, please let me know functions you don't want to include
in the core, if any. I'll remove them at the same time.

--
Itagaki Takahiro