Jackie-Jiang opened a new pull request, #19138:
URL: https://github.com/apache/pinot/pull/19138
## Summary
Follow-up to #19029, which fixed `jsonPathString` for the non-JSON-native
scalars a record extractor can materialize. The numeric families have the same
class of bug.
`jsonPathLong` / `jsonPathDouble` (and their `Fast` / `FirstMatch` variants)
handled only `Number`, falling through to `Long.parseLong(value.toString())` /
`Double.parseDouble(value.toString())` for everything else. That throws — and
the surrounding `catch` silently returns the caller's default — for:
- **`Boolean`**, which is JSON-native and so reaches these functions from
plain JSON strings too: `JSONPATHLONG('{"a":true}', '$.a', -1)` returned `-1`,
while the query-time `jsonExtractScalar(..., 'LONG')` returns `1`.
- **`Timestamp` / `LocalDate` / `LocalTime`**, whose `toString` is a JDBC or
ISO-8601 string rather than a number. Like the `UUID` case in #19029, these
only arise when the function operates on an already-materialized record tree —
e.g. an ingestion transform reading an extractor-produced value — never from
parsing a JSON string.
### Fix
All entry points route the resolved value through shared `jsonValueToLong` /
`jsonValueToDouble` helpers, following the conversion conventions of
`PinotDataType`:
| Value | Result |
|---|---|
| `Number` | `longValue()` / `doubleValue()` (unchanged) |
| `Boolean` | `1` / `0`, matching `jsonExtractScalar` |
| `Timestamp` | `getTime()` epoch millis (`PinotDataType.TIMESTAMP`) |
| `LocalDate` | `toEpochDay()` (`PinotDataType.DATE`) |
| `LocalTime` | `toNanoOfDay() / 1_000_000` millis since midnight
(`PinotDataType.TIME`) |
| anything else | parsed from `toString` (unchanged), so `UUID` / `byte[]` /
containers still yield the default |
The three temporal forms stay integral in the `double` helper as well —
`PinotDataType.TIMESTAMP.toDouble` and `TIME.toDouble` both truncate
sub-millisecond nanos, so a value extracted into a `DOUBLE` column holds
exactly what the corresponding `TIMESTAMP` / `DATE` / `TIME` column would store.
The `Timestamp` mapping also keeps the family self-consistent:
`jsonPathString` already renders a `Timestamp` as epoch millis (via
`JsonUtils.objectToString`), so the string and numeric extractions of the same
value now agree.
### Compatibility note
This changes the observable output of the released `jsonPathLong*` /
`jsonPathDouble*` scalar functions wherever they previously fell back to the
default value. The `Boolean` case is the wider one — unlike #19029, it is
reachable from ordinary JSON-string input, so a mixed-version cluster can
briefly return both `1` and the caller's default for the same boolean leaf, and
previously-persisted derived-column values will differ from newly-ingested
ones. Every affected input previously produced a default rather than a
meaningful number, so no version gate is added.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
