Project

General

Profile

Bug #9906

Incorrect date parsing in FWD DATE(CHARACTER) implementation

Added by Vladimir Tsichevski over 1 year ago. Updated 9 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Vladimir Tsichevski
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
15497
version_resolved:
production:
No
env_name:
topics:

TestParse.cls - Parse string to date tests (30.6 KB) Vladimir Tsichevski, 04/15/2025 11:11 AM

TestParse.cls (30.6 KB) Vladimir Tsichevski, 04/16/2025 11:29 AM


Related issues

Related to Base Language - Bug #9956: Client crashes in DATE function Closed
Related to Base Language - Bug #9963: Date literal issues New

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

  1. 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.
  2. 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).
  3. Current Year: A number used as the year when the year component is omitted.
  4. 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

  1. Check for Unknown Value:
    • If the Date String contains only separators (".", "/", "-") or spaces, return the "unknown" indicator.
  2. Check for Today Value:
    • If the Date String matches "today" (case-insensitive, allowing leading spaces), return the current date’s year, month, and day.
  3. 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).
  4. 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".
  5. 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).
  6. 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
  7. 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).
  8. 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

  1. 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).
  2. Missing Year:
    • Date String: "01-02", Date Order: "mdy", Current Year: 2025, Window Year Base: 1950
    • Output: [2025, 1, 2] (uses Current Year).
  3. 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").
  4. 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").
  5. 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.
  6. Unknown Value:
    • Date String: " / -.", Date Order: "mdy", Current Year: 2025, Window Year Base: 1950
    • Output: Unknown indicator.
  7. 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").
  8. Negative Numbers:
    • Date String: "-1--2--3", Date Order: "mdy", Current Year: 2025, Window Year Base: 1950
    • Output: [-3, -1, -2].
  9. 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).

#5 Updated by Vladimir Tsichevski over 1 year ago

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

#9906-6 and #9906-67 fixed in 9906a rev. 15861.

#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 final modifiers to local variables or parameter definitions.
  • Don't add @SuppressWarnings to 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 use throw new ErrorConditionException() directly instead of EM.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 final modifiers 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 @SuppressWarnings to 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 date class 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.cls has 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.cls has 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.

#28 Updated by Vladimir Tsichevski 9 months ago

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.

#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.

#32 Updated by Vladimir Tsichevski 9 months ago

Hynek Cihlar wrote:

Please merge 9906a to trunk.

Merged as rev. 16241

#33 Updated by Hynek Cihlar 9 months ago

  • Status changed from Merge Pending to Closed

#34 Updated by Vladimir Tsichevski 9 months ago

This change introduced a regression due to overly restrictive access to date methods (see #7143-1728).
I created 9906b to fix this.

#35 Updated by Vladimir Tsichevski 9 months ago

Vladimir Tsichevski wrote:

This change introduced a regression due to overly restrictive access to date methods (see #7143-1728).
I created 9906b to fix this.

Fixed in 9906b rev. 16259.

#36 Updated by Sergey Ivanovskiy 9 months ago

Hynek, please review. Could 9906b rev. 16259. be merged into the trunk?

#37 Updated by Greg Shah 9 months ago

There is no active 9906b branch to review.

#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.

#40 Updated by Greg Shah 9 months ago

Vladimir: Please post the commit message here and we can close the task.

#41 Updated by Greg Shah 9 months ago

  • Status changed from Review to Merge Pending

#42 Updated by Vladimir Tsichevski 9 months ago

Greg Shah wrote:

Vladimir: Please post the commit message here and we can close the task.

The branch 9906b was merged to trunk 27.20.2025 as rev. 16259 and archived.

#43 Updated by Greg Shah 9 months ago

  • Status changed from Merge Pending to Closed

Also available in: Atom PDF