Looks good. Thank you, team!
Hello, The current binary requires JDK 11. https://repo1.maven.org/maven2/org/hsqldb/hsqldb/2.6.0/ It would be difficult to add JDK-8 version to 2.6.0, so please consider it when you release future versions. Thanks in advance! Iwao
It looks good to me. :) If I noticed anything, I'll open a new ticket or post message to the mailing list. Thank you again for helping me with the request!
I squashed all commits, then added an extra commit that adds the quick check to ByteDataType#typeCast(). Regarding the extra commit, it seemed to be better to do it separately rather than as a refactoring as there was no comparable check before, I also didn't apply the formatter because it wasn't applied previously.
F224: Add new syntax to specify relative date, time and timestamp in dataset
Added a quick check to avoid unnecessary regex operation
And refactoring to DataType is preferred to centralize for all, agreed? Yes. :) I have updated the branch. Please let me know if you want me to squash some commits after review.
F224: Replaced magic numbers with named constants.
F224: More helpful error message on pattern mismatch. Tidying up tests.
F224: Refactoring. Extracted the common quick check for the extended syntax to the parent class.
Thank you for the thorough review! > 1 BLOB has similar extended syntax (i.e. [TEXT], [FILE], etc.). So, if you are comfortable with making a rule like "extended data type syntax always starts with '['", it would make sense to extract this check to a method (e.g. isExtendedSyntax(String)) in DataType. It could be useful even in the existing ByteDataType#typeCast to save the relatively expensive regex operation. If, on the other hand, you consider this check to be specific to this relative date/time...
Thank you for the thorough review! > 1 BLOB has similar extended syntax (i.e. [TEXT], [FILE], etc.). So, if you are comfortable with making a rule like "extended data type syntax always starts with '['", it would make sense to extract this check to a method (e.g. isExtendedSyntax(String)) in DataType. It could be useful even in the existing ByteDataType#typeCast to save the relatively expensive regex operation. And if you consider this check to be specific to this relative date/time enhancement,...
I rebased the branch onto master and squashed all commits (tests are passing). [feature-requests:#224] is the new ticket. I'm not so confident about the commit message and changes.xml, though. Please let me know if there's any correction needed to be made!
F224: Add new syntax to specify relative date, time and timestamp in dataset.
Add new syntax to specify relative date, time and timestamp in dataset.
Makes sense. :) I took the approach you mentioned in 4. DataType.RELATIVE_DATE_TIME_PARSER is the public shared instance of the parser. To allow users to setup ReplacementDataSet, getClock() and setClock() is added to the parser (I am not so sure about the necessity of the setter, but if you think it's worth making the class mutable, I have no objection). In case the class needs to be thread-safe, I would probably just make the setter/getter synchronized. I believe the point 1, 2 and 3 are resolved...
Diffs should be applied in the supplied order in the test. Otherwise, it could fail depending on the execution date.
DataType now holds a public static instance of RelativeDateTimeParser. Users can get/set the internal fixed Clock via this instance.
Updated documentation.
I've committed the changes. An instance of RelativeDateTimeParser is cached in each data type. For consistency, the same fixed Clock instance is used in all calculations. The Clock is passed via constructors mainly for better testability. If modifying constructors is not appropriate, please let me know and I'll make adjustments. I'll update the doc (datatypes.xml) maybe tomorrow!
Alternative approach : injecting a fixed Clock via data type constructor.
Correct test failure message.
Converted RelativeDateTimeParser#parse() to a static method to reduce object creation. Also 'now' now returns the exact same value during a test run.
Thank you for reviewing this, Jeff! I like this approach better because it is simpler and does not require extra preparation. The other approach may be more flexible, but that level of flexibility is rarely needed, I would think. I also agree with the points you have raised and plan to add new commits within a day or two.
Add relative value support to Date/Time/Timestamp data types
Add relative date/time syntax to Date, Time and Timestamp data types.
Hi again, However, a couple of benefits of the current approach are: I understand the benefits and am not denying it, but I still believe that there should be a way to skip the redundancy. In my case, most dates/timestamps have no symbolic meaning and are used only in one place, so there is little benefit in declaring/referencing tokens. He also wrote "we should also rewrite the internals of the old ReplacementDataSet to use this new functionality." Is this your proposal for the next step? Kyle's...
Thank you for reviewing this, Jeff ! It is possible, for sure, but it is quite cumbersome if one needs dozens of those dates/timestamps. In [#138] , mattias g wrote that this feature "avoids redundant code and improves reusability". In [#152] , John Hurst wrote "The current ReplacementDataSet is too limited. It's too hardcoded around strings and maps of replacements." And I completely agree with them. Wouldn't you? Many developers seemed to expect the dynamic approach as well. http://dbunit.996259.n3.nabble.com/Can-you-somehow-use-variable-date-values-within-dataset-XML-td373.html...
Allow users to customize ReplacementDataSet behavior.
Allow users to customize ReplacementDataSet behavior.
Allow users to customize ReplacementDataSet behavior.
Thank you, Jeff! I have verified that the latest snapshot fixed the issue and sent...
Added a test case for the latter part of issue ...
Hi Jeff, I have the same problem as Jörg. And the second patch from Weifeng (dbunit_tinyint2.patch)...