DanielCarter-stack commented on PR #10360:
URL: https://github.com/apache/seatunnel/pull/10360#issuecomment-3764429339
<!-- code-pr-reviewer -->
Thanks for this enhancement! Here are some items to address before merge:
**BLOCKER**
* `docs/en/introduction/concepts/incompatible-changes.md:16-18` - This PR
introduces a breaking change (restricting datetime format to whitelist) but
doesn't document it in incompatible-changes.md. Users' existing
PARSEDATETIME/TO_DATE queries will fail at runtime after upgrade without any
migration guide.
**MAJOR**
*
`seatunnel-transforms-v2/src/main/java/org/apache/seatunnel/transform/sql/zeta/ZetaSQLType.java:433-436`
- Uses `expression.toString()` then manual regex `replaceAll("^'|'$", "")`
instead of JSQLParser's dedicated API like `StringValue.getNotExcapedValue()`.
This depends on implementation details and may break with future versions.
Consider using type-specific value extraction APIs.
*
`seatunnel-transforms-v2/src/main/java/org/apache/seatunnel/transform/sql/zeta/ZetaSQLType.java:433-449`
- Assumes format is a string literal. If format is a column expression,
`toString()` returns the column name instead of a format string, causing
validation failure with unclear error messages. Please add type checking and
provide clear error when format is not a literal.
**MINOR**
*
`seatunnel-transforms-v2/src/main/java/org/apache/seatunnel/transform/sql/zeta/ZetaSQLType.java:447-449`
vs `DateTimeFunction.java:653-655` - Inconsistent error messages: "Unknown
pattern letter %s for function: %s" vs "Unsupported datetime format: '%s'." The
former is inaccurate since entire patterns are rejected, not individual letters.
*
`seatunnel-transforms-v2/src/main/java/org/apache/seatunnel/transform/sql/zeta/functions/DateTimeFunction.java:638-641`
- Redundant null check before `fromPattern(format)`, since `fromPattern(null)`
returns `Optional.empty()` which is handled by `orElseThrow()`.
--
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]
