Normalize symbol names #21204
Normalize symbol names #21204
Conversation
martint
force-pushed
the
ir-symbol-names
branch
2 times, most recently
from
c14c341 to
2134474
Compare
core/trino-main/src/main/java/io/trino/sql/planner/SymbolAllocator.java
Outdated
Show resolved
Hide resolved
martint
force-pushed
the
ir-symbol-names
branch
from
2134474 to
b0213c2
Compare
There was a problem hiding this comment.
i still think Symbol::Symbol should do checks as well
otherwise ir.ExpressionFormatter won't be able to assume symbol names do not contain unwanted characters.
i.e. this is about class design. if Symbol class doesn't provide guarantees, ir.ExpressionFormatter should avoid relying on these guarantees.
core/trino-main/src/main/java/io/trino/sql/planner/SymbolAllocator.java
Outdated
Show resolved
Hide resolved
martint
force-pushed
the
ir-symbol-names
branch
from
b0213c2 to
a4d5109
Compare
Symbol doesn't care what characters are used. If the ExpressionFormatter has constraints, it can/should escape any characters it doesn't know how to render unambiguously. |
martint
force-pushed
the
ir-symbol-names
branch
4 times, most recently
from
9f1aee2 to
fc457b2
Compare
|
1 similar comment
|
The SymbolAllocator should not have to be aware of specific uses. It's up to the caller to decide on symbol name hint and type.
martint
force-pushed
the
ir-symbol-names
branch
from
fc457b2 to
22bfcbe
Compare
It complicates the interface and provides very little value. Callers can build a composite name on their own if they desire.
The connector was incorrectly relying on the engine-provided variable name to represent the underlying column name. Variables are synthetic names generated by the engine for the purpose of mapping column assignments in and out of the call to the connector's pushdown API, but they bear no relationship with the name of the underlying columns.
Replace any sequence of non-alphanumeric with underscore.
martint
force-pushed
the
ir-symbol-names
branch
from
22bfcbe to
3c595a0
Compare
Release notes
(x) This is not user-visible or is docs only, and no release notes are required.