Invalid YAML output from EXPLAIN

-----BEGIN PGP SIGNED MESSAGE-----
Hash: RIPEMD160

Dean Rasheed wrote:
...

So the current code in escape_yaml() is inadequate for producing valid
YAML. I think it would have to also consider at least the following
characters as special "-" ":" "[" "]" "{" "}" "," "\"" "'"
"|" "*" "&". Technically, it would also need to trap empty strings,
and strings with leading or trailing whitespace.

Making escape_yaml() completely bulletproof with this approach would
be quite difficult, and (IMO) not worth the effort

...

Doesn't seem like a lot of effort to me. You've already laid out most of
the exceptions above, although they require a few tweaks.
The rules should be:

Requires quoting only if the first character:
& * ! | > ' " % @ ` #

Same as above, but no quoting if the second character is "safe":
- ? :

Always requires quoting:
":<space>" "<space>#" aka ': ' ' #'

Always requires quoting:
, [ ] { }

Always require quoting:
(leading space) (trailing space) (empty string)

See:
http://yaml.org/spec/1.2/spec.html section 5.3 and 7.3.3

- --
Greg Sabino Mullane greg@turnstep.com
End Point Corporation http://www.endpoint.com/
PGP Key: 0x14964AC8 201006070943
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----

iEYEAREDAAYFAkwM+wAACgkQvJuQZxSWSsgWZACcCgb0rDvA6ZVhHId/q568gBGo
sjgAoLY7HbkI7sRpO45vi0jSRJ2Fiytk
=v7T/
-----END PGP SIGNATURE-----

"Greg Sabino Mullane" <greg@turnstep.com> writes:

The rules should be:

Requires quoting only if the first character:
& * ! | > ' " % @ ` #

Same as above, but no quoting if the second character is "safe":
- ? :

Always requires quoting:
":<space>" "<space>#" aka ': ' ' #'

Always requires quoting:
, [ ] { }

Always require quoting:
(leading space) (trailing space) (empty string)

Egad ... this is supposed to be an easily machine-generatable format?

If it's really as broken as the above suggests, I think we should
rip it out while we still can.

regards, tom lane

-----BEGIN PGP SIGNED MESSAGE-----
Hash: RIPEMD160

Tom Lane wrote:
...

Egad ... this is supposed to be an easily machine-generatable format?

If it's really as broken as the above suggests, I think we should
rip it out while we still can.

Heh ... not like you to shrink from a challenge. ;)

I don't think the above would be particularly hard to implement myself,
but if it becomes a really big deal, we can certainly punt by simply
quoting anything containing an indicator (the special characters above).
It will still be 100% valid YAML, just with some excess quoting for the
very rare case when a value contains one of the special characters.

- --
Greg Sabino Mullane greg@turnstep.com
End Point Corporation http://www.endpoint.com/
PGP Key: 0x14964AC8 201006071035
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----

iEYEAREDAAYFAkwNA+4ACgkQvJuQZxSWSshSswCg81kd3FdYnQup1eLWGesm+vm+
VO8AoL1Fwil/vXfRdRHx4A4zZUTDbZuT
=oPDv
-----END PGP SIGNATURE-----

On Mon, Jun 7, 2010 at 10:37 AM, Greg Sabino Mullane <greg@turnstep.com> wrote:

Tom Lane wrote:
I don't think the above would be particularly hard to implement myself,
but if it becomes a really big deal, we can certainly punt by simply
quoting anything containing an indicator (the special characters above).
It will still be 100% valid YAML, just with some excess quoting for the
very rare case when a value contains one of the special characters.

Since you're the main advocate of this feature, I think you should
implement it rather than leaving it to Tom or I.

The reason why I was initially skeptical of adding a YAML output
format is that JSON is a subset of YAML. Therefore, the JSON output
format ought to be perfectly sufficient for anyone using a YAML
parser. If it's not, that's because their YAML processor is broken,
and they should get a new one, or because the YAML spec is defective.
The YAML format got voted in by consensus because people thought that
it would also make a nice alternative to the text format for human
readable output. I don't believe that (it uses way too much vertical
space) but even if you accept the argument, the more we make the YAML
format look like the JSON format, the less water that argument holds.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

