Re: hstore parser incorrectly handles malformed input

I wrote:
> Ryan Kelly <rpkelly22@gmail.com> writes:
>> In my mind, all of these should have been rejected as erroneous input.
>> To that end, I have attached a patch which causes all of these inputs
>> to be rejected as invalid.

> Hm, I would have expected all three of these to result in "a" having
> an empty-string value.  I see nothing in the hstore documentation
> suggesting that I must write a=>"" or some such to get an empty value,

Attached is an alternative patch that fixes it that way.

Does anybody else have an opinion as to which of these solutions is
more preferable?  And should we regard this as a back-patchable bug
fix, or a definition change suitable only for HEAD?

            regards, tom lane

diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
index dde6c4b376ea2a82ed7d3cc0ef040e52f4df393a..08eafa18f1600d21e35eb4c3298215318dfebd9d 100644
*** a/contrib/hstore/hstore_io.c
--- b/contrib/hstore/hstore_io.c
*************** get_val(HSParser *state, bool ignoreeq,
*** 74,81 ****
--- 74,88 ----
              }
              else if (*(state->ptr) == '=' && !ignoreeq)
              {
+                 /* Empty key is a syntax error */
                  elog(ERROR, "Syntax error near '%c' at position %d", *(state->ptr), (int4) (state->ptr -
state->begin));
              }
+             else if (*(state->ptr) == ',' && ignoreeq)
+             {
+                 /* Empty value is perfectly OK */
+                 state->ptr--;
+                 return true;
+             }
              else if (*(state->ptr) == '\\')
              {
                  st = GV_WAITESCIN;
*************** parse_hstore(HSParser *state)
*** 191,197 ****
          if (st == WKEY)
          {
              if (!get_val(state, false, &escaped))
!                 return;
              if (state->pcur >= state->plen)
              {
                  state->plen *= 2;
--- 198,204 ----
          if (st == WKEY)
          {
              if (!get_val(state, false, &escaped))
!                 return;            /* end of string */
              if (state->pcur >= state->plen)
              {
                  state->plen *= 2;
*************** parse_hstore(HSParser *state)
*** 236,242 ****
          else if (st == WVAL)
          {
              if (!get_val(state, true, &escaped))
!                 elog(ERROR, "Unexpected end of string");
              state->pairs[state->pcur].val = state->word;
              state->pairs[state->pcur].vallen = hstoreCheckValLen(state->cur - state->word);
              state->pairs[state->pcur].isnull = false;
--- 243,252 ----
          else if (st == WVAL)
          {
              if (!get_val(state, true, &escaped))
!             {
!                 /* end of string, treat as empty value */
!                 state->ptr--;
!             }
              state->pairs[state->pcur].val = state->word;
              state->pairs[state->pcur].vallen = hstoreCheckValLen(state->cur - state->word);
              state->pairs[state->pcur].isnull = false;