public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Jeff Davis <[email protected]>
Cc: Nathan Bossart <[email protected]>
Cc: Joe Conway <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: Davin Shearer <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: micro-optimizing json.c
Date: Thu, 07 Dec 2023 19:40:30 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <20231207231251.GB3359478@nathanxps13>
	<[email protected]>

Jeff Davis <[email protected]> writes:
> Nice improvement. The use of (len = ...) in a conditional is slightly
> out of the ordinary, but it makes the conditionals a bit simpler and
> you have a comment, so it's fine with me.

Yeah.  It's a little not per project style, but I don't see a nice
way to do it differently, granting that we don't want to put the
strlen() ahead of the !key_scalar test.  More straightforward
coding would end up with two else-path calls of escape_json,
which doesn't seem all that much more straightforward.

> I wonder, if there were an efficient cast from numeric to text, then
> perhaps you could avoid the strlen() entirely?

Hmm ... I think that might not be the way to think about it.  What
I'm wondering is why we need a test as expensive as IsValidJsonNumber
in the first place, given that we know this is a numeric data type's
output.  ISTM we only need to reject "Inf"/"-Inf" and "NaN", which
surely does not require a full parse.  Skip over a sign, check for
"I"/"N", and you're done.

... and for that matter, why does quoting of Inf/NaN require
that we apply something as expensive as escape_json?  Several other
paths in this switch have no hesitation about assuming that they
can just plaster double quotes around what was emitted.  How is
that safe for timestamps but not Inf/NaN?

> In any case, +1 to your simple change.

If we end up not using IsValidJsonNumber then this strlen hackery
would become irrelevant, so maybe that idea should be looked at first.

			regards, tom lane

view thread (3+ 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], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: micro-optimizing json.c
  In-Reply-To: <[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