Robert Haas wrote:

On Mon, Jun 7, 2010 at 10:37 AM, Greg Sabino Mullane <greg@turnstep.com> wrote:

Tom Lane wrote:
I don't think the above would be particularly hard to implement myself,
but if it becomes a really big deal, we can certainly punt by simply
quoting anything containing an indicator (the special characters above).
It will still be 100% valid YAML, just with some excess quoting for the
very rare case when a value contains one of the special characters.

Since you're the main advocate of this feature, I think you should
implement it rather than leaving it to Tom or I.

Or anyone else :-)

The reason why I was initially skeptical of adding a YAML output
format is that JSON is a subset of YAML. Therefore, the JSON output
format ought to be perfectly sufficient for anyone using a YAML
parser.

There is some debate on this point, IIRC.

cheers

andrew

"Greg Sabino Mullane" <greg@turnstep.com> writes:

I don't think the above would be particularly hard to implement myself,
but if it becomes a really big deal, we can certainly punt by simply
quoting anything containing an indicator (the special characters above).

I would go with that. The quoting rules you proposed previously seem
way too complicated --- meaning potentially buggy, and even if they're
not buggy, the behavior would seem unpredictable to most users.

regards, tom lane

On 7 June 2010 15:56, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Greg Sabino Mullane" <greg@turnstep.com> writes:

I don't think the above would be particularly hard to implement myself,
but if it becomes a really big deal, we can certainly punt by simply
quoting anything containing an indicator (the special characters above).

I would go with that.  The quoting rules you proposed previously seem
way too complicated --- meaning potentially buggy, and even if they're
not buggy, the behavior would seem unpredictable to most users.

Well actually it's not just everything containing a special character,
it's also anything with leading or trailing whitespace, and empty
strings (not sure that can ever happen in practice).

It's because of the potential for bugs in this area, that I'd propose
just quoting everything (except numeric values) as in my original
patch.

Regards,
Dean

* Tom Lane:

Egad ... this is supposed to be an easily machine-generatable format?

Perhaps you could surround all strings with "" in the generator, and
escape all potentially special characters (which seems to include some
whitespace even in quoted strings, unfortunately)?

It has been claimed before that YAML is a superset of JSON, so why
can't the YAML folks use the existing JSON output instead?

--
Florian Weimer <fweimer@bfk.de>
BFK edv-consulting GmbH http://www.bfk.de/
Kriegsstraße 100 tel: +49-721-96201-1
D-76133 Karlsruhe fax: +49-721-96201-99

Florian Weimer wrote:

It has been claimed before that YAML is a superset of JSON, so why
can't the YAML folks use the existing JSON output instead?

Because JSON just crosses the line where it feels like there's so much
markup that people expect a tool is necessary to read it, which has
always been the issue with XML too--bad human readability. I was on the
fence about YAML until I used it for a client issue over the weekend. I
was able to hack together a quick tool to work on the issue that parsed
enough YAML *without using an external library* well enough for my
purposes in an hour, one that was still far more robust than a similar
hack trying to read plain old text format for EXPLAIN. And the client
was able to follow what was going on as I passed YAML output back and
forth with them. Just having every field labeled clearly cut off all
the usual "which of these is the startup cost again?" questions I'm used
to getting.

The complaints about YAML taking up too much vertical space are
understandable, but completely opposite of what I care about. I can
e-mail a customer a YAML plan and it will survive to the other side and
even in a reply back to me. Whereas any non-trivial text format one is
guaranteed to utterly destroyed by line wrapping along the way.

I think this thread could use a fresh example to remind anyone who
hasn't played with the curent YAML format what it looks like. Here's
one from a query against the Dell Store 2 database:

