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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
