Skip to content

fix(amber): stop JSONL scan reading null as text - #8740

Open
CaroFernando wants to merge 1 commit into
apache:mainfrom
CaroFernando:fix/jsonl-null
Open

CaroFernando wants to merge 1 commit into
apache:mainfrom
CaroFernando:fix/jsonl-null

Conversation

@CaroFernando

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

When a JSONL file has a JSON null in a row, the JSONL File Scan reads it as
the literal text "null". That happens because JSONToMap writes every value
node into the map using asText(), and for NullNode that returns "null". One
null in a column then breaks schema inference: it can't parse "null" as a
number, so the column falls back to STRING and the cell shows the word null.

This PR makes JSONToMap skip NullNode in both places it can show up, object
fields and array elements in flatten mode. A null now simply has no entry in
the map, which is exactly how a missing key already behaves. The JSONL scan
operator handles missing keys fine already: at runtime the cell becomes an
empty value, and during schema inference the other rows decide the type.

So for the example in the issue:

{"a":1} {"a":null} used to give column a as STRING with values "1" and "null"
it now gives INTEGER with values 1 and an empty cell

The two tests in JSONUtilsSpec that expected the string "null" were written
on purpose to pin the old behavior until a fix came along, so they are
updated here. I also added two cases: a row where every field is null gives
an empty map, and a null inside a flattened array skips only its own indexed
key.

Any related issues, documentation, discussions?

Closes #8674

How was this PR tested?

The new tests fail before the fix and pass after it.

sbt "WorkflowCore/testOnly org.apache.texera.amber.util.JSONUtilsSpec" (25/25)
sbt WorkflowCore/test (820/820)
sbt WorkflowOperator/test (2605 passed, 11 canceled which also fail on main)
sbt scalafmtAll

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude

JSONUtils.JSONToMap turned a JSON null into the literal string
"null" (NullNode.asText()), so one null widened the whole column
to STRING during schema inference and the cell held the text null.

Nulls now produce no map entry, so the JSONL scan operator treats
them as missing values: schema inference stays numeric and the
cell is empty.

Closes apache#8674
@Yicong-Huang Yicong-Huang added release/v1.3 back porting to release/v1.3 release/v1.2 back porting to release/v1.2 labels Sep 29, 2026
@github-actions
github-actions Bot requested review from mengw15 and xuang7 September 29, 2026 03:14
@github-actions

