Skip to content

feat(workflow-operator): read a column named in a different case the way the engine does - #9035

Open
kz930 wants to merge 3 commits into
apache:mainfrom
kz930:feat/export-ignore-column-case
Open

kz930 wants to merge 3 commits into
apache:mainfrom
kz930:feat/export-ignore-column-case

Conversation

@kz930

@kz930 kz930 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

The engine looks a column up through Schema, which ignores case. So a setting that names Name reads the column name, and the workflow runs in Texera. The exported script indexed the pandas frame with the name as written and stopped with a KeyError.

StandaloneCodeGenerator gains columnAsStored, which resolves a setting's column name against the input schema and returns the table's spelling. A name the schema does not hold comes back unchanged, so the script still fails where the run would. Seven operators read their column through it: Projection, Filter, Keyword Search, Regular Expression, Substring Search, Dictionary Matcher and Unnest String. Projection still names each output column by its alias, which falls back to the name as the setting writes it. That matches ProjectionOpExec.

Type Casting is the exception. Its schema propagation and its executor both match the column by its exact name, so in the engine a unit whose column differs in case casts nothing and the column passes through unchanged. The export now skips that unit too, instead of raising a KeyError.

Operators that run as Python in the engine already compare names exactly on both sides, so they are left alone.

Any related issues, documentation, discussions?

Closes #9004. @carloea2 found the mismatch while reviewing #8918.

#8980 rewrites Filter's export and looks up the declared type by the column's name. Whichever of the two merges second should look that type up under the resolved name as well.

How was this PR tested?

Each of the eight operators' OpDescSpec gains a test with a setting in a different case from the table. It asserts that the generated code reads the table's spelling. For Type Casting, the test also checks that the engine's output schema leaves that column's type unchanged, and that the export skips the unit. All of WorkflowOperator/test passes. Merged with the other open export PRs, the verification suite still passes all 150 cases (3 ignored, as before).

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

Generated-by: Claude Code (Claude Opus 5.5)

🤖 Generated with Claude Code

…way the engine does

The engine looks a column up through Schema, which ignores case, so a setting
that names `Name` reads the column `name`. The exported script indexed the
frame with the name as written and raised a KeyError.

StandaloneCodeGenerator.columnAsStored resolves a name against the input
schema to the table's spelling. Projection, Filter, Keyword Search, Regular
Expression, Substring Search, Dictionary Matcher and Unnest String read their
column through it. Projection still names its output by the alias, as the
engine does.

Type Casting matches a column by its exact name in the engine, so a unit whose
column differs in case casts nothing there. The script now skips that unit too.

Closes apache#9004

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@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: @aglinxinyuan, @sshiv012
    You can notify them by mentioning @aglinxinyuan, @sshiv012 in a comment.

@kz930

kz930 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@carloea2 could you review this one when you have time? It fixes the column-case mismatch you found on #8918. The red CI is the repo-wide ampel-bootstrap action being blocked, not this change.

@carloea2 carloea2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

106 focused tests pass locally. An additional comparison against the native executor found the projection mismatch below.

Two units can read one column: `Name` as `first` and `NAME` as `second`
both resolve to the table's `Name`. A rename map keyed by that column kept
only the last alias, so the script returned `second, second` where the
engine returns `first, second`. The script now sets the output names in
order with `set_axis`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@carloea2 carloea2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alias finding is fixed. Retested my original three probes, including two spellings of one input column with distinct aliases and null values. All 95 focused tests across nine suites pass on fresh upstream main, and scalafmtCheckAll passes. CI still needs clearing; its current failures stop at the Ampel action allowlist.

@codecov-commenter

codecov-commenter commented Oct 26, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.95%. Comparing base (8fbd4f2) to head (17e4cb0).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...amber/operator/typecasting/TypeCastingOpDesc.scala 40.00% 0 Missing and 3 partials ⚠️
.../operator/dictionary/DictionaryMatcherOpDesc.scala 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #9035      +/-   ##
============================================
+ Coverage     91.81%   91.95%   +0.13%     
- Complexity     5069     5082      +13     
============================================
  Files          1271     1271              
  Lines         55208    55218      +10     
  Branches       6987     6989       +2     
