feat: Db2 binary host variable support #2337

mm-broadcom · 2024-06-13T12:59:43Z

How Has This Been Tested?

VS Code Extension

Tested the new grammar locally and using the following small COBOL program with valid/invalid/negative values:

       Identification Division.
        Program-Id. 'TEST1'.
        Data Division.
         Working-Storage Section.
         01 VAR-BIN USAGE IS SQL TYPE IS BINARY(255).
         01 VAR-BIN-VARY USAGE IS SQL TYPE IS BINARY VARYING.
         01 VAR-VBIN USAGE IS SQL TYPE IS VARBINARY(32704).

         01 VAR-NOUSAGE-BIN SQL TYPE IS BINARY(255).
         01 VAR-BIN-NOUSAGE-VARY SQL TYPE IS BINARY VARYING.
         01 VAR-NOUSAGE-VBIN SQL TYPE IS VARBINARY(32704).
         
         01 VAR-INVALID-BIN USAGE IS SQL TYPE IS BINARY(256).
         01 VAR-INVALID-VBIN USAGE IS SQL TYPE IS VARBINARY(32705).

         01 VAR-NEGATIVE-BIN USAGE IS SQL TYPE IS BINARY(-1);
         01 VAR-NEGATIVE-VBIN USAGE IS SQL TYPE IS VARBINARY(-1234).
        PROCEDURE division.

Was given the intended results matching what is in IBM's documentation:

Unit Tests

Wrote four new unit tests checking the bounds of the size for both BINARY and VARBINARY types.

Checklist:

Each of my commits contains one meaningful change
I have performed rebase of my branch on top of the development
I have performed a self-review of my own code
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
I have checked my code and corrected any misspellings

Adds support for binary host variable declarations for DB2 within COBOL conforming to the IBM standard for their preprocessor.

Added bounds check per IBM's documentation for BINARY and VARBINARY types for their DB2 preprocessor.

Simplified the errors given to the end user. No need to tell them twice that negatives are not allowed.

…ost-Variable-Support

Add unit tests and changed the range check to an easier to read and understand function.

Remove unused message that was previously added as a commit to this PR.

Added a check for the offending token such that it properly denotes where the issue is rather than the one after it. Updated unit tests to reflect these changes.

mm-broadcom · 2024-06-14T11:56:18Z

Updated the underlining of the offending token to be where the user would expect.
Now looks like this:

ilidio-lopes · 2024-06-18T08:12:57Z

After your comment on #2338 I came to check your implementation and it looks like in your case it cannot be 01 for level.

ap891843 · 2024-06-18T09:36:55Z

server/engine/src/main/antlr4/org/eclipse/lsp/db2/parser/Db2SqlParser.g4

