You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "suyashj1231 (via GitHub)" <gi...@apache.org> on 2026/09/19 01:56:15 UTC
[PR] fix(workflow-operator): honor the File Scan Encoding field [texera]
suyashj1231 opened a new pull request, #8605:
URL: https://github.com/apache/texera/pull/8605
### What changes were proposed in this PR?
File Scan offers an **Encoding** field, and picking anything other than UTF-8 changed nothing. A UTF-16 file came back decoded as UTF-8 rather than as its text.
`FileScanSourceOpDesc` re-declares the inherited `fileEncoding` as its own `encoding` property so the field can carry a hide annotation, and suppresses the inherited one with `@JsonIgnoreProperties(value = Array("limit", "offset", "fileEncoding"))`. That is the same pattern `TextSourceOpDesc` uses for `fileScanLimit` and `fileScanOffset`, and those two work.
Encoding did not, for two reasons that compounded. First, `encoding` was `private val`, so the executor could not read it even if it wanted to. Second, `FileScanSourceOpExec` instead read `desc.fileEncoding`, the inherited field that the annotation above strips during serialization, so it always came back as its `UTF_8` default.
The fix makes `encoding` a `var`, matching `ScanSourceOpDesc.fileEncoding` and `FileScanOpDesc.fileEncoding`, and reads it in the executor:
```
before: fileEncoding = desc.fileEncoding
after: fileEncoding = desc.encoding
```
`FileScanOpDesc` (the non-source variant) declares its own public `fileEncoding` with no `@JsonIgnoreProperties`, so it was never affected and is untouched here.
### Any related issues, documentation, discussions?
Closes #8596
### How was this PR tested?
`FileScanSourceOpDescSpec` already had a "with US_ASCII encoding" case, but it set the inherited `fileEncoding`, so it exercised the encoding path without being able to detect this bug: ASCII and UTF-8 agree on ASCII bytes, so it passed either way. It now sets `encoding`. The `before` block was pointed at `encoding` for the same reason.
Two cases were added:
| Case | Asserts |
| --- | --- |
| carry the Encoding field through serialization into the executor | the charset survives the `writeValueAsString` to `readValue` round trip that `getPhysicalOp` uses to reach the executor |
| decode a UTF-16 file with the charset the Encoding field names | a real UTF-16 temp file decodes to its lines, not to a BOM plus NUL-interleaved characters |
The second case is a genuine regression test. It was confirmed to fail on the old wiring before the fix was applied, with the executor line reverted to `desc.fileEncoding`:
```
[info] should decode a UTF-16 file with the charset the Encoding field names *** FAILED ***
[info] processedTuple.next().getField[Nothing]("line").equals("line1") was false
[info] (FileScanSourceOpDescSpec.scala:251)
[info] Tests: succeeded 13, failed 1
```
and to pass with the fix in place:
```
[info] Tests: succeeded 14, failed 0, canceled 0, ignored 0, pending 0
```
No regressions across the sibling scan operators:
```
sbt "WorkflowOperator/testOnly org.apache.texera.amber.operator.source.scan.*"
[info] Suites: completed 18, aborted 0
[info] Tests: succeeded 164, failed 0, canceled 0, ignored 0, pending 0
```
`sbt WorkflowOperator/scalafmtAll` and `sbt WorkflowOperator/scalafixAll` produce no further diff.
### Was this PR authored or co-authored using generative AI tooling?
Yes, partially. I (Suyash Jain) worked on this PR together with Claude Code as a pair-programming assistant. The fix was verified locally by confirming the new regression test fails before it and passes after, and by running the full scan-operator suite.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-operator): honor the File Scan Encoding field [texera]
Posted by "suyashj1231 (via GitHub)" <gi...@apache.org>.
suyashj1231 commented on PR #8605:
URL: https://github.com/apache/texera/pull/8605#issuecomment-5751363852
Changed to feat
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(workflow-operator): honor the File Scan Encoding field [texera]
Posted by "xuang7 (via GitHub)" <gi...@apache.org>.
xuang7 commented on PR #8605:
URL: https://github.com/apache/texera/pull/8605#issuecomment-5745264096
Removing the release/v1.2 label: the Encoding field never took effect, so this enables a capability rather than restoring previously working behavior. Ships with 1.3.0.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(workflow-operator): honor the File Scan Encoding field [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8605:
URL: https://github.com/apache/texera/pull/8605#issuecomment-5738460210
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 2 better · 🔴 7 worse · ⚪ 6 noise (<±5%) · 0 without baseline
> Compared against main `957b6c9` 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](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/35414185129)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🔴 | bs=10 sw=10 sl=64 | 406 | 0.248 | 23,574/29,213/29,213 us | 🟢 -10.2% / 🔴 +89.0% |
| 🔴 | bs=100 sw=10 sl=64 | 810 | 0.494 | 119,831/166,056/166,056 us | 🔴 +25.4% / 🔴 +56.8% |
| 🔴 | bs=1000 sw=10 sl=64 | 915 | 0.558 | 1,083,649/1,265,132/1,265,132 us | 🔴 +12.2% / 🔴 +24.2% |
<details><summary>Baseline details</summary>
Latest main `957b6c9` from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 406 tuples/sec | 443 tuples/sec | 796.29 tuples/sec | -8.4% | -49.0% |
| bs=10 sw=10 sl=64 | MB/s | 0.248 MB/s | 0.27 MB/s | 0.486 MB/s | -8.1% | -49.0% |
| bs=10 sw=10 sl=64 | p50 | 23,574 us | 22,127 us | 12,549 us | +6.5% | +87.9% |
| bs=10 sw=10 sl=64 | p95 | 29,213 us | 32,530 us | 15,458 us | -10.2% | +89.0% |
| bs=10 sw=10 sl=64 | p99 | 29,213 us | 32,530 us | 19,275 us | -10.2% | +51.6% |
| bs=100 sw=10 sl=64 | throughput | 810 tuples/sec | 842 tuples/sec | 1,027 tuples/sec | -3.8% | -21.1% |
| bs=100 sw=10 sl=64 | MB/s | 0.494 MB/s | 0.514 MB/s | 0.627 MB/s | -3.9% | -21.2% |
| bs=100 sw=10 sl=64 | p50 | 119,831 us | 118,044 us | 100,085 us | +1.5% | +19.7% |
| bs=100 sw=10 sl=64 | p95 | 166,056 us | 132,450 us | 105,895 us | +25.4% | +56.8% |
| bs=100 sw=10 sl=64 | p99 | 166,056 us | 132,450 us | 116,076 us | +25.4% | +43.1% |
| bs=1000 sw=10 sl=64 | throughput | 915 tuples/sec | 923 tuples/sec | 1,055 tuples/sec | -0.9% | -13.3% |
| bs=1000 sw=10 sl=64 | MB/s | 0.558 MB/s | 0.563 MB/s | 0.644 MB/s | -0.9% | -13.4% |
| bs=1000 sw=10 sl=64 | p50 | 1,083,649 us | 1,083,265 us | 979,292 us | +0.0% | +10.7% |
| bs=1000 sw=10 sl=64 | p95 | 1,265,132 us | 1,127,683 us | 1,018,835 us | +12.2% | +24.2% |
| bs=1000 sw=10 sl=64 | p99 | 1,265,132 us | 1,127,683 us | 1,039,692 us | +12.2% | +21.7% |
</details>
<details><summary>Raw CSV</summary>
```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,492.79,200,128000,406,0.248,23574.06,29212.88,29212.88
1,100,10,64,20,2470.10,2000,1280000,810,0.494,119831.24,166055.98,166055.98
2,1000,10,64,20,21860.53,20000,12800000,915,0.558,1083649.17,1265132.06,1265132.06
```
</details>
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(workflow-operator): honor the File Scan Encoding field [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8605:
URL: https://github.com/apache/texera/pull/8605#issuecomment-5738425133
<!-- backport-auto-label-report -->
### 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](https://github.com/apache/texera/actions/runs/35414174832)._
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(workflow-operator): honor the File Scan Encoding field [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8605:
URL: https://github.com/apache/texera/pull/8605#issuecomment-5738425004
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- **Contributors with relevant context:** `@aglinxinyuan`, `@eugenegujing`
You can notify them by mentioning `@aglinxinyuan`, `@eugenegujing` in a comment.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-operator): honor the File Scan Encoding field [texera]
Posted by "xuang7 (via GitHub)" <gi...@apache.org>.
xuang7 merged PR #8605:
URL: https://github.com/apache/texera/pull/8605
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(workflow-operator): honor the File Scan Encoding field [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #8605:
URL: https://github.com/apache/texera/pull/8605#issuecomment-5738441462
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/8605?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 92.79%. Comparing base ([`957b6c9`](https://app.codecov.io/gh/apache/texera/commit/957b6c965640c07ace034eb9edcae53ef974324a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`b55f917`](https://app.codecov.io/gh/apache/texera/commit/b55f9176db79e5102fb7c7d325432e4688b3a6c1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #8605 +/- ##
=========================================
Coverage 92.78% 92.79%
- Complexity 4902 4919 +17
=========================================
Files 1236 1236
Lines 52165 52197 +32
Branches 6411 6414 +3
=========================================
+ Hits 48402 48434 +32
Misses 2191 2191
Partials 1572 1572
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/8605/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/8605/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.78% <ø> (ø)` | | |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/8605/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `99.16% <ø> (ø)` | | Carriedforward from [957b6c9](https://app.codecov.io/gh/apache/texera/commit/957b6c965640c07ace034eb9edcae53ef974324a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/8605/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `88.63% <ø> (-0.01%)` | :arrow_down: | Carriedforward from [957b6c9](https://app.codecov.io/gh/apache/texera/commit/957b6c965640c07ace034eb9edcae53ef974324a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/8605/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.20% <ø> (ø)` | | Carriedforward from [957b6c9](https://app.codecov.io/gh/apache/texera/commit/957b6c965640c07ace034eb9edcae53ef974324a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/8605/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.51% <ø> (+3.13%)` | :arrow_up: | |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/8605/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.53% <ø> (ø)` | | Carriedforward from [957b6c9](https://app.codecov.io/gh/apache/texera/commit/957b6c965640c07ace034eb9edcae53ef974324a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/8605/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `96.59% <ø> (ø)` | | Carriedforward from [957b6c9](https://app.codecov.io/gh/apache/texera/commit/957b6c965640c07ace034eb9edcae53ef974324a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/8605/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.73% <ø> (ø)` | | Carriedforward from [957b6c9](https://app.codecov.io/gh/apache/texera/commit/957b6c965640c07ace034eb9edcae53ef974324a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/8605/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `98.48% <ø> (ø)` | | Carriedforward from [957b6c9](https://app.codecov.io/gh/apache/texera/commit/957b6c965640c07ace034eb9edcae53ef974324a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/8605/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `74.09% <ø> (ø)` | | Carriedforward from [957b6c9](https://app.codecov.io/gh/apache/texera/commit/957b6c965640c07ace034eb9edcae53ef974324a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/8605?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org