cloud-fan commented on PR #56892:
URL: https://github.com/apache/spark/pull/56892#issuecomment-4849900908
Thanks for the careful trace, @MaxGekk -- I looked into the column-naming
path specifically, and I don't think this PR actually changes auto-generated
column names for `UTF8_BINARY`. Sharing what I found in case it saves the extra
test.
The key is that `usePrettyExpression` *does* special-case string `Literal`s,
in `catalyst/util/package.scala`:
```scala
case Literal(s: UTF8String, StringType) => PrettyAttribute(s.toString,
StringType)
```
This matches `StringType` by **value equality**, and `StringType.equals`
compares only `collationId`/`constraint`. Explicit `UTF8_BINARY` has
`collationId 0` (same as the default singleton), so this arm intercepts an
explicit-`UTF8_BINARY` string literal and rewrites it into a bare
`PrettyAttribute` **before `.sql` is ever called** -- the changed `Literal.sql`
never runs for it. That's exactly the same value-equality collapse the old
`Literal.sql` arm had; it lives independently here and this PR doesn't touch it.
For `UTF8_LCASE` (collationId != 0) line 94 doesn't match, the `Literal`
survives, and `.sql`/`Literal.sql` runs -- which is why `concat_ws(' ' collate
UTF8_LCASE, ...)` shows up. The `UTF8_BINARY` path is genuinely different.
I confirmed empirically by generating goldens for:
- `SELECT concat_ws(' ', utf8_binary, utf8_binary) FROM t1` -> column name
`concat_ws( , utf8_binary, utf8_binary):string` (plain `string`, no clause);
- a view under `DEFAULT COLLATION UTF8_BINARY` -> columns `string` (while
the `UTF8_LCASE` analog gets `collate UTF8_LCASE`).
And a grep over all `sql-tests` goldens finds no string-literal `'...'
collate UTF8_BINARY` in any column name.
So the user-facing-change note is accurate as written (column/alias naming
is unaffected), and a `collations-aliases` `utf8_binary` case would render
identically with or without this PR. Happy to add one anyway as a guard that
binary literals *don't* leak a collate clause into names, if you think it's
worth pinning -- but it's independent of this change. Let me know what you
prefer!
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
