[SQL] JSON_* and VARIANT_* functions for filtering and transformations#6686
[SQL] JSON_* and VARIANT_* functions for filtering and transformations#6686mihaibudiu wants to merge 1 commit into
Conversation
Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
mythical-fred
left a comment
There was a problem hiding this comment.
LGTM — clean design, thorough docs, solid test coverage. One question inline about the scope of the standard-function filter.
| List<SqlOperator> standard = new ArrayList<>(); | ||
| for (SqlOperator operator : SqlLibraryOperatorTableFactory.INSTANCE | ||
| .getOperatorTable(SqlLibrary.STANDARD).getOperatorList()) { | ||
| if (!operator.getName().startsWith("JSON_")) |
There was a problem hiding this comment.
This removes all standard Calcite JSON_* functions (JSON_VALUE, JSON_QUERY, JSON_EXISTS, JSON_ARRAY, JSON_OBJECT, JSON_DEPTH, JSON_LENGTH, JSON_PRETTY, JSON_REMOVE, etc.) — not just the ones that conflict with the new Feldera functions. The direct conflict is JSON_KEYS and JSON_OBJECT_KEYS, but the rest are collateral. Were any of the removed functions previously usable in Feldera SQL? If so, this is a breaking change worth documenting. If not (because Feldera uses VARIANT instead of string-based JSON), then this is fine — but a comment saying "Feldera replaces all standard JSON functions with VARIANT-based equivalents" would make the intent clearer for future readers.
| Variant::Timestamp(_) | Variant::String(_) | ||
| )); | ||
|
|
||
| /////////////// JSON_OBJECT_KEYS and JSON_KEYS |
There was a problem hiding this comment.
| /////////////// JSON_OBJECT_KEYS and JSON_KEYS | |
| // JSON_OBJECT_KEYS and JSON_KEYS |
| variant_deep_filter_(value?, predicate) | ||
| } | ||
|
|
||
| /////////////// VARIANT_MAP |
There was a problem hiding this comment.
| /////////////// VARIANT_MAP | |
| // VARIANT_MAP |
| variant_deep_map_(value?, mapper) | ||
| } | ||
|
|
||
| /////////////// VARIANT_MERGE |
There was a problem hiding this comment.
| /////////////// VARIANT_MERGE | |
| // VARIANT_MERGE |
| Variant::VariantNull | ||
| } | ||
|
|
||
| /////////////// JSON_EACH_* functions |
There was a problem hiding this comment.
you probably have to port this to flat_variant too now
mythical-fred
left a comment
There was a problem hiding this comment.
APPROVE. This is a substantial and carefully designed addition — the JSON_* extraction family, the two filter/map higher-order pairs (shallow + deep, with the intentional asymmetry that shallow VARIANT_MAP passes the raw Variant key while VARIANT_DEEP_MAP labels leaves by their dot-joined path), and a JSON Merge Patch style VARIANT_MERGE with the well-documented deviation that JSON null does not delete. Path quoting matches BigQuery's JSON_KEYS strict mode, so {"a.b": 1} and {"a": {"b": 1}} never collide.
The docs are unusually good — every rule is spelled out (non-string keys skipped, SQL NULL predicate drops like WHERE, arrays not concatenated on merge, date/time grammars accepted from strings, JSON_EACH_BIGINT never truncates), and the accompanying tests cover the corners I would have asked about (non-map inputs → empty result, top-level array element-wise filter, quoted path with embedded escapes, variant_merge shortcut on empty right).
CI is red on a self-hosted-runner infra failure ("Executing the custom container implementation failed. Please contact your self hosted runner administrator") — unrelated to the diff. A rerun should clear it.
A few small non-blocking notes:
is_integral_numeric_variantforSqlDecimal((sig, exp))uses10i128.checked_pow(exp as u32); forexp > 38the pow overflows and the fallback*sig == 0only accepts an exact zero. Fine in practice (a decimal with scale ≥ 39 is almost certainly fractional), just worth an inline comment.- The shallow
VARIANT_MAPincludes non-string keys (mapper receives them as raw Variant), whereasVARIANT_DEEP_MAPandVARIANT_DEEP_FILTERskip non-string keys and keep those fields untouched. This asymmetry is deliberate (paths must be strings) but the shallow docs don't explicitly call it out — one sentence in theVARIANT_MAPbullet list would preempt user surprise. VARIANT_MAPdocs say "does not recurse into nested objects"; strictly it also doesn't recurse into arrays. Minor wording tweak.is_identifier_keyis ASCII-only (BigQuery-compatible). Worth mentioning in the docs alongside the quoting rule so users with Unicode keys know they'll be quoted.
None of these should hold the PR.
Fixes #6685
Fixes #6424
Checklist
The documentation describes in detail all the functions added. The JSON_* functions are targeted for VARIANT values produced by JSON_PARSE (but work correctly for arbitrary variants). The VARIANT_* functions are designed for higher-order transformations on tree-shaped VARIANT values.