[CALCITE-7801] JSON_VALUE(..., RETURNING DOUBLE) throws ClassCastException when the JSON number is an integer

[wasabii]

JSON_VALUE never converted the value it pulled out of the document to the type in the RETURNING clause. Its return type is always nullable, so code generation asks EnumUtils.convert for Object to a boxed primitive, and that case falls through to a plain (Double) x. The cast only worked when the JSON value already happened to be that class:

JSON_VALUE('{"c":0}', '$.c' RETURNING DOUBLE)     -- Integer cannot be cast to Double
JSON_VALUE('{"c":0.5}', '$.c' RETURNING INTEGER)  -- Double cannot be cast to Integer

INTEGER, VARCHAR and DECIMAL happened to work; everything else threw. And because the exception escaped the runtime function, ON ERROR never saw it. JSON_QUERY has the same problem with RETURNING ... ARRAY, where the elements are never converted at all, so I fixed both.

The conversion has to happen inside the runtime function, since that is where the ON EMPTY/ON ERROR logic lives. It can't simply call CAST: that is generated code, chosen from the statically known source and target types, and here the source is only known at run time — the scalar may arrive as an Integer, Double, BigInteger, String or Boolean. None of that is specific to JSON, so I put it in SqlFunctions, next to the toInt(Object) family it delegates to:

SqlFunctions.cast(value, typeName, precision, scale, roundingMode)
SqlFunctions.castArray(value, elementType, precision, scale, roundingMode)

These dispatch on the target type and call the same methods cast's generated code would have: Primitive.integerCast for the exact numerics, Primitive.charToDecimalCast and friends for DECIMAL, truncate/truncateOrPad for character types, the DateTimeUtils parsers for datetimes. The type, its precision and scale, and the type system's rounding mode come down from the two implementors as constants. castArray converts a nested array at every level, so RETURNING DOUBLE ARRAY ARRAY works where it used to throw, and it checks the value against the declared type rather than following whatever shape the value happens to have.

Precision and scale were being dropped entirely, and now aren't:

JSON_VALUE('{"c":100}',      '$.c' RETURNING DECIMAL(5,2))  -- was 100,    now 100.00
JSON_VALUE('{"c":"abcdef"}', '$.c' RETURNING VARCHAR(3))    -- was abcdef, now abc
JSON_VALUE('{"c":"2020-01-01 10:20:30.987"}', '$.c' RETURNING TIMESTAMP(0))  -- was .987, now truncated

Datetimes parse from the JSON string, including both WITH LOCAL TIME ZONE types — no time zone needs supplying, because cast passes one only when converting a TIME or TIMESTAMP, which is a local reading of a clock, and a JSON datetime is always a string. A JSON number is not a datetime, so converting one is an error; Oracle and MySQL agree, PostgreSQL doesn't.

cast raises for any target type it doesn't handle, so a conversion failure can no longer escape ON ERROR. Cases that used to throw an uncatchable ClassCastException now go through it: RETURNING VARBINARY(n); RETURNING <type> ARRAY on JSON_VALUE, which cannot match anything as Calcite implements it, since JSON_VALUE returns a scalar; and a value whose shape does not fit an array RETURNING type, such as a JSON object, or an array nested more or less deeply than the type says.

RETURNING DECIMAL that yields NULL still doesn't work, here and on main alike, because linq4j's constant-hoisting lifts the conversion out of its null guard. That can be fixed separately.

[mihaibudiu]

These 3 (or even 4) fields seem to be a low-level representation of a RelDataType value.
It's true that this layer should not depend on types from Rel, but maybe an abstraction packaging them together could be useful.

[wasabii]

Yeah..... I'm going to ask for your advice on this one.

And what the breakdown would be.... I guess the bundle of new things "returningType", "precision", "scale", "roundingMode". Could be.... a CastItem or something?

[wasabii]

What about BasicSqlTypeRtti?

Maybe that's wrong actually, since it doesn't have precision or scale. And seems to combine CHAR and VARCHAR.

[wasabii]

