brucearctor opened a new issue, #14460:
URL: https://github.com/apache/gravitino/issues/14460
### Version
main (1.3.0-SNAPSHOT)
### Describe what's wrong
Following recent Doris and JDBC default-value fixes (#12740 / #12764 and
#13973 / #13975), several gaps remain in `catalog-jdbc-doris` default-value
conversion and version-check ordering:
1. **`DorisColumnDefaultValueConverter.fromGravitinoForAddColumn` bypasses
numeric literal validation:**
In #13975, `JdbcColumnDefaultValueConverter.fromGravitino` added numeric
literal validation (`Invalid numeric column default value: ...`) and whitespace
trimming for `Type.NumericType` defaults. However,
`DorisColumnDefaultValueConverter.fromGravitinoForAddColumn` intercepts
`Type.NumericType` before calling `super.fromGravitino(defaultValue)` and
directly calls `quoteDorisLiteral(String.valueOf(literal.value()), ...)`. As a
result, Doris `ADD COLUMN` accepts non-numeric default values on numeric
columns (e.g., `Literals.of("abc", Types.IntegerType.get())` or
`Literals.of("1); DROP TABLE t; --", Types.IntegerType.get())`) and emits them
into DDL instead of throwing `IllegalArgumentException`, and does not trim
padded numeric literals (e.g., `" 42 "`).
2. **`DorisColumnDefaultValueConverter.toGravitino` misses canonical Doris
`STRING` and `DATEV2` column types:**
`DorisTypeConverter` maps both `STRING` (`"string"`) and `TEXT`
(`"text"`) to `Types.StringType.get()`, and maps both `DATEV2` (`"datev2"`) and
`DATE` (`"date"`) to `Types.DateType.get()`. However,
`DorisColumnDefaultValueConverter.toGravitino` only switches on
`JdbcTypeConverter.TEXT` and `JdbcTypeConverter.DATE`, causing `"string"` and
`"datev2"` column defaults to fall through to `UnparsedExpression` (and
skipping `unescapeDorisLiteral` for `"string"` defaults, which in turn skips
`MODIFY COLUMN` string-literal escaping when the column is later altered).
3. **`DorisTableOperations` queries `getDorisVersion` before local
validation (and queries it twice in `generateCreateTableSql`):**
- In `generateCreateTableSql`, `validateAutoIncrementVersion(columns)`
and `useDoubleQuotedCreateTableStringDefaults(columns)` both run before
`validateIncrementCol(columns, indexes)` and
`validateDistribution(distribution, columns)`, and each independently calls
`getDorisVersion(...)` when a table has both an `AUTO_INCREMENT` column and a
string default containing quotes or backslashes.
- In `generateAlterTableSql`, `TableChange.UpdateColumnNullability` calls
`resolveModifyColumnVersion(...)` before `updateColumnNullabilityDefinition`
validates the nullability change (including rejecting Doris's unsupported
nullable-to-not-null transition), causing invalid `UpdateColumnNullability`
requests on columns with quote/backslash defaults to query `getDorisVersion`
before failing local argument validation.
### Error message and/or stacktrace
N/A (invalid numeric defaults are silently emitted into `ADD COLUMN` DDL;
`STRING`/`DATEV2` defaults are returned as `UnparsedExpression`; invalid
`CREATE TABLE` / `UpdateColumnNullability` requests fail with version lookup
errors when no JDBC `DataSource` is present instead of
`IllegalArgumentException`).
### How to reproduce
1. Call `new
DorisColumnDefaultValueConverter().fromGravitinoForAddColumn(Literals.of("abc",
Types.IntegerType.get()), false)` — returns `"\"abc\""` instead of throwing
`IllegalArgumentException: Invalid numeric column default value: abc`.
2. Call `new DorisColumnDefaultValueConverter().toGravitino(new
JdbcTypeConverter.JdbcTypeBean("string"), "a\\\"b", false, false)` or
`toGravitino(new JdbcTypeConverter.JdbcTypeBean("datev2"), "2024-01-02", false,
false)` — returns `UnparsedExpression` instead of
`Literals.stringLiteral("a\"b")` / `Literals.dateLiteral(LocalDate.of(2024, 1,
2))`.
3. Call `generateAlterTableSql` with
`TableChange.updateColumnNullability(new String[] {"col"}, false)` on a
nullable column whose default contains `"` or `\` without a live `DataSource` —
fails in `getDorisVersion` instead of failing fast with
`IllegalArgumentException: Doris does not support changing nullable column col
to not null`.
### Additional context
Follow-up to #12740 / #12764 and #13973 / #13975.
--
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]
