NIFI-15588 - Improve date, time, and timestamp type compatibility with ISO 8601 default formatters#10891
Open
pvillard31 wants to merge 2 commits intoapache:mainfrom
Open
NIFI-15588 - Improve date, time, and timestamp type compatibility with ISO 8601 default formatters#10891pvillard31 wants to merge 2 commits intoapache:mainfrom
pvillard31 wants to merge 2 commits intoapache:mainfrom
Conversation
…h ISO 8601 default formatters
Contributor
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for putting this together @pvillard31. On general review, I'm concerned about the potential impact of looping through multiple date and time formats when attempting conversion. I think an optimal approach would minimize the passes over the input string. I can take a closer look soon.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NIFI-15588 - Improve date, time, and timestamp type compatibility with ISO 8601 default formatters
Summary
isDateTypeCompatible,isTimeTypeCompatible,isTimestampTypeCompatible) and to the field converters (ObjectLocalDateFieldConverter,ObjectLocalTimeFieldConverter,ObjectLocalDateTimeFieldConverter), allowing ISO-formatted strings to be recognized and converted without requiring an explicit format pattern.allowDefaultFormatsparameter, enabling callers to opt out of the new ISO fallback and retain the legacy epoch-only behavior.ResolverStyle.STRICTto both the default ISO formatters and the explicit format path in compatibility checks, improving date validation strictness.Motivation
Prior to this change, the temporal compatibility methods (
isDateTypeCompatible,isTimeTypeCompatible,isTimestampTypeCompatible) only accepted numeric (epoch) strings when no explicit format pattern was provided. ISO 8601 strings such as"2024-01-15"or"13:45:00"were rejected, even though they are the most common standard date/time representations. Similarly, the field converters would throw aFieldConversionExceptionfor ISO strings when no pattern was supplied, falling through directly to the epoch-millis numeric parse.Additionally,
isTimeTypeCompatibleandisTimestampTypeCompatiblesimply delegated toisDateTypeCompatible, meaning all three types shared the same set of accepted formats. This made it impossible to use type-specific default formatters.Important notes
AbstractCSVRecordReaderUpdated
convertSimpleIfPossibleto use the three-argument overloads withallowDefaultFormatsset totrueonly when the corresponding format (dateFormat,timeFormat,timestampFormat) is non-null and non-blank. This preserves existing CSV reader behavior: without an explicit format configured, only epoch-like numeric strings are coerced.ExcelRecordReaderApplied the same guarding pattern as
AbstractCSVRecordReadertoconvertSimpleIfPossible, preventing the new ISO fallback from changing behavior for the Excel reader when no explicit temporal format is configured.Behavior Changes
isDateTypeCompatible("2024-01-15", null)false(epoch-only)true(ISO fallback)isTimeTypeCompatible("13:45:00", null)false(delegated to date check)true(ISO fallback)isTimestampTypeCompatible("2024-01-15T13:45:00Z", null)false(delegated to date check)true(ISO fallback)isDateTypeCompatible("2024-02-30", null)falsefalse(ISO STRICT rejects)isDateTypeCompatible("2024-01-15", null, false)false(epoch-only)FieldConversionException" "passed to compatibility checkIllegalArgumentExceptionCallers unaffected by the two-argument overload change:
isCompatibleDataType(value, dataType)— always passes non-null default formats ("yyyy-MM-dd","HH:mm:ss","yyyy-MM-dd HH:mm:ss"), so the null-format ISO fallback path is never reached.CEFRecordReader— always passes non-null format constants.AbstractCSVRecordReaderandExcelRecordReader— explicitly use the three-argument overload to opt out.Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation