Bug #9906
Incorrect date parsing in FWD DATE(CHARACTER) implementation
100%
Related issues
History
#2 Updated by Vladimir Tsichevski over 1 year ago
- File TestParse.cls added
- Subject changed from DATE(CHARACTER) function errors to Incorrect date parsing in FWD @DATE(CHARACTER)@ implementation
- Status changed from New to WIP
The FWD implementation of the 4GL DATE(CHARACTER) function is parsing date strings incorrectly. Attached is a 4GL test class covering a comprehensive set of parsing scenarios. These tests pass in OpenEdge (OE) but fail in FWD, confirming issues in the FWD implementation.
The same parsing logic is likely used for the INPUT-VALUE attribute of date FILL-IN widgets in OE. I want to resolve this parsing issue first before #9846, as the solution will likely apply also to #9846.
Providing automated tests for #9846 is a separate challenge, and I currently lack a clear solution for it. Addressing this #9906 issue first could provide a fix for both issues.
#3 Updated by Vladimir Tsichevski over 1 year ago
- Subject changed from Incorrect date parsing in FWD @DATE(CHARACTER)@ implementation to Incorrect date parsing in FWD DATE(CHARACTER) implementation
#4 Updated by Vladimir Tsichevski over 1 year ago
Date Parsing Rule Set¶
Overview¶
This rule set defines how to interpret a text input representing a date into three components—year, month, and day—based on a specified order.
Input Elements¶
- Date String: A text input containing the date (e.g.,
"1-2-3","010223","today"). May include leading spaces, trailing non-date characters (e.g., letters), or consist only of separators/spaces. - Date Order: A three-letter code defining the arrangement of date components (case-sensitive):
- Valid codes:
"mdy"(month-day-year),"dmy"(day-month-year),"ymd"(year-month-day),"myd"(month-year-day),"dym"(day-year-month),"ydm"(year-day-month).
- Valid codes:
- Current Year: A number used as the year when the year component is omitted.
- Window Year Base: A reference year for adjusting two-digit years.
Output¶
- Complete Date: A set of three numbers:
[year, month, day], representing the date components. - Unknown Value: An "unknown" indicator if the input contains only separators (
".","/","-") or spaces. - Error: A specific error code for invalid inputs or unmet requirements (see Errors section).
Parsing Rules¶
- Check for Unknown Value:
- If the Date String contains only separators (
".","/","-") or spaces, return the "unknown" indicator.
- If the Date String contains only separators (
- Check for Today Value:
- If the Date String matches
"today"(case-insensitive, allowing leading spaces), return the current date’s year, month, and day.
- If the Date String matches
- Extract Date Portion:
- Identify the date portion by capturing all consecutive digits (0-9) and separators (
".","/","-") after any leading spaces, stopping at the first non-date character (e.g., letter, space, or other symbol). - Example:
" 1-2-3xyz"captures"1-2-3";"123456ab"captures"123456";"abc"captures nothing. - If no valid date portion (digits or separators) is found, raise error #85 (invalid format).
- Identify the date portion by capturing all consecutive digits (0-9) and separators (
- Identify Components:
- With Separators (if
".","/", or"-"is present in the date portion):- Split the date portion into components at each separator, collecting as many as possible.
- Each component is a sequence of digits, optionally starting with a hyphen (
"-") for negative numbers. - Stop parsing components at a non-digit (excluding separators).
- Example:
"1-2-3"yields"1","2","3";"-1--2--3"yields"-1","-2","-3";"1-2-3-4"yields"1","2","3","4".
- Without Separators:
- After discarding leading spaces, calculate the length of the entire string (including trailing non-date characters).
- Divide the date portion into up to three components, assuming two-digit lengths for each ([2,2,2] split).
- If the string length (post-leading spaces) is exactly eight characters, assign four digits to the year component based on Date Order ([4,2,2] split).
- Example:
"123456"(length=6) splits as"12","34","56"for"ymd"(year=12, month=34, day=56)." 12345678"(length=8) splits nibh as"1234","56","78"for"ymd"(year=1234, month=56, day=78)."12345"(length=5) splits as"12","34","5"."1234"(length=4) splits as"12","34".
- With Separators (if
- Adjust Component Count:
- Three Components: Proceed to interpretation.
- Two Components: Insert an empty year component at the position specified by Date Order (e.g.,
"1-2"with"mdy"becomes"1","2",""). - Fewer than Two or More than Three: Raise error #85 (invalid format).
- Assign Components:
- Map components to year, month, and day based on Date Order:
"mdy": first=month, second=day, third=year"dmy": first=day, second=month, third=year"ymd": first=year, second=month, third=day"myd": first=month, second=year, third=day"dym": first=day, second=year, third=month"ydm": first=year, second=day, third=month
- Map components to year, month, and day based on Date Order:
- Process Year:
- Empty Year: Use Current Year as the year.
- Non-Empty Year:
- Convert the year’s text to a number (see Number Conversion).
- If the text has one or two characters, apply year windowing (see separate document).
- If the text has three or more characters, use the number as-is.
- If the resulting year is zero, raise error #79 (invalid year).
- Process Month and Day:
- Convert month and day texts to numbers (see Number Conversion).
- If month or day text is empty, raise error #85 (invalid format).
- Ensure month is between 1 and 12, else raise error #80 (invalid month).
- Ensure day is valid for the given year and month (e.g., 29 for February in leap years), else raise error #81 (invalid day).
Number Conversion¶
- Transform a component’s text into a number.
- Treat a lone hyphen (
"-") as 0. - Ensure the number is between -32768 and 32767.
- If the text cannot be converted to a number (e.g., non-numeric) or the number is outside [-32768, 32767], raise error #85 (invalid format).
Errors¶
- 79: Year is zero.
- 80: Month is not between 1 and 12.
- 81: Day is not valid for the given year and month.
- 85: Invalid input, including:
- No valid date portion (e.g., no digits or separators).
- Fewer than two components.
- Empty month or day components.
- Text that cannot be converted to numbers.
- Numbers outside [-32768, 32767].
- 299: Invalid Date Order (not one of the six valid codes).
Examples¶
- Valid Date with Separators:
- Date String:
"01-02-23", Date Order:"mdy", Current Year: 2025, Window Year Base: 1950 - Output:
[2023, 1, 2](year=23 windowed to 2023).
- Date String:
- Missing Year:
- Date String:
"01-02", Date Order:"mdy", Current Year: 2025, Window Year Base: 1950 - Output:
[2025, 1, 2](uses Current Year).
- Date String:
- Separator-less (6-digit):
- Date String:
"010223", Date Order:"mdy", Current Year: 2025, Window Year Base: 1950 - Output:
[2023, 1, 2](split as"01","02","23").
- Date String:
- Separator-less (8-digit):
- Date String:
" 20010102", Date Order:"ymd", Current Year: 2025, Window Year Base: 1950 - Output:
[2001, 1, 2](length=8, split as"2001","01","02").
- Date String:
- Today Value:
- Date String:
" today ", Date Order:"mdy", Current Year: 2025, Window Year Base: 1950 - Output: Current date, e.g.,
[2025, 4, 15]on April 15, 2025.
- Date String:
- Unknown Value:
- Date String:
" / -.", Date Order:"mdy", Current Year: 2025, Window Year Base: 1950 - Output: Unknown indicator.
- Date String:
- Trailing Non-Date Characters:
- Date String:
"1-2-3xyz", Date Order:"mdy", Current Year: 2025, Window Year Base: 1950 - Output:
[2023, 1, 2](parses"1-2-3", ignores"xyz").
- Date String:
- Negative Numbers:
- Date String:
"-1--2--3", Date Order:"mdy", Current Year: 2025, Window Year Base: 1950 - Output:
[-3, -1, -2].
- Date String:
- Invalid Inputs:
- Date String:
"13-02-23", Date Order:"mdy"→ Error #80 (month=13 invalid). - Date String:
"01-32-23", Date Order:"mdy"→ Error #81 (day=32 invalid). - Date String:
"abc", Date Order:"mdy"→ Error #85 (no valid date portion).
- Date String:
#5 Updated by Vladimir Tsichevski over 1 year ago
- File TestParse.cls added
Attached is a simplified version of the ABLUnit test class for date string parsing, updated to no longer be affected by issue #9910.
#6 Updated by Vladimir Tsichevski over 1 year ago
In OE, the today string in date parsing accepts leading and trailing spaces (e.g., today ), but in FWD, it only accepts today without any spaces.
#7 Updated by Vladimir Tsichevski over 1 year ago
Similar problem with now, illustrated by the following additional tests:
@Test.
METHOD PUBLIC VOID testNow():
testParseFailure("now", 12003).
testParseFailure("NOW", 12003).
testParseFailure(" now", 12003).
testParseFailure(" now ", 12003).
END METHOD.
In OE, the now string in date parsing accepts any number of leading and trailing spaces (e.g., now ), but in FWD, it rejects spaces and incorrectly reports error 85 instead of the expected error 12003.
#8 Updated by Vladimir Tsichevski over 1 year ago
- % Done changed from 0 to 10
#9 Updated by Vladimir Tsichevski over 1 year ago
- Status changed from WIP to Review
- % Done changed from 10 to 100
Issue fixed in 9906a rev. 15862. Now all tests in TestParse.cls do pass. Please, review.
After review succeeds, I am going to sort all members of the date class to match the GCD requirements.
#10 Updated by Greg Shah over 1 year ago
- reviewer Greg Shah added
Code Review Task Branch 9906a Revisions 15860 through 15862
1. The rework of parseWorker() is so extensive that it is not possible to easily reason about the correctness of the new implementation. The method itself is much longer, so it is harder to consume/maintain. Things like error handling are greatly changed so it is no easy to know if it is correct or not. I appreciate that you have written many tests for this. Even so, this code will need to be tested across multiple applications to ensure we don't break something that is not represented in the tests.
2. In multiple cases you use throw new ErrorConditionException() directly instead of EM.recordOrThrowError(). That change is not OK.
3. Don't use the text "OpenEdge" in our code. That word is a trademark of PSC. Just use "4GL" instead.
4. Per our coding standards, please:
- Remove all use of
assert. - Don't add unnecessary
finalmodifiers to local variables or parameter definitions. - Don't add
@SuppressWarningsto the code. (IDEs)
#11 Updated by Greg Shah over 1 year ago
- reviewer deleted (
Hynek Cihlar)
#12 Updated by Vladimir Tsichevski over 1 year ago
- reviewer Hynek Cihlar added
Greg Shah wrote:
Code Review Task Branch 9906a Revisions 15860 through 15862
1. The rework of
parseWorker()is so extensive that it is not possible to easily reason about the correctness of the new implementation. The method itself is much longer, so it is harder to consume/maintain.
The implementation precisely adheres to the specification in #9906-4. Developing this specification required a week of experimentation, modeling, and writing unit tests. In contrast, rewriting the implementation took only a few hours.
I completely redesigned the parse function, as the original algorithm was nearly impossible to decipher, and flawed in its logic.
Things like error handling are greatly changed so it is no easy to know if it is correct or not.
2. In multiple cases you usethrow new ErrorConditionException()directly instead ofEM.recordOrThrowError(). That change is not OK.
Not exactly. I simplify error handling by throwing exceptions at various points in the method body (as exceptions are designed for this purpose) and then handling all of them in a single catch block using EM.recordOrThrowError(). This avoids the need to define incorrect return values and analyze method return values in multiple places.
I appreciate that you have written many tests for this. Even so, this code will need to be tested across multiple applications to ensure we don't break something that is not represented in the tests.
Agreed.
3. Don't use the text "OpenEdge" in our code. That word is a trademark of PSC. Just use "4GL" instead.
Agreed. Removed 3 occurrences of the word.
4. Per our coding standards, please:
- Remove all use of
assert.
Removed (they are here, because compiler cannot guarantee local variables are really initialized in this case.
I recommend introducing a Java enum to manage the six variants of date component order (e.g., MDY, DMY, YMD, etc.) instead of using strings like "mdy", "MDY", or "dMy" or byte arrays. Strings are prone to errors due to their unrestricted nature (case sensitivity, typos), and byte arrays are similarly problematic as they can vary in size and content, making it impossible for the compiler to verify that all possible cases are handled in the code. An enum ensures only valid values are used, and enables the compiler to check for exhaustive handling in constructs like switch statements.
- Don't add unnecessary
finalmodifiers to local variables or parameter definitions.
Done. I use the final modifier when analyzing others' code or writing my own to ensure variables and parameters are immutable. Once the code is verified to work correctly, I may remove final if needed. Ideally, I’d prefer a language design where variables and parameters are final by default, with explicit annotations (e.g., modifiable) for mutable ones, but Java doesn’t support this :-(
- Don't add
@SuppressWarningsto the code. (IDEs)
Removed.
Fixes in rev. 15863
#13 Updated by Greg Shah over 1 year ago
I simplify error handling by throwing exceptions at various points in the method body (as exceptions are designed for this purpose) and then handling all of them in a single catch block using EM.recordOrThrowError(). This avoids the need to define incorrect return values and analyze method return values in multiple places.
Throwing exceptions is expensive at runtime and this approach will introduce potential slowdown. We don't want to add the use of exceptions here. You can create helper methods that generate specific errors using recordOrThrowError() and call them from multiple places as needed.
#14 Updated by Vladimir Tsichevski over 1 year ago
Greg Shah wrote:
I simplify error handling by throwing exceptions at various points in the method body (as exceptions are designed for this purpose) and then handling all of them in a single catch block using EM.recordOrThrowError(). This avoids the need to define incorrect return values and analyze method return values in multiple places.
Throwing exceptions is expensive at runtime and this approach will introduce potential slowdown. We don't want to add the use of exceptions here. You can create helper methods that generate specific errors using
recordOrThrowError()and call them from multiple places as needed.
Fixed in rev. 15864.
#15 Updated by Greg Shah over 1 year ago
- Status changed from Review to Internal Test
Code Review Task Branch 9906a Revisions 15863 through 15864
I'm good with the changes.
#16 Updated by Vladimir Tsichevski over 1 year ago
Vladimir Tsichevski wrote:
After review succeeds, I am going to sort all members of the
dateclass to match the GCD requirements.
Done in rev. 15865.
#17 Updated by Vladimir Tsichevski about 1 year ago
- Related to Bug #9956: Client crashes in DATE function added
#18 Updated by Vladimir Tsichevski about 1 year ago
The same issues occur when setting the SCREEN-VALUE attribute of a date FILL-IN, because the SCREEN-VALUE parsing logic uses the same date parsing code as the DATE function.
Patch 9906a addresses these issues as well.
#19 Updated by Vladimir Tsichevski about 1 year ago
- Related to Bug #9963: Date literal issues added
#20 Updated by Vladimir Tsichevski about 1 year ago
Can this be merged? This blocks the #9846 customer issue.
#21 Updated by Greg Shah about 1 year ago
What testing has been done?
#22 Updated by Vladimir Tsichevski about 1 year ago
Greg Shah wrote:
What testing has been done?
The TestParse.cls has 600+ test cases.
#23 Updated by Greg Shah about 1 year ago
Vladimir Tsichevski wrote:
Greg Shah wrote:
What testing has been done?
The
TestParse.clshas 600+ test cases.
I understand. But as I stated in #9906-10:
Even so, this code will need to be tested across multiple applications to ensure we don't break something that is not represented in the tests.
Let's test ChUI regression with this and also let's test the application for #9846.
#24 Updated by Vladimir Tsichevski about 1 year ago
Greg Shah wrote:
Vladimir Tsichevski wrote:
Greg Shah wrote:
What testing has been done?
The
TestParse.clshas 600+ test cases.I understand. But as I stated in #9906-10:
Even so, this code will need to be tested across multiple applications to ensure we don't break something that is not represented in the tests.
Let's test ChUI regression with this and also let's test the application for #9846.
For ChUI I usually ask help from Hynek.
As for #9846, the 9906a change itself does not fix #9846, but the fix for #9846 will depend on correct date parsing.
#25 Updated by Hynek Cihlar about 1 year ago
Vladimir, there are build errors:
[ant:javac] /home/hc/gcd/p2j_repo/review/src/com/goldencode/p2j/util/datetime.java:134: error: cannot find symbol [ant:javac] import com.goldencode.p2j.util.date.ContextIndependentDate; [ant:javac] ^ [ant:javac] symbol: class ContextIndependentDate [ant:javac] location: class date [ant:javac] /home/hc/gcd/p2j_repo/review/src/com/goldencode/p2j/util/datetimetz.java:138: error: cannot find symbol [ant:javac] import com.goldencode.p2j.util.date.ContextIndependentDate;
#26 Updated by Vladimir Tsichevski about 1 year ago
Hynek Cihlar wrote:
Vladimir, there are build errors:
[...]
The ContextIndependentDate class was inadvertently removed during a manual code tree merge. Restored in rev 16020. Please, update and try again.
#27 Updated by Hynek Cihlar about 1 year ago
9906a passed ChUI regression tests.
#29 Updated by Hynek Cihlar 9 months ago
Vladimir Tsichevski wrote:
Rebased to latest trunk, fixed after rebase to make it compile. Revno is now 16235.
Can we merge this change now? Fixing #9846 depends on this.
Was all regression testing finished?
#30 Updated by Vladimir Tsichevski 9 months ago
Hynek Cihlar wrote:
Vladimir Tsichevski wrote:
Rebased to latest trunk, fixed after rebase to make it compile. Revno is now 16235.
Can we merge this change now? Fixing #9846 depends on this.
Was all regression testing finished?
I tested with our big customer application, and found that I need to restore one previously removed method and revert method access back to public for methods used in converted code.
After this everything worked OK with this application.
The new revision is 16237. No functional changes in this revision. So, I hope we can merge the change now.
#31 Updated by Hynek Cihlar 9 months ago
- Status changed from Internal Test to Merge Pending
Please merge 9906a to trunk.
#33 Updated by Hynek Cihlar 9 months ago
- Status changed from Merge Pending to Closed
#36 Updated by Sergey Ivanovskiy 9 months ago
Hynek, please review. Could 9906b rev. 16259. be merged into the trunk?
#38 Updated by Hynek Cihlar 9 months ago
- Status changed from Closed to Review
#39 Updated by Alexandru Lungu 9 months ago
There is no active 9906b branch to review.
I approved merge in #7143-1731 after reviewing in #7143. I wasn't aware of this task because I was not a watcher. The changes in 9906b redo the "date methods" from protected to public and are safe.