LiJie20190102 commented on code in PR #13167:
URL: https://github.com/apache/gravitino/pull/13167#discussion_r4025598388
##########
catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java:
##########
@@ -173,9 +173,16 @@ private String fromGravitinoArrayType(ListType listType) {
return elementTypeString + ARRAY_TOKEN;
}
- private ListType toGravitinoArrayType(String typeName) {
+ private ListType toGravitinoArrayType(String typeName, JdbcTypeBean
columnBean) {
String elementTypeName = typeName.substring(JDBC_ARRAY_PREFIX.length(),
typeName.length());
+ // PostgreSQL has no dedicated element metadata for array columns: the
driver reports the
+ // element's type information (length, scale, precision) on the array
column itself, encoded
+ // through atttypmod. Propagate it to the element bean, otherwise
length-driven element types
+ // such as BPCHAR unbox a null column size and table loading fails with an
NPE.
JdbcTypeBean bean = new JdbcTypeBean(elementTypeName);
+ bean.setColumnSize(columnBean.getColumnSize());
+ bean.setScale(columnBean.getScale());
+ bean.setDatetimePrecision(columnBean.getDatetimePrecision());
Review Comment:
`datetimePrecision` is always `null` for PostgreSQL array columns, so this
line is currently a no-op.
The call chain: `JdbcTableOperations.getBasicJdbcColumnInfo` sets the column
bean's `datetimePrecision` via `calculateDatetimePrecision(typeName, ...)`,
where `typeName` is the array type name reported by pgjdbc (e.g. `_timestamp`).
In `PostgreSqlTableOperations.calculateDatetimePrecision`, the switch matches
on `"TIME"`, `"TIMETZ"`, `"TIMESTAMP"`, `"TIMESTAMPTZ"` — but the array type
name is `"_TIMESTAMP"`, which never matches and falls through to `default:
return null`.
So for `timestamp(3)[]`, even though pgjdbc reports the element precision
`3` in `DECIMAL_DIGITS`, it never reaches the element bean — the column bean's
`datetimePrecision` is `null` before it gets propagated here, and the element
ends up as `TimestampType.withoutTimeZone()` (no precision) instead of
`withoutTimeZone(3)`.
This isn't a regression (the old code didn't propagate it either), but the
PR description and inline comment state that datetime precision is propagated,
which isn't the case in practice.
**Fix option** — strip the `_` prefix in
`PostgreSqlTableOperations.calculateDatetimePrecision` before the switch:
```java
String upperTypeName = typeName.toUpperCase();
if (upperTypeName.startsWith("_")) {
upperTypeName = upperTypeName.substring(1);
}
switch (upperTypeName) { ... }
```
Alternatively, if that's out of scope, drop this line and the corresponding
wording to avoid misleading future readers.
--
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]