EXPLAIN SELECT * FROM customers WHERE customerid>1000 ORDER BY zip;
QUERY PLAN
----------
Sort (cost=4449.30..4496.80 rows=19000 width=268)
Sort Key: zip
-> Seq Scan on customers (cost=0.00..726.00 rows=19000 width=268)
Filter: (customerid > 1000)

EXPLAIN (FORMAT YAML) SELECT * FROM customers WHERE customerid>1000
ORDER BY zip;
QUERY PLAN
-------------------------------------
- Plan: +
Node Type: Sort +
Startup Cost: 4449.30 +
Total Cost: 4496.80 +
Plan Rows: 19000 +
Plan Width: 268 +
Sort Key: +
- zip +
Plans: +
- Node Type: Seq Scan +
Parent Relationship: Outer +
Relation Name: customers +
Alias: customers +
Startup Cost: 0.00 +
Total Cost: 726.00 +
Plan Rows: 19000 +
Plan Width: 268 +
Filter: (customerid > 1000)

--
Greg Smith 2ndQuadrant US Baltimore, MD
PostgreSQL Training, Services and Support
greg@2ndQuadrant.com www.2ndQuadrant.us

Greg Smith <greg@2ndquadrant.com> writes:

The complaints about YAML taking up too much vertical space are
understandable, but completely opposite of what I care about. I can
e-mail a customer a YAML plan and it will survive to the other side and
even in a reply back to me. Whereas any non-trivial text format one is
guaranteed to utterly destroyed by line wrapping along the way.

I think this thread could use a fresh example to remind anyone who
hasn't played with the curent YAML format what it looks like.

So? This doesn't look amazingly unlike the current JSON output,
and to the extent that we have to add more quoting to it, it's
going to look even more like the JSON output.

Given the lack of any field separators other than newlines, I'm also
finding myself extremely doubtful about the claim that it survives
line-wrapping mutilations well. For instance this bit:

- Node Type: Seq Scan
Parent Relationship: Outer

doesn't appear to have anything but whitespace to distinguish it from

- Node Type: Seq Scan Parent
Relationship: Outer

regards, tom lane

It's because of the potential for bugs in this area, that I'd propose
just quoting everything (except numeric values) as in my original
patch.

I don't see a problem with this.

I supported YAML output because I find it easier to read and copy&paste
than the other outputs. This is still the case even with quoting. And
it's not exactly a hugely intrusive patch.

--
-- Josh Berkus
PostgreSQL Experts Inc.
http://www.pgexperts.com

Tom Lane wrote:

This doesn't look amazingly unlike the current JSON output,
and to the extent that we have to add more quoting to it, it's
going to look even more like the JSON output.

I don't know about that; here's the JSON one:

EXPLAIN (FORMAT JSON) SELECT * FROM customers WHERE customerid>1000
ORDER BY zip;
QUERY PLAN
-------------------------------------------
[ +
{ +
"Plan": { +
"Node Type": "Sort", +
"Startup Cost": 4449.30, +
"Total Cost": 4496.80, +
"Plan Rows": 19000, +
"Plan Width": 268, +
"Sort Key": ["zip"], +
"Plans": [ +
{ +
"Node Type": "Seq Scan", +
"Parent Relationship": "Outer",+
"Relation Name": "customers", +
"Alias": "customers", +
"Startup Cost": 0.00, +
"Total Cost": 726.00, +
"Plan Rows": 19000, +
"Plan Width": 268, +
"Filter": "(customerid > 1000)"+
} +
] +
} +
} +
]

From the perspective of how that's less useful as a human form of
output, it's longer, wider, and has redundant punctuation that gets in
the way.

I think that YAML quoting will need to respect one of the special cases
to keep from ruining its readability: "Requires quoting only if the
first character" for " will make its current format look terrible if
that rule is applied to the whole line instead. That sounds like a
necessary special case to include: don't quote any quote characters
that appear unless they're the first character on the line. Everything
else could switch back to really aggressive quoting in every spot and
that wouldn't hurt the readability of the format very much IMHO.

Given the lack of any field separators other than newlines, I'm also
finding myself extremely doubtful about the claim that it survives
line-wrapping mutilations well.

