Enable Error Prone check: DefaultCharset

[ksobolew]

Description

This is focused on charsets, so it won't catch locale-related problems like String#toLowerCase with default locale. But it will catch String to byte[] (and vice versa) conversion with default charset, in addition to the IO stream related issues fixed in this commit.

The existing cases were fixed to explicitly use defaultCharset() where the data being read comes from (or is written to a file on) the local system, otherwise they are fixed to use UTF_8 - unless there are special considerations to use a different charset.

Is this change a fix, improvement, new feature, refactoring, or other?

Static analysis improvement.

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

All over the code (some fixes in places reported by Error Prone).

How would you describe this change to a non-technical end user or system administrator?

Some internal improvements that should make everything slightly more stable.

Related issues, pull requests, and links

https://errorprone.info/bugpattern/DefaultCharset

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

[findepi]

what does this read? where does modelData come from?

[ksobolew]

Hm. Looks like nobody calls this method?

[ksobolew]

Ah, it's called dynamically in io.trino.plugin.ml.ModelUtils#deserialize(io.airlift.slice.Slice). The byte[] data comes from theSlice. I have no idea what are the contents of the slice - some representation of the ML model, it appears, and it doesn't look like a text 🤔

[ksobolew]

Upon further investigation: the model data is some binary metadata followed by a textual representation of the model. The text is generated by writing to a temporary file using java.io.DataOutputStream#writeBytes(String), which... treats the string as "a sequence of bytes" (out.write((byte)s.charAt(i)) 😱). So it's actually either US_ASCII or ISO_8859_1.

[findepi]

ouch. use UTF-8 and add a comment that this may or may not be correct choice

[findepi]

what is out? sounds like default charset can be appropriate, but idk

[ksobolew]

this.out = new PrintStream(config.getEventLogFile());

IOW, this is a log file.

[ksobolew]

I'll change it to default then.

[ksobolew]

AC, thanks @findepi