============================================
+ Hits          50688    50774      +86     
+ Misses         2800     2716      -84     
- Partials       1720     1728       +8     
Flag Coverage Δ *Carryforward flag
access-control-service 77.44% <ø> (ø)
agent-service 99.16% <ø> (ø) Carriedforward from 8fbd4f2
amber 86.36% <83.33%> (+0.39%) ⬆️
computing-unit-managing-service 61.84% <ø> (ø)
config-service 87.50% <ø> (ø)
file-service 81.92% <ø> (ø)
frontend 96.50% <ø> (ø) Carriedforward from 8fbd4f2
notebook-migration-service 83.77% <ø> (ø)
pyamber 98.54% <ø> (ø) Carriedforward from 8fbd4f2
workflow-compiling-service 72.61% <ø> (ø)

*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

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

Compared against main 8fbd4f2 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 384 0.234 24,861/32,196/32,196 us 🟢 -15.2% / 🔴 +163.4%
🔴 bs=100 sw=10 sl=64 932 0.569 105,580/140,018/140,018 us 🔴 +8.2% / 🔴 +60.7%
⚪ bs=1000 sw=10 sl=64 1,113 0.679 890,560/1,060,263/1,060,263 us ⚪ within ±5% / 🔴 +29.6%
Baseline details

Latest main 8fbd4f2 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 384 tuples/sec 423 tuples/sec 1,019 tuples/sec -9.2% -62.3%
bs=10 sw=10 sl=64 MB/s 0.234 MB/s 0.258 MB/s 0.622 MB/s -9.3% -62.4%
bs=10 sw=10 sl=64 p50 24,861 us 23,932 us 10,144 us +3.9% +145.1%
bs=10 sw=10 sl=64 p95 32,196 us 37,974 us 12,225 us -15.2% +163.4%
bs=10 sw=10 sl=64 p99 32,196 us 37,974 us 15,527 us -15.2% +107.4%
bs=100 sw=10 sl=64 throughput 932 tuples/sec 955 tuples/sec 1,371 tuples/sec -2.4% -32.0%
bs=100 sw=10 sl=64 MB/s 0.569 MB/s 0.583 MB/s 0.837 MB/s -2.4% -32.0%
bs=100 sw=10 sl=64 p50 105,580 us 103,877 us 79,910 us +1.6% +32.1%
bs=100 sw=10 sl=64 p95 140,018 us 129,424 us 87,155 us +8.2% +60.7%
bs=100 sw=10 sl=64 p99 140,018 us 129,424 us 106,643 us +8.2% +31.3%
bs=1000 sw=10 sl=64 throughput 1,113 tuples/sec 1,129 tuples/sec 1,415 tuples/sec -1.4% -21.4%
bs=1000 sw=10 sl=64 MB/s 0.679 MB/s 0.689 MB/s 0.864 MB/s -1.5% -21.4%
bs=1000 sw=10 sl=64 p50 890,560 us 875,723 us 780,407 us +1.7% +14.1%
bs=1000 sw=10 sl=64 p95 1,060,263 us 1,011,232 us 817,814 us +4.8% +29.6%
bs=1000 sw=10 sl=64 p99 1,060,263 us 1,011,232 us 848,683 us +4.8% +24.9%
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,521.02,200,128000,384,0.234,24860.75,32195.78,32195.78
1,100,10,64,20,2146.74,2000,1280000,932,0.569,105579.58,140018.27,140018.27
2,1000,10,64,20,17974.57,20000,12800000,1113,0.679,890560.38,1060262.82,1060262.82

@carloea2 carloea2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retested on fresh upstream main 8b89d0c: all 96 focused tests across eight suites and scalafmtCheckAll pass. No new findings.

The local schema cannot compile the unrelated DAO UserWarehouseSpec, so that source alone was excluded for this run. No repository source or test assertions were changed. Required CI checks are green.

Command: sbt -batch 'set DAO / Test / sources := (DAO / Test / sources).value.filterNot(_.getName == "UserWarehouseSpec.scala")' 'WorkflowOperator/testOnly *ProjectionOpDescSpec *DictionaryMatcherOpDescSpec *KeywordSearchOpDescSpec *RegexOpDescSpec *SubstringSearchOpDescSpec *TypeCastingOpDescSpec *UnnestStringOpDescSpec *SpecializedFilterOpDescSpec -- -oDF' scalafmtCheckAll

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exported Python looks up a column with its exact case where the engine ignores case

3 participants