Project

General

Profile

Bug #10935

Inherit the NO-UNDO option for a temp-table defined using the LIKE temp-table option

Added by Paul Bodale 8 months ago. Updated 6 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Paul Bodale
Target version:
-
Start date:
Due date:
% Done:

100%

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

History

#1 Updated by Paul Bodale 8 months ago

  • Assignee set to Paul Bodale

For this 4gl testcase:

DEFINE TEMP-TABLE tt1 NO-UNDO
   FIELD f1 AS INT.

MESSAGE TEMP-TABLE tt1:UNDO. // no

DEFINE TEMP-TABLE tt2 LIKE tt1.

MESSAGE TEMP-TABLE tt2:UNDO. // no

CREATE tt2.
MESSAGE RECID(tt2). // 2304

DELETE tt2.

CREATE tt2.
MESSAGE RECID(tt2). // 2304

Progress prints no no 2304 2304 but FWD prints no yes 2304 2305.

This shows that if a temp-table is defined using the LIKE temp-table option, then it should also inherit the NO-UNDO option if stated for the temp-table indicated through the LIKE option.

#3 Updated by Paul Bodale 8 months ago

  • Status changed from New to WIP
  • % Done changed from 0 to 10

I tried adding KW_NO_UNDO among the elements of the SchemaDictionary.TABLE_PROPS converted the testcases I have and tested again. Unfortunately it doesn't solve the issue but it doesn't look like it hurts so it might be worth keeping.

#4 Updated by Paul Bodale 8 months ago

It looks like FWD partially ignores the NO-UNDO keyword for a temp-table that also has the LIKE option.

Check this testcase for example:

DEFINE TEMP-TABLE tt1 NO-UNDO
   FIELD f1 AS INT.

MESSAGE TEMP-TABLE tt1:UNDO.

DEFINE TEMP-TABLE tt2 NO-UNDO LIKE tt1.

MESSAGE TEMP-TABLE tt2:UNDO.

CREATE tt2.
MESSAGE RECID(tt2).

DELETE tt2.

CREATE tt2.
MESSAGE RECID(tt2).

The only difference from the testcase in note 1 is that I explicitly declare the tt2 temp-table as being NO-UNDO. Those definitions convert to:

   Tt2_1_1.Buf tt2 = TemporaryBuffer.define(Tt2_1_1.Buf.class, "tt2", "tt2", false, false);

   Tt1_1_1.Buf tt1 = TemporaryBuffer.define(Tt1_1_1.Buf.class, "tt1", "tt1", false, false);

Last boolean arguments sets the undoable argument. false means is NO-UNDO so here everything should be correct.

The difference found between the 2 temp-tables is the @Table annotation from the DMO:

//tt2
@Table(name = "tt2", legacy = "tt2")

//tt1
@Table(name = "tt1", legacy = "tt1", noUndo = true)

#5 Updated by Paul Bodale 8 months ago

I did a little bit of digging into the methods that are related in some way to the SchemaDictionary.TABLE_PROPS.

There is the SchemaDictionary.copyTableProperties method which looks like it does the thing we are looking for but it's actually creating a different ast of the same temp-table but which is saved in the schema namespace. From what I understand it's parsing the definition encountered in the procedure file and saving it into a global scope which can be altered according to the definition.

Before the call to the SchemaDictionary.copyTableProperties method there is the ProgressParser.like_clause method which matches on the LIKE keyword and simply returns a tt1 ast which is annotated with some information. This is also added to the definition saved in the schema namespace.

#6 Updated by Paul Bodale 8 months ago

  • % Done changed from 10 to 100
  • Status changed from WIP to Review
  • reviewer Constantin Asofiei added

Created task branch 10935a and committed rev. 16297 which introduces conversion changes to allow a temp-table defined with LIKE temp-table option to inherit the NO-UNDO option.

Also committed rev. 16298 to add the history entry.

#7 Updated by Constantin Asofiei 8 months ago

  • % Done changed from 100 to 90
  • Status changed from Review to WIP
Paul, the direction is good, but the implementation needs to be rewritten to:
  • execute somewhere in schema phase (p2o_pre?) and not in post_parse_fixups
  • do not use createString2LongMap - this is a 'persistent map' saved in the cvtdb; this is not needed for this processing
  • more, in post_parse_fixups this map is global per the entire conversion, and not the current file

#8 Updated by Constantin Asofiei 8 months ago

But, why not get the NO-UNDO at the parse time and save it maybe as a special annotation, like-no-undo?

#9 Updated by Paul Bodale 8 months ago

Constantin Asofiei wrote:

Paul, the direction is good, but the implementation needs to be rewritten to:
  • execute somewhere in schema phase (p2o_pre?) and not in post_parse_fixups

