Re: hstore parser incorrectly handles malformed input

On Fri, Apr 27, 2012 at 10:22:11AM -0400, Tom Lane wrote:
> Ryan Kelly <rpkelly22@gmail.com> writes:
> > As long as we make it consistent on both sides of the '=>' (and document
> > it, too), then I don't really care either way. Currently you have to use
> > quotes to get an empty key, so I thought it natural to that you should
> > have to quote to get an empty value.
>
> > I've attached a modified version of Tom's patch which also allows empty
> > keys.
>
> Hm ... I don't agree that keys and values are interchangeable, and
> I don't see that empty keys are a good thing (whereas empty values are
> clearly a reasonable edge case).  So I think this is going a bit far;
> it seems to me it'd be giving up a lot of syntax-error detection
> capability in return for some not-actually-helpful symmetry.
I don't think any language supports the empty key edge case in this way.
The only language I know of that will let you get away with it is Perl,
and in that case you get undef, not the empty string, and it's a warning
if you have warnings on:

$ use warnings;
$ my $h = { 1 => };
Odd number of elements in anonymous hash
$HASH1 = { 1 => undef };

In fact, it's akin to doing:
[db]> select hstore('{foo}'::text[]);
ERROR:  array must have even number of elements

Or maybe even this:
[db]> select hstore('{foo,}'::text[]);
ERROR:  malformed array literal: "{foo,}"

Python:
>>> h = {1 : }
SyntaxError: invalid syntax

Ruby:
>> h = {1 => }
SyntaxError: compile error

C#:
csharp> var h = new Dictionary<String, String>(){ { "1", } };
{interactive}(1,50): error CS1525: Unexpected symbol `}'
csharp> var h = new Dictionary<String, String>(){ { "1" } };
{interactive}(1,44): error CS1501: No overload for method `Add' takes `1' arguments

JavaScript:
js> a = { 1: }
typein:1: SyntaxError: syntax error:

PHP:
$ echo '<?php $h = array( 1 => ); ?>' | php
PHP Parse error:  syntax error, unexpected ')'

> On the other hand, I seldom use hstore so I'm probably not the best
> person to be judging the appropriateness of these options.  Any other
> votes out there?
As long as we're voting, I vote for my original patch. I think requiring
something on the RHS of => to get a value is a sane and rather common thing.

>
>             regards, tom lane

-Ryan Kelly