@@ -1783,6 +1794,8 @@ dbs_sql_identifier: NONNUMERICLITERAL | IDENTIFIER | FILENAME | FILENAME (DOT_FS
 dbs_comma_separator: (COMMASEPARATORDB2 | COMMACHAR);
 dbs_semicolon_end: SEMICOLON_FS | SEMICOLONSEPARATORSQL;

+dbs_integerliteral_expanded: MINUSCHAR? (INTEGERLITERAL|SINGLEDIGITLITERAL|SINGLEDIGIT_1);


I am not sure if -ve length should be allowed at grammar level. This would always be +ve integer as per doc.

That is correct, but if the user types a minus sign by accident we can now catch that. See the screenshot at the bottom of the initial comment/post in the PR as an example.

make sense 👍

ap891843 · 2024-06-18T09:41:25Z

server/engine/src/main/java/org/eclipse/lsp/cobol/implicitDialects/sql/Db2SqlVisitor.java

@@ -112,6 +112,33 @@ public List<Node> visitResult_set_locator_host_variable(
    return ImmutableList.of(variableDefinitionNode);
  }

+   @Override
+   public List<Node> visitBinary_host_variable(Db2SqlParser.Binary_host_variableContext ctx) {


Doc suggest this VARBINARY would generate source member as mentioned below

This would mean insert new variables.
We can refer Db2ImplicitVariablesGenerator for a similar use case.

Also, we would need to add test, for usage of generated variables

The IBM DB2 preprocessor converts the DB2 variable types to those supported by COBOL. These generated variables should already be taken care of.

The idea is that below program with db2 pre/co-processor would compile fine. But cobol-ls would start to throw error variable not defined.

Identification Division. Program-Id. 'TEST1'. Data Division. Working-Storage Section. LINKAGE SECTION. 01 VBIN-VAR USAGE IS SQL TYPE IS BINARY VARYING(10). PROCEDURE division. display VBIN-VAR-LEN. display VBIN-VAR-TEXT.

I'll take it from here. Let's merge this PR if everything else is fine.

ap891843 · 2024-06-18T09:45:14Z

server/engine/src/main/antlr4/org/eclipse/lsp/db2/parser/Db2SqlParser.g4


 result_set_locator_host_variable: dbs_level_01 entry_name  (USAGE IS?)? SQL TYPE IS RESULT_SET_LOCATOR VARYING;
+
+binary_host_variable: dbs_level_01 entry_name host_variable_usage binary_host_variable_type;
+binary_host_variable_type: BINARY LPARENCHAR binary_host_variable_binary_size RPARENCHAR | VARBINARY LPARENCHAR binary_host_variable_varbinary_size RPARENCHAR  | BINARY VARYING;


( BINARY | VARBINARY | BINARY VARYING) LPARENCHAR binary_host_variable_varbinary_size RPARENCHAR

looks like 01 VBIN-VAR USAGE IS SQL TYPE IS BINARY VARYING(10). is a valid syntax
Also, this introduces

7 1 VBIN-VAR. . . . . . . . . . . . . . . . . . . BLL=00001 000000000 DS 0CL12 Group 0 2 VBIN-VAR-LEN. . . . . . . . . . . . . . . . BLL=00001 000000000 DS 2C Binary 0 2 VBIN-VAR-TEXT . . . . . . . . . . . . . . . BLL=00001 000000002 DS 10C Display

Do we know the max length when using BINARY VARYING? I've kept the sizes separate in order to allow for bounds/range checking as BINARY and VARBINARY have different max sizes. (255 and 32704 respectively)

Well theoretically, these are semantics check and not a syntax issue. But I don't mind it either ways. As a general rule I like to consolidate sub rules to avoid look ahead burden. This is explained at improving-the-performance-of-an-antlr-parse ( check section - Consolidating sub rules)

Since this is a small rule, it doesn't matter a lot here.

For the semantics check, we can always do these in Visitor as the binary_host_variable parser rule context would have all the appropriate value within it.

...er/engine/src/main/java/org/eclipse/lsp/cobol/implicitDialects/sql/MessageServiceParser.java

…ects/sql/MessageServiceParser.java Co-authored-by: Aman Prashant <[email protected]>

mm-broadcom · 2024-06-18T12:03:10Z

After your comment on #2338 I came to check your implementation and it looks like in your case it cannot be 01 for level.

That is for the arrays, not this PR which is for the non-array variable(s). Here is IBM's documentation on this one: https://www.ibm.com/docs/en/db2-for-zos/13?topic=cobol-host-variables-in#db2z_hostvariablecobol__title__16

Add size requirement for BINARY VARYING.

mm-broadcom added 5 commits June 12, 2024 08:21

Add support for Binary Host Variables

39ac9ec

Adds support for binary host variable declarations for DB2 within COBOL conforming to the IBM standard for their preprocessor.

Added bounds check per IBM documentation

9a442d8

Added bounds check per IBM's documentation for BINARY and VARBINARY types for their DB2 preprocessor.

Removed duplicate error warnings

acbb561

Simplified the errors given to the end user. No need to tell them twice that negatives are not allowed.

Merge remote-tracking branch 'upstream/development' into DB2-Binary-H…

066bba1

…ost-Variable-Support

Add unit tests, better range checking

e6c4be5

Add unit tests and changed the range check to an easier to read and understand function.

mm-broadcom changed the title ~~Db2 binary host variable support~~ feat: Db2 binary host variable support Jun 13, 2024

mm-broadcom added 2 commits June 13, 2024 10:34

Remove unused message

e90f196

Remove unused message that was previously added as a commit to this PR.

Underlines proper offending token

536eff3

Added a check for the offending token such that it properly denotes where the issue is rather than the one after it. Updated unit tests to reflect these changes.

ap891843 reviewed Jun 18, 2024

View reviewed changes

...er/engine/src/main/java/org/eclipse/lsp/cobol/implicitDialects/sql/MessageServiceParser.java Outdated Show resolved Hide resolved

Update server/engine/src/main/java/org/eclipse/lsp/cobol/implicitDial…

d2303c8

…ects/sql/MessageServiceParser.java Co-authored-by: Aman Prashant <[email protected]>

Add size requirement for BINARY VARYING

43275ab

Add size requirement for BINARY VARYING.

mm-broadcom marked this pull request as draft June 19, 2024 18:57

ishche marked this pull request as ready for review June 24, 2024 08:25

ishche approved these changes Jun 24, 2024

View reviewed changes

ap891843 approved these changes Jun 24, 2024

View reviewed changes

ishche merged commit ad621d9 into eclipse-che4z:development Jun 24, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Db2 binary host variable support #2337

feat: Db2 binary host variable support #2337

mm-broadcom commented Jun 13, 2024 •

edited

Loading

mm-broadcom commented Jun 14, 2024

ilidio-lopes commented Jun 18, 2024 •

edited

Loading

ap891843 Jun 18, 2024

mm-broadcom Jun 18, 2024

ap891843 Jun 18, 2024

ap891843 Jun 18, 2024 •

edited

Loading

ap891843 Jun 18, 2024

mm-broadcom Jun 18, 2024

ap891843 Jun 19, 2024

ishche Jun 24, 2024

ap891843 Jun 18, 2024

ap891843 Jun 18, 2024

mm-broadcom Jun 18, 2024

ap891843 Jun 18, 2024

mm-broadcom commented Jun 18, 2024

feat: Db2 binary host variable support #2337

feat: Db2 binary host variable support #2337

Conversation

mm-broadcom commented Jun 13, 2024 • edited Loading

How Has This Been Tested?

VS Code Extension

Unit Tests

Checklist:

mm-broadcom commented Jun 14, 2024

ilidio-lopes commented Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ap891843 Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mm-broadcom commented Jun 18, 2024

mm-broadcom commented Jun 13, 2024 •

edited

Loading

ilidio-lopes commented Jun 18, 2024 •

edited

Loading

ap891843 Jun 18, 2024 •

edited

Loading