github-actions[bot] commented on code in PR #61816:
URL: https://github.com/apache/doris/pull/61816#discussion_r3001915692
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java:
##########
@@ -415,25 +405,9 @@ public String getNonShadowName() {
}
public String getDisplayName() {
- if (defineExpr == null) {
- return name;
- } else {
- return MaterializedIndexMeta.normalizeName(
- defineExpr.accept(ExprToSqlVisitor.INSTANCE,
ToSqlParams.WITHOUT_TABLE));
- }
- }
-
- public String getNameWithoutPrefix(String prefix) {
- if (isNameWithPrefix(prefix)) {
- return name.substring(prefix.length());
- }
return name;
}
Review Comment:
**[Bug]** This simplification changes observable behavior for materialized
view columns.
The old implementation returned
`MaterializedIndexMeta.normalizeName(defineExpr.toSql())` when `defineExpr !=
null` (e.g., `to_bitmap(v1)` for MV columns). Now it always returns `name`
(e.g., `mv_bitmap_union_v1`).
This breaks at least:
1. **`IndexSchemaProcNode.java:89`** — `DESC table ALL` will show internal
MV column names instead of expression names
2. **`TableSchemaAction.java:87`** — REST API `/api/{db}/{table}/_schema`
returns internal names instead of expressions — this is a potential API
compatibility break for Spark/Flink connectors
3. **`IndexSchemaProcNodeTest.java:51-52`** — Test explicitly asserts
`getDisplayName() != getName()` for MV columns; both now return
`mv_bitmap_union_v1`, causing line 52's `assertFalse` to fail
This is not a safe refactor — it changes user-facing behavior.
##########
fe/fe-core/src/main/java/org/apache/doris/info/TableNameInfo.java:
##########
@@ -250,7 +250,7 @@ public String getTableAlias() {
@Override
public String toString() {
StringBuilder stringBuilder = new StringBuilder();
- if (ctl != null && !ctl.equals(InternalCatalog.INTERNAL_CATALOG_NAME))
{
+ if (ctl != null) {
stringBuilder.append(ctl).append(".");
Review Comment:
**[Behavior Change]** Removing the
`!ctl.equals(InternalCatalog.INTERNAL_CATALOG_NAME)` guard means `toString()`
now always includes the catalog name (e.g., `internal.db.table` instead of
`db.table`).
This affects:
- `ForeignKeyConstraint.toString()` (user-visible in `SHOW CONSTRAINTS`
output)
- `SlotRef.toString()` and `SlotRef.hashCode()` (the hash value changes)
- Error messages and log output throughout the system
The PR description claims "no behavior change" but this is user-visible.
Please update the PR description to accurately reflect this, and verify no
other regression tests are affected beyond `constraint.out`.
--
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]