All I was claiming there is that the output is dramatically less wide
than the standard text format of the same plan, and therefore far less
likely to get nailed by a mail client that wraps at normal line widths.
Agreed that once wrapping does occur, it has serious problems too.

Here are the stats for this plan, leaving off the QUERY PLAN header from
each:

TEXT: 4 vertical, 69 horizontal
YAML: 18 vertical, 36 horizontal
JSON: 25 vertical, 43 horizontal
XML[1]Might as well make this a complete example:: 27 vertical, 60 horizontal

Quote the TEXT line with "> " or get a plan with one more line of
intendation, and you're likely to get wrapped badly at the 72 character
line limit some clients use. Quite a bit more headroom before the YAML
format will wrap like that; JSON is in the middle.

I now see plenty of use for YAML when exchanging plans over e-mail, and
it's a bonus that should survive that format to be parseable on the
other side. JSON and XML are certainly the preferred way to feed plans
into analysis tools. unambiguously.

[1]: Might as well make this a complete example:

<explain xmlns="http://www.postgresql.org/2009/explain&quot;&gt; +
<Query> +
<Plan> +
<Node-Type>Sort</Node-Type> +
<Startup-Cost>4449.30</Startup-Cost> +
<Total-Cost>4496.80</Total-Cost> +
<Plan-Rows>19000</Plan-Rows> +
<Plan-Width>268</Plan-Width> +
<Sort-Key> +
<Item>zip</Item> +
</Sort-Key> +
<Plans> +
<Plan> +
<Node-Type>Seq Scan</Node-Type> +
<Parent-Relationship>Outer</Parent-Relationship>+
<Relation-Name>customers</Relation-Name> +
<Alias>customers</Alias> +
<Startup-Cost>0.00</Startup-Cost> +
<Total-Cost>726.00</Total-Cost> +
<Plan-Rows>19000</Plan-Rows> +
<Plan-Width>268</Plan-Width> +
<Filter>(customerid &gt; 1000)</Filter> +
</Plan> +
</Plans> +
</Plan> +
</Query> +
</explain>

--
Greg Smith 2ndQuadrant US Baltimore, MD
PostgreSQL Training, Services and Support
greg@2ndQuadrant.com www.2ndQuadrant.us

* Greg Smith:

Florian Weimer wrote:

It has been claimed before that YAML is a superset of JSON, so why
can't the YAML folks use the existing JSON output instead?

Because JSON just crosses the line where it feels like there's so much
markup that people expect a tool is necessary to read it, which has
always been the issue with XML too--bad human readability.