After adding KW_NO_UNDO among the elements of the SchemaDictionary.TABLE_PROPS the problem is not the @Table annotation from the DMO anymore. The only thing left to do is to make FWD convert the definition of the temp-table to include the parameter false that corresponds to the undo option when necessary.
I don't think that the .schema files which are parsed in p2o.xml correspond to this. I looked at the .jast files and that parameter is set based on the following action from the buffer_definitions.rules line 452:

               <rule on="false">type == prog.define_temp_table or type == prog.define_work_table
                  <action>
                     ttnoundo = downPath(this, prog.kw_temp_tab, prog.kw_no_undo) or
                                downPath(this, prog.kw_work_tab, prog.kw_no_undo)
                  </action>

  • do not use createString2LongMap - this is a 'persistent map' saved in the cvtdb; this is not needed for this processing
  • more, in post_parse_fixups this map is global per the entire conversion, and not the current file

Understood. I'll use createMap instead.

But, why not get the NO-UNDO at the parse time and save it maybe as a special annotation, like-no-undo?

Can do but I'll still need a Map to store the ids of the temp-tables with their name as the key because that's all I have at the definition level. There is no "refid" annotation for the LIKE temp-table. And I think at that point I might as well add the NO-UNDO node then and let the conversion process do the rest.

Also, for this to work, I need the HashMap to contain the corresponding entry before I hit the rule where I process the temp-table definition that contains the LIKE option. I noticed that the order in which the temp-tables are declared is altered at some point in the conversion process.

So is there a better place to introduce those changes?

#10 Updated by Constantin Asofiei 8 months ago

I think we need to go the same way as variable references are resolved, via tempIdx:
  • in SymbolResolver, add a case-insensitive map of temp-table names to tempidx values
  • when a temp-table is defined, it will put a new tempidx key in this map
  • when a LIKE temp-table is referenced, it will put an annotation under the LIKE'd temp-table with the tempidx value for that name
  • during post_parse_fixups, this then can be processed in the same way as the variables, and transform the tempidx into AST IDs
  • later, during conversion, you can have access to the full AST of the LIKE'd temp-table.

#11 Updated by Paul Bodale 8 months ago

  • % Done changed from 90 to 70

There is a problem. For the following test case:
A.cls

USING Progress.Lang.*.

BLOCK-LEVEL ON ERROR UNDO, THROW.

CLASS task.A: 

   DEFINE PROTECTED TEMP-TABLE tt1 NO-UNDO
      FIELD f1 AS INT.

   DEFINE TEMP-TABLE tt3 LIKE tt1.

   METHOD PUBLIC VOID printMesA(  ):
     MESSAGE TEMP-TABLE tt1:UNDO.
     MESSAGE TEMP-TABLE tt3:UNDO.
   END METHOD.

END CLASS.

B.cls

USING Progress.Lang.*.
USING task.A.

BLOCK-LEVEL ON ERROR UNDO, THROW.

CLASS task.B INHERITS A: 

   DEFINE TEMP-TABLE tt2 LIKE tt1.

    METHOD PUBLIC VOID printMesB(  ):
      MESSAGE TEMP-TABLE tt1:UNDO.
      MESSAGE TEMP-TABLE tt2:UNDO.
    END METHOD.

END CLASS.

And procedure:

DEFINE VARIABLE b AS B NO-UNDO.
b = NEW B().
b:printMesB(). // no yes

DEFINE VARIABLE a AS A NO-UNDO.
a = NEW A().
a:printMesA(). // no no

So the NO-UNDO option is not inherited if the declaration of the temp-table used in the LIKE option is not present in the same file.

#12 Updated by Paul Bodale 8 months ago

I tested this on OE version 11.6 and 11.7.

#13 Updated by Paul Bodale 8 months ago

Committed rev. 16299 on branch 10935a which contains the solution described in #10935-10.

After committing this revision I realized that converting with this revision results in the noUndo parameter of the @Table annotation from the DMO of the temp-table being omitted yet again.

Constantin please take a look at those changes if you have the time. I'm currently investigating the problem with noUndo parameter.

#14 Updated by Paul Bodale 8 months ago

  • % Done changed from 70 to 90

Committed rev. 16301 on branch 10935a which fixes the process of copying table properties in case the filter is null.

This fix also contains the following patches from branch 9457c:

=== modified file 'src/com/goldencode/p2j/schema/SchemaDictionary.java'
--- old/src/com/goldencode/p2j/schema/SchemaDictionary.java    2025-10-17 05:20:23 +0000
+++ new/src/com/goldencode/p2j/schema/SchemaDictionary.java    2025-12-02 08:26:45 +0000
@@ -760,7 +760,8 @@
    {
       // these token types are used in a binary search, so they must be in ascending order by
       // token number
-      KW_LABEL_SA, KW_VALMSG, KW_VALMG_SA, KW_LABEL, KW_NAMESP_P, KW_NAMESP_U, KW_SERIALZN, KW_XML_NNAM 
+      KW_LABEL_SA, KW_VALMSG, KW_VALMG_SA, KW_LABEL, KW_NO_UNDO, KW_NAMESP_P, KW_NAMESP_U, KW_SERIALZN,
+      KW_XML_NNAM
    };

    // Sort field properties token type array into ascending numeric order,

