public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: Evan Jones <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] hstore: Fix parsing on Mac OS X: isspace() is locale specific
Date: Sun, 18 Jun 2023 10:50:16 +0900
Message-ID: <ZI5i2NCWirDuE/[email protected]> (raw)
In-Reply-To: <CA+HWA9bTRDf52DHyU+JOoqEALgRGRo5uHUYTFuduoj3cBfer+Q@mail.gmail.com>
References: <CA+HWA9awUW0+RV_gO9r1ABZwGoZxPztcJxPy8vMFSTbTfi4jig@mail.gmail.com>
	<[email protected]>
	<CA+HWA9bET27MxdO4VOgK==w4P_G9H0aStCq+C_DP4YnarAw87g@mail.gmail.com>
	<[email protected]>
	<CA+HWA9bTRDf52DHyU+JOoqEALgRGRo5uHUYTFuduoj3cBfer+Q@mail.gmail.com>

On Sat, Jun 17, 2023 at 10:57:05AM -0400, Evan Jones wrote:
> However, if we think this change could be a problem, one fix would be to
> switch scanner_isspace() to array_isspace(), which returns true for these
> *six* ASCII characters. I am happy to submit a patch to do this.

The difference between scanner_isspace() and array_isspace() is that
the former matches with what scan.l stores as rules for whitespace
characters, but the latter works on values.  For hstore, we want the
latter, with something that works on values.  To keep the change
locale to hstore, I think that we should just introduce an
hstore_isspace() which is a copy of array_isspace.  That's a
duplication, sure, but I think that we may want to think harder about
\v in the flex scanner, and that's just a few extra lines for 
something that has not changed in 13 years for arrays.  That's also
easier to think about for stable branches.  If you can send a patch,
that helps a lot, for sure!

Worth noting that the array part has been changed in 2010, with
95cacd1, for the same reason as what you've proposed for hstore.
Thread is here, and it does not mention our flex rules, either:
https://www.postgresql.org/message-id/[email protected]

Perhaps we could consider \v as a whitespace in the flex scanner
itself, but I am scared to do that in any stable branch.  Perhaps
we could consider that for HEAD in 17~?  That's a lot to work around
an old BSD bug that macOS has inherited, though.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../ZI5i2NCWirDuE%[email protected]/2-signature.asc)
  download

view thread (4+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: [PATCH] hstore: Fix parsing on Mac OS X: isspace() is locale specific
  In-Reply-To: <ZI5i2NCWirDuE/[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox