Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix db2 #330

Open
wants to merge 6 commits into
base: Developer
Choose a base branch
from
Open

Fix db2 #330

wants to merge 6 commits into from

Conversation

samdion1994
Copy link
Collaborator

Fix the expands db2 copybooks.

@@ -66,7 +66,7 @@ void commonSetup() throws IOException {
}
}

@Test
/* @Test
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samdion1994
Would you please elaborate on why you need to remove this test ?
If it is no longer passing, then the test should be fixed, instead of removed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see why the correct location for these lines is not in the writeToSource function.
I am right in assuming that the new location, in the processingBeforeEcho... function, will place the lines just before the procedure division?
What would happen, if there is a linkage section, located before the procedure division?
I would like a test, that verifies the functionality of adding the fileSectionStatements.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You were right, I had to add stuff to handle le linkage section. I wanted to know what you want to do with the stuff in the linkage? If there isn't integrated testing and no subcall we could just comment it?

@Rune-Christensen
Copy link
Collaborator

Hi @samdion1994

Thank you for the tests, it helped tremendously in understanding what is happening.

With that understanding now in place, I cannot approve the pull request, because the expansion of the copybook is no longer happening "in place".
Meaning, we cannot move the working storage elements, to other locations in working storage.
Consider this example:

EXEC SQL INCLUDE TEXEM  END-EXEC.
01  TEXEM2 REDEFINES TEXEM.
   10 FULL-NAME PIC X(20).
   10 FILLER PIC X(26).

The current changes would move TEXEM below the redefine, meaning the compile would fail.

Looking at the code again, I think the insertion of code is in the correct location, the issue is, that .getFileSectionStatements() is a bad name. Either the function is misused in this context, and there ought to be another function managing the expansion of copybooks, or the function should have another name, making it useable in more contexts.

Cheers,
Rune

Added sqlCopyBookStatement to better represent that what is happening.

When reading SQL statement which are copybook. They are then expanded and stored within the sqlCopyBookStatement variable.
@Rune-Christensen
Copy link
Collaborator

Hi @samdion1994
Thank you, the new function is a much better encapsulation of what is happening in the code.

But, the changes still moves the expanded copybook to a new location, that will have to be fixed before approval.
Thank you

Cheers

@Rune-Christensen
Copy link
Collaborator

Hi @samdion1994
Any progress on reverting the changes, that causes the expansion of SQLCA/TEXEM to move from where it was?

Regards
Rune

@samdion1994
Copy link
Collaborator Author

Hi rune, Sorry been busy!

Probably this week :)

@samdion1994
Copy link
Collaborator Author

samdion1994 commented Nov 16, 2023

Hey @Rune-Christensen,

Think you can merge this and I could do another fix later?

There a lot of stuff I need to change for this request and little time, but I get what you're saying and I agree that this needs to be fixed. Also I think we might have the same problem with the file section expander.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Output file group declaration copied twice in the generated program
3 participants