=== modified file 'src/com/goldencode/p2j/schema/SchemaDictionary.java'
--- old/src/com/goldencode/p2j/schema/SchemaDictionary.java    2025-12-02 08:26:45 +0000
+++ new/src/com/goldencode/p2j/schema/SchemaDictionary.java    2025-12-08 09:57:15 +0000

@@ -5650,7 +5649,7 @@
             case EntityName.TABLE:
                source = modelTable;
                target = getTable(toNode);
-               copyTableProperties(source, target, null);
+               copyTableProperties(source, target, TABLE_PROPS);
                String modelName = (String) source.getAnnotation("model");
                if (modelName == null)
                {

There is still the following testcase which doesn't behave the same as in Progress:

DEFINE TEMP-TABLE tt1 NO-UNDO
   FIELD f1 AS INT.

DEFINE TEMP-TABLE tt2 LIKE tt1.
DEFINE TEMP-TABLE tt3 LIKE tt2.
DEFINE TEMP-TABLE tt4 LIKE tt3.
DEFINE TEMP-TABLE tt5 LIKE tt4.

MESSAGE TEMP-TABLE tt1:UNDO. // no
MESSAGE TEMP-TABLE tt2:UNDO. // no
MESSAGE TEMP-TABLE tt3:UNDO. // no
MESSAGE TEMP-TABLE tt4:UNDO. // no
MESSAGE TEMP-TABLE tt5:UNDO. // no

I'll continue working on this.

#15 Updated by Paul Bodale 8 months ago

  • % Done changed from 90 to 100

Committed rev. 16302 on branch 10935a which contains the fix for the case described in the note above.

This commit also contains the following patches from branch 9457c:

=== modified file 'src/com/goldencode/p2j/schema/SchemaDictionary.java'
--- old/src/com/goldencode/p2j/schema/SchemaDictionary.java    2025-10-17 05:20:23 +0000
+++ new/src/com/goldencode/p2j/schema/SchemaDictionary.java    2025-12-02 08:26:45 +0000
@@ -760,7 +760,8 @@
    {
       // these token types are used in a binary search, so they must be in ascending order by
       // token number
-      KW_LABEL_SA, KW_VALMSG, KW_VALMG_SA, KW_LABEL, KW_NAMESP_P, KW_NAMESP_U, KW_SERIALZN, KW_XML_NNAM 
+      KW_LABEL_SA, KW_VALMSG, KW_VALMG_SA, KW_LABEL, KW_NO_UNDO, KW_NAMESP_P, KW_NAMESP_U, KW_SERIALZN,
+      KW_XML_NNAM
    };

    // Sort field properties token type array into ascending numeric order,

=== modified file 'src/com/goldencode/p2j/schema/SchemaDictionary.java'
--- old/src/com/goldencode/p2j/schema/SchemaDictionary.java    2025-12-02 08:26:45 +0000
+++ new/src/com/goldencode/p2j/schema/SchemaDictionary.java    2025-12-08 09:57:15 +0000

@@ -5650,7 +5649,7 @@
             case EntityName.TABLE:
                source = modelTable;
                target = getTable(toNode);
-               copyTableProperties(source, target, null);
+               copyTableProperties(source, target, TABLE_PROPS);
                String modelName = (String) source.getAnnotation("model");
                if (modelName == null)
                {

With this commit the scope for this task is completed but I'll keep this task in WIP to investigate why are there 2 NO-UNDO ast properties (one uppercase and one lowercase) in the .dict and other schema files when a temp-table definition contains the property NO-UNDO.

Constantin, when you're available, please review.

#16 Updated by Constantin Asofiei 8 months ago

Paul, I'm good with the changes.

#17 Updated by Constantin Asofiei 6 months ago

  • Status changed from WIP to Internal Test

#18 Updated by Paul Bodale 6 months ago

Rebased branch 10935a with trunk rev. 16359

#19 Updated by Paul Bodale 6 months ago

Rebased branch 10935a with trunk rev. 16377

#20 Updated by Constantin Asofiei 6 months ago

  • Status changed from Internal Test to Merge Pending

Merge to trunk after 10352a

#21 Updated by Paul Bodale 6 months ago

  • Status changed from Merge Pending to Test

Branch 10935a was merged into trunk as rev. 16379 and archived.

#22 Updated by Constantin Asofiei 6 months ago

  • Status changed from Test to Closed

Also available in: Atom PDF