Okay. I added a new type named CastSpec. I couldn't really find any existing patterns for naming, etc. So, we can rename this if ya'll don't like it. But the idea is there. It also contains the rounding mode. I'm not totally certain that belongs on CastSpec. It is something a cast needs to know, but it isn't variable per-cast. But, I guess, the semantics can be that CastSpec is what a cast needs, not what might appear in cast-like syntax.

[sonarqubecloud]

Quality Gate passed

Issues
15 New issues
0 Accepted issues

Measures
0 Security Hotspots
93.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[wasabii]

So, need advice here probably.... maybe....

I made VARIANT work. By passing in RTTI. Which the JSON functions do know, at least loosely. They know whether they extracted a String, Integer, etc out of the JSON. So they can safely pass this. But, it turns out, RETURNING ARRAY VARIANT should probably be legal syntax. And on this one I'm stymied. Since the array can be non homogeneous. Or homogeneous. And the existing RTTI types don't have anything for that. I could pass in a List. Or a single one. Or wrap either in another class........

This is a pretty narrow case I'm sure. But I'd like this as complete as possible I suppose. And it seems perfectly valid to want to take a JSON_QUERY over a hetereogenious array [1, "Hello", false, []], and CAST it to ARRAY VARIANT.

[wasabii]

@mihaibudiu I'm going back to the drawing board on this one. I was fundamentally wrong in what cast() needs to be a full cast function. It needs the source type information. Even if it's a runtime cast...it needs to know a specific value is a DATE, as opposed to just an Integer, for it to be a real cast() function. And this subtly matters matters a little bit with pure JSON.

So I need to come up with a way to pass the values into cast() with their type metadata. Without using RelDataType.

I'm thinking to wrap the entire input into cast() with a CastValue object or something, which includes it's type information.

Variant does already have much of this information in the same way. But it seems weird to require the user of cast() to wrap everything in a VariantValue. And that doesn't really deal with arrays. Which I need to.

Here's a scenario:

[1, 1.0, 1.2, "01"]

JSON_QUERY('foo' RETURNING VARCHAR ARRAY)

This should return ["1", "1", "1.2", "01"]

But also, "1", RETURNING TIMESTAMP, should not just assume the Integer that it receices is in fact a DATE. It should probably error.

1.0 is a NUMBER in JSON. And as a normal even number, it's natural SQL type would just be INTEGER. So it SHOULD lose the decimal place on a varchar conversion. "1.2" is a number, but truly is a fraction, so it should be considered a FLOAT/DOUBLE, and thus render it's fractional component. The last one is already a real string. So it just is what it is.

This means I need to have JSON_QUERY here pull the array from JSON, and associate natural type information with it. And then run THAT through CAST, which behaves normally. Which it needs that type information to do.

[mihaibudiu]

Please consider the fact that a type can be deeply nested, like a MAP<VARCHAR, MAP<INT INT ARRAY, ROW(INT ARRAY)>>
A solution should work for arbitrary types the user specifies.

RTTI is probably one of the right ingredients; I forgot, I added that object exactly for handling VARIANT.

[mihaibudiu]

BTW: CAST(variant AS TYPE) should handle all these cases already, and VARIANT is in some sense a slightly more general form of JSON, so hopefully you should be able to use a similar approach.

[wasabii]

@mihaibudiu So what I have found so far.... I have a strong urge to drop my work here, and maybe rebuild around the latest ANSI SQL JSON stuff. I have spent days untangling which parts of the JSON support in Calcite is a standard. Which part is a Calcite specific extension. And which part is a Calcite specific extension that was created to work around something that later became part of a standard. As far as I know. From the 2014 proposal.

For instance, there is a JSON_TABLE function. That overlaps with Calcite's usage of RETURNING ARRAY. Apparently RETURNING ARRAY and such isn't part of the standard. Or at least wasn't back then. I have no idea about 2023.

And doing this has made me want to spend money on the ISO standard.

Before I do this, if I were to go down such a path, would it even be acceptable to Calcite? It would potentially break stuff. What are the Calcite policies around this?

In case you're wondering why I am even working on this. I am building an Azure Cosmos DB adapter, with insane amounts of push down. In that model, I represent every document container as a table, with every document as a ROW, wiht a single column with it's JSON in it. So my entire rule set is about parsing rel expressions witH JSON_* functions in them, and mapping them to backend operations.