But YAML is not human-readable. There are human-readable subsets of
it, but the general serializers do not produce them, and specific
serializers are difficult to get right (as we've seen).

EXPLAIN (FORMAT YAML) SELECT * FROM customers WHERE customerid>1000
ORDER BY zip;
QUERY PLAN
-------------------------------------
- Plan: +
Node Type: Sort +
Startup Cost: 4449.30 +
Total Cost: 4496.80 +
Plan Rows: 19000 +
Plan Width: 268 +
Sort Key: +
- zip +
Plans: +
- Node Type: Seq Scan +
Parent Relationship: Outer +
Relation Name: customers +
Alias: customers +
Startup Cost: 0.00 +
Total Cost: 726.00 +
Plan Rows: 19000 +
Plan Width: 268 +
Filter: (customerid > 1000)

What does your parser do with this (equivalent but shorter) YAML
output?

- Plan: !!map
&0 Node Type: Sort
&1 Startup Cost: 4449.30
&2 Total Cost: 4496.80
&3 Plan Rows: &5 19000
&4 Plan Width: &6 268
Sort Key: ["zip"]
Plans: !!seq
- *0: Seq Scan
Parent Relationship: Outer
Relation Name: &7 customers
Alias: *7
*1: 0.00
*2: 726.00
*3: *5
*4: *6
Filter: (customerid > 1000)

Looking at the spec, it's rather difficult to come up with a readable
subset which can parsed easily and is general in the sense that it can
express empty strings, strings with embedded newlines, and so on.
YAML's rules for dealing with whitespace are fairly complex, but are
probably needed to get a more compact notation than JSON.

--
Florian Weimer <fweimer@bfk.de>
BFK edv-consulting GmbH http://www.bfk.de/
Kriegsstraße 100 tel: +49-721-96201-1
D-76133 Karlsruhe fax: +49-721-96201-99

-----BEGIN PGP SIGNED MESSAGE-----
Hash: RIPEMD160

But YAML is not human-readable. There are human-readable subsets of
it, but the general serializers do not produce them, and specific
serializers are difficult to get right (as we've seen).

No, it *is* human readable. Indeed, that's one of the things that
differentiates it from JSON: readability is the main goal, whereas
JSON's goals are different. The readablity necessarily makes
the parsing rules more complex, but that's the implicit tradeoff.
(Did you miss the part where the other Greg is sending explain
plans via email?)

What does your parser do with this (equivalent but shorter)
YAML output?

- Plan: !!map
&0 Node Type: Sort
&1 Startup Cost: 4449.30
&2 Total Cost: 4496.80
&3 Plan Rows: &5 19000
&4 Plan Width: &6 268
Sort Key: ["zip"]
Plans: !!seq
- *0: Seq Scan
Parent Relationship: Outer
Relation Name: &7 customers
Alias: *7
*1: 0.00
*2: 726.00
*3: *5
*4: *6
Filter: (customerid > 1000)

But we're not using alias nodes (nor would we ever want to), so I'm not
sure what the point of your contrived example is. That's shorter, but
certainly not easier to read by human /or/ machine.

Looking at the spec, it's rather difficult to come up with a readable
subset which can parsed easily and is general in the sense that it can
express empty strings, strings with embedded newlines, and so on.
YAML's rules for dealing with whitespace are fairly complex, but are
probably needed to get a more compact notation than JSON.

I'll state that both embedded newlines and column names and values with
funny characters like '*' and '|' are rare events, and the great majority
of things you'll see in an explain plan are plain ol' ASCII, in which
YAML produces a very good representation. But you are right that we need
to make sure we are handling the whitespace correctly.

When I get some free time, I'll make a patch to implement as much of
the spec as we sanely can. As I said before, I don't think we need to
strive for putting everything we possibly can into "plain scalar"
objects, as we can cover 99% of the cases easy enough and fall back to
'when in doubt, quote' for the rest.

- --
Greg Sabino Mullane greg@turnstep.com
End Point Corporation http://www.endpoint.com/
PGP Key: 0x14964AC8 201006080931
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----

iEYEAREDAAYFAkwOR2gACgkQvJuQZxSWSshkVwCgzqunUkawnBRGwOV8msQPudN8
UmkAoM1wz+wFCEz34CMJ7VH+S7T3mc43
=8OjG
-----END PGP SIGNATURE-----

On Tue, Jun 8, 2010 at 9:37 AM, Greg Sabino Mullane <greg@turnstep.com> wrote:

When I get some free time, I'll make a patch to implement as much of
the spec as we sanely can.

Saying that you'll fix it but not on any particular timetable is
basically equivalent to saying that you're not willing to fix it at
all. We are trying to get a release out the door. I'm not trying to
be rude, but it's frustrating to me when people object to having their
code ripped out but also won't commit to getting it fixed in a timely
fashion.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On Mon, Jun 7, 2010 at 4:14 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:

Testing 9.0 beta, I found that EXPLAINing certain queries in YAML
format will produce invalid YAML, for example:

explain (format yaml) select * from foo where str_val = 'a: b';

The problem in this case is that a colon followed by whitespace is not
allowed in an unquoted plain YAML string because a parser would
interpret it as the start of a map.

So the current code in escape_yaml() is inadequate for producing valid
YAML. I think it would have to also consider at least the following
characters as special "-"  ":"  "["  "]"  "{"  "}"  ","  "\""  "'"
"|"  "*"  "&". Technically, it would also need to trap empty strings,
and strings with leading or trailing whitespace.

Making escape_yaml() completely bulletproof with this approach would
be quite difficult, and (IMO) not worth the effort, especially given
that an important requirement is that the output be machine readable,
and in my experience YAML parsers are often far from perfect.

I would therefore argue for simply calling escape_json() to produce
double quoted output for all string values, and only have numeric
values unquoted. This is not really any less human readable, and is
far more machine readable.

Patch attached.

I've committed a patch which I think will address this issue without
uglifying the output quite so much. Also, I didn't like the idea of
not applying escaping to both the keys and values, even though we
think we'll never have a key that requires escaping. With this
approach, that change isn't needed.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On Tue, Jun 8, 2010 at 10:47 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Jun 7, 2010 at 4:14 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:

Testing 9.0 beta, I found that EXPLAINing certain queries in YAML
format will produce invalid YAML, for example:

explain (format yaml) select * from foo where str_val = 'a: b';

The problem in this case is that a colon followed by whitespace is not
allowed in an unquoted plain YAML string because a parser would
interpret it as the start of a map.

So the current code in escape_yaml() is inadequate for producing valid
YAML. I think it would have to also consider at least the following
characters as special "-"  ":"  "["  "]"  "{"  "}"  ","  "\""  "'"
"|"  "*"  "&". Technically, it would also need to trap empty strings,
and strings with leading or trailing whitespace.

Making escape_yaml() completely bulletproof with this approach would
be quite difficult, and (IMO) not worth the effort, especially given
that an important requirement is that the output be machine readable,
and in my experience YAML parsers are often far from perfect.

I would therefore argue for simply calling escape_json() to produce
double quoted output for all string values, and only have numeric
values unquoted. This is not really any less human readable, and is
far more machine readable.

Patch attached.

I've committed a patch which I think will address this issue without
uglifying the output quite so much.  Also, I didn't like the idea of
not applying escaping to both the keys and values, even though we
think we'll never have a key that requires escaping.  With this
approach, that change isn't needed.

Er, I should also say, thanks for the report, and please test. I am
definitely not an expert on YAML.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On 9 June 2010 03:48, Robert Haas <robertmhaas@gmail.com> wrote:

Er, I should also say, thanks for the report, and please test.  I am
definitely not an expert on YAML.

I'm not an expert on YAML either, but I don't think this works (at
least it breaks against the online YAML parser here:
http://yaml-online-parser.appspot.com/). If the string starts with a
".", then it tries to treat it as a floating point number and baulks
if the rest of the string isn't a valid number.

Actually that gives me another argument for quoting *all* string
values: if a string value happens to be a valid number and we output
it unquoted, then parsers will treat is a number, forcing the user to
cast it to a string in their code. This is likely to lead to bugs in
user code, where things initially appear to work, then unexpectedly
start failing in pathalogical cases.

Personally, I'd also not try to escape keys at all. If in the future
we add a new key, then we'd be forced to choose between quoting it, or
more likely choosing a different key that doesn't require quoting,
which is what most people do in my (admittedly limited) experience.

In short, despite everything that's been said on this thread, I still
prefer my patch - but then I suppose I would say that :-)

Cheers,
Dean

On 9 June 2010 07:58, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:

I prefer my patch - but then I suppose I would say that :-)

Seriously though, I can think of a number of good arguments in favour
of the "quote all string values unconditionally" approach:

- It's the simplest, least obtrusive fix to our code.
- It's the least likely to have bugs in our code.
- It's the least likely to lead to bugs in client code.
- It gives clean, consistent output (IMO).
- It's the easiest to machine parse (marginally).
- Strings are always strings.
- It's still perfectly human readable.
- Re arguments about mailers chopping it up, it's easier to piece
together again if you know all long lines end with a double quote.

I love Florian's example, because it illustrates that there are all
sorts of YAML output that we *could* produce that would be technically
equivalent. However, I think this is a case of the simplest solution
being the best.

Regards,
Dean