Copy link
Copy Markdown
Contributor

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. A release/* label nominates a backport target; the branch's release manager approving this PR is what sends the fix there. The required Backport Approvals check stays red until every label below is approved, so each manager either approves or removes their own label — which is why the labels left on a merged PR are exactly the branches it reached.

Release branch Analysis
✅ release/v1.3 Change detected on this branch — label added; this fix is queued to backport here. @mengw15 decides: approving sends the fix here, removing this label declines it. The merge waits on one or the other. Review requested.
✅ release/v1.2 Change detected on this branch — label added; this fix is queued to backport here. @xuang7 decides: approving sends the fix here, removing this label declines it. The merge waits on one or the other. Review requested.

Auto-label run.

@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @eugenegujing
    You can notify them by mentioning @eugenegujing in a comment.

@codecov-commenter

codecov-commenter commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.08%. Comparing base (acc6dd1) to head (3d8ffde).
⚠️ Report is 222 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8740      +/-   ##
============================================
- Coverage     92.59%   89.08%   -3.51%     
- Complexity     4962     6931    +1969     
============================================
  Files          1242     1358     +116     
  Lines         52883    66626   +13743     
  Branches       6546     8839    +2293     
============================================
+ Hits          48965    59357   +10392     
- Misses         2290     5230    +2940     
- Partials       1628     2039     +411     
Flag Coverage Δ *Carryforward flag
access-control-service 79.14% <ø> (+1.76%) ⬆️
agent-service 99.16% <ø> (ø) Carriedforward from acc6dd1
amber 83.08% <100.00%> (-4.99%) ⬇️
computing-unit-managing-service 63.75% <ø> (+3.34%) ⬆️
config-service 83.00% <ø> (-4.38%) ⬇️
file-service 80.99% <ø> (-0.55%) ⬇️
frontend 96.55% <ø> (ø) Carriedforward from acc6dd1
notebook-migration-service 87.57% <ø> (+3.84%) ⬆️
pyamber 98.56% <ø> (ø) Carriedforward from acc6dd1
workflow-compiling-service 76.82% <ø> (+2.72%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 2 better · 🔴 6 worse · ⚪ 7 noise (<±5%) · 0 without baseline

Compared against main acc6dd1 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🔴 bs=10 sw=10 sl=64 358 0.218 27,844/32,037/32,037 us 🔴 +7.2% / 🔴 +155.0%
🔴 bs=100 sw=10 sl=64 799 0.488 122,998/153,537/153,537 us 🔴 +7.9% / 🔴 +67.0%
🔴 bs=1000 sw=10 sl=64 943 0.576 1,059,170/1,226,225/1,226,225 us 🔴 +10.0% / 🔴 +37.9%
Baseline details

Latest main acc6dd1 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 358 tuples/sec 370 tuples/sec 929.71 tuples/sec -3.2% -61.5%
bs=10 sw=10 sl=64 MB/s 0.218 MB/s 0.226 MB/s 0.567 MB/s -3.5% -61.6%
bs=10 sw=10 sl=64 p50 27,844 us 25,966 us 10,919 us +7.2% +155.0%
bs=10 sw=10 sl=64 p95 32,037 us 33,971 us 13,231 us -5.7% +142.1%
bs=10 sw=10 sl=64 p99 32,037 us 33,971 us 16,777 us -5.7% +91.0%
bs=100 sw=10 sl=64 throughput 799 tuples/sec 849 tuples/sec 1,209 tuples/sec -5.9% -33.9%
bs=100 sw=10 sl=64 MB/s 0.488 MB/s 0.518 MB/s 0.738 MB/s -5.8% -33.9%
bs=100 sw=10 sl=64 p50 122,998 us 113,967 us 86,081 us +7.9% +42.9%
bs=100 sw=10 sl=64 p95 153,537 us 147,081 us 91,913 us +4.4% +67.0%
bs=100 sw=10 sl=64 p99 153,537 us 147,081 us 106,071 us +4.4% +44.8%
bs=1000 sw=10 sl=64 throughput 943 tuples/sec 967 tuples/sec 1,244 tuples/sec -2.5% -24.2%
bs=1000 sw=10 sl=64 MB/s 0.576 MB/s 0.59 MB/s 0.76 MB/s -2.4% -24.2%
bs=1000 sw=10 sl=64 p50 1,059,170 us 1,032,328 us 838,767 us +2.6% +26.3%
bs=1000 sw=10 sl=64 p95 1,226,225 us 1,114,653 us 889,385 us +10.0% +37.9%
bs=1000 sw=10 sl=64 p99 1,226,225 us 1,114,653 us 923,616 us +10.0% +32.8%
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,559.24,200,128000,358,0.218,27843.96,32037.45,32037.45
1,100,10,64,20,2503.94,2000,1280000,799,0.488,122997.64,153537.36,153537.36
2,1000,10,64,20,21208.88,20000,12800000,943,0.576,1059169.81,1226225.11,1226225.11

renovate-bot pushed a commit to renovate-bot/apache-_-texera that referenced this pull request Nov 5, 2026
…ython (apache#8704)

### What changes were proposed in this PR?

The JSON Lines and Arrow sources implement `StandaloneCodeGenerator`, so
an exported script starts from the same data the workflow did.

Arrow is read into pandas' nullable dtypes rather than its numpy ones.
Arrow says of every value whether it is there, and a numpy column has
nowhere to put that: a missing double and a stored NaN both land on NaN,
and a missing integer costs the column its integer type. The engine
reads the file's own answer, so a set operation keeps a NaN row a null
row does not match, a holed integer column is still made of integers,
and a long past 2^53 is not rounded on its way through a float. The
other sources state the types they need one at a time, their formats
having no way to say it once.

A file also states the width and the sign of each of its numbers, and
Texera has no column narrower than a double or a 32-bit integer. A
single-precision or 16-bit column was arriving null, the parse having no
case for the Float and Short those vectors hand back, and an unsigned
one arrived as the -1 its storage counts down to. Each now reads as the
Texera type that holds it, an unsigned 32-bit column as a long and an
unsigned 64-bit one refused for want of a wider one, and the script
normalizes the same widths so a later sum comes to the same number on
both sides. This is the rule the Parquet source already follows. The end
of an exported Arrow window is counted in Long, as the CSV readers count
theirs.

A timestamp is read as the engine reads one. The engine hands each field
to DateParserUtils, so a row states its own format, any year reads, an
offset is moved to the local zone, and the reading is cut to the
millisecond. pandas' date parsing does none of that. So JSONL reads the
column as text and converts it with the helper apache#8597 adds.

This PR also changes the engine. `ArrowUtils` handed a TIMESTAMP column
to a Python worker as an Arrow millisecond timestamp labelled UTC. A
Python operator saw an aware datetime cut to the millisecond, and a year
past 2262 overflowed. It is now a zoneless microsecond timestamp. That
is what Texera's own Python side declares for a TIMESTAMP
(`pa.timestamp("us")`), and what an exported script holds. Two things a
user can see change. A Python UDF now receives a naive datetime instead
of one in UTC. A result downloaded as an Arrow file now carries a
zoneless microsecond timestamp column instead of a UTC millisecond one.
A TIMESTAMP parameter a Python UDF declares is now parsed the same way,
as a naive wall clock with any offset moved to the machine's zone, so it
compares with a row's instead of raising.

### Any related issues, documentation, discussions?

Part of apache#8325, step 12b; that issue lists the set in order. It uses
apache#8341's shared source code and apache#8597's timestamp helpers.

Its JSON Lines export reads a JSON `null` as the text `"null"`, as the
engine does today (apache#8674). If apache#8740 merges first, that part changes to
read a null before this one merges.

Closes apache#8702, the task this change is the whole of.

It also finishes apache#8602, where a scan window of no rows left the type
inference nothing to read and JSONL declared a schema of no columns.
apache#8341 fixes the CSV half.

Closes apache#8602.

### How was this PR tested?

Each operator asserts the block it emits in its own spec, and the Arrow
spec runs the engine's own executor and the generated script over the
same file. The engine change is covered in both ArrowUtilsSpec files,
which include a round trip on either side of the nanosecond range, and
in the Python tests of the timestamp parser. Once the verification
lands, every operator is also run both ways on every configuration its
schema offers; this branch is cut from main and does not carry that
machinery, so those runs are not on this diff's CI.

Arrow is the one worth calling out. Its column of timestamps only
matches once apache#7672 is in: before that the engine read the file's numbers
through the JVM's zone while `pd.read_feather` read the wall clock the
file states, and the two differed by whatever the machine was set to.
That change has since landed.

### Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 5)

---------

Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
@mengw15 mengw15 removed the release/v1.3 back porting to release/v1.3 label Nov 9, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common fix release/v1.2 back porting to release/v1.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSONL File Scan reads a JSON null as the text "null"

4 participants