sql: tuples not properly preserved during serialization

[knz]

Found while investigating #26621.
The expression ROW(x) gets serialized after normalization (format with FmtParsable) to (x) which has a completely different meaning. This causes the following error when distribution is enabled, but not with local execution:

> select row(1) in (row(2), row(1), row(v)) from kv;
pq: (1:::INT) IN ((2:::INT), (1:::INT), ROW("$0")): unsupported comparison operator: (1:::INT) IN ((2:::INT), (1:::INT), ROW("$0")): expected ROW("$0") to be of type int, found type tuple{int}

Also all the labeled tuples lose their labels during serialization. This likely breaks distribution of queries with labeled tuples.

[knz]

cc @RaduBerinde

[knz]

I tried to always pretty-print using the syntax row(...) but that breaks pretty-printing of a IN b because the syntax a IN ROWS(...) is not valid.

[RaduBerinde]

Could we introduce syntax to annotate as tuple? Something like (1):::tuple{int} or (1):::(int)

[knz]

Yes that might be a solution.

[knz]

some brainstorming:

1. the syntax (...):::(...) is interesting conceptually but it introduces a grammar problem: when parsing an expression of the form (x+y):::(...), the parser must determine by the time it encounters the 1st closing parenthesis that it must conclude parsing a TupleExpr and not ParenExpr, whereas only the token after that (the :::) indicates how to make this choice. Remember a LALR parser usually works with just 1 token lookahead. It's doable - it requires introducing a special "extra lookahead" token (we've done that in a few places - that's the _LA trick). But it's a bit complex.

2. maybe we should add the ROW keyword always for every 1-valued tuple, but only 1-valued tuples (not for 2+-valued tuples), to keep the pretty-printing compact in the general case. Then the only remaining case where there might be a problem is an expression like 123 IN (456) (1-valued list of values on the RHS of IN. Then we can handle IN with a special pretty-printing case for ComparisonExpr.

3. another approach is to support the syntax (x,) to indicate a 1-valued tuple. This avoids the lookahead problem and has precedents in other languages. If we recognize this syntax we can do so everywhere, including on the RHS of IN and then there is never a problem.

Then after that for tuples with labels we need to find a new syntax which doesn't use parentheses like the current one. We can look at that after we've solved the no-label case.

[knz]

regarding the 2nd point above. The problem of adding row just for 1-valued tuples is like this:

• (1,2) in ((1,2),(3,4)) remains without row because these are 2+-valued - all good
• row(1) in (row(2),row(3)) gets row and remains correct - all good
• 123 in (456) becomes 123 in row(456) which is incorrect - hence the problem.

So in that last case I was proposing to special case the pretty-print of ComparisonExpr to detect the combination tuple RHS + length 1 and have ComparisonExpr spell out the parentheses without ROW itself, instead of delegating to (Tuple).Format().

[knz]

although technically perhaps we could allow X IN Y for any Y with a tuple type, so 123 in row(456) would be recognized (as a crdb extension) too.

That might be another way to do solution 2.

[rjnn]

So I went down path (2), and one problem is that there are many special cases. It is not just ComparisonExpr, it is also PARTITION, VALUES, and other such representations that store their internal expressions as Tuples.

I would prefer allowing ROW in the syntax anywhere where we allow (foo). So Values, Partition, any clause that takes a parenthesized list of values, can be prefixed with ROW. This would be purely optional with respect to users, but would clearly disambiguate things for our internal parseable format.

[knz]

for PARTITION, VALUES and all other places the syntax is unambiguous (nothing else than a tuple can be placed there). Therefore it never fails to roundtrip even if you dont use row there.

The ambiguity really exists only for comparisons, IIRC.

[knz]

Ok I just changed my mind and I now recognize path (2) is non-viable, but for a different reason.

The problem is not the incompatibility of syntax between IN and other things. This could be indeed solved like you did in #28100.

The problem is that a DTuple does not contain the "row" attribute (like Tuple does), so after constant folding, as soon as there are DTuples in the expression tree, the pretty-print creates broken expressions.

As long as we try to solve this with the ROW keyword, things break like pushing down the pieces of a jigsaw puzzle:

• if you try to add ROW for 0- and 1-valued DTuples, that will make the grammar happy for the tuples themselves, but things like x in (1) will pretty-print to x in row(1:::int) and that becomes invalid.

• if you you never add row in DTuple and instead attempt to add it from other expressions (e.g. ComparisonExpr), then 0- and 1- valued DTuples that appear in other contexts start to break.

So really the entire ROW approach is bankrupt and cannot be saved. Therefore I have changed my mind and I think that indeed the path (3) is better. There is also a simple grammar solution, which I will post shortly.