Bug #3324
Incorrect handling of temp tables with the same name makes translated code non-compilable
100%
Related issues
History
#1 Updated by Jaroslaw Haziak almost 7 years ago
- File test.tt.zip added
To reproduce:
1) Create 10 files with the name test.<NN>.p, where <NN> = 01..10 containing the temp table definition with the same name but different name of the field:
DEF TEMP-TABLE row NO-UNDO FIELD f<N> AS CHAR .
where <N> = 1..10
2) Create the 11th (test.11.p) with the following content:
DEF TEMP-TABLE row NO-UNDO FIELD f11 AS CHAR . DEF TEMP-TABLE row1 NO-UNDO FIELD ff1 AS CHAR .
These 11 files (attached to this note) are translated to non-compilable code: created class Row11_1Impl is not compilable.
As a side note:
I think that suffixing names of temp-tables with serial numbers is a bad idea because readability of translated code is very poor.
In my opinion, prefixing such names with names of procedures that define them would be much better.
#2 Updated by Greg Shah almost 7 years ago
From Ovidiu, working in #3330-277 which is the same root cause:
I think I have a slight idea about what is wrong. I analyzed the temp-table definitions from some source files that fails to compile. Most of them have two copies of same definition, using the LIKE syntax:
def TEMP-TABLE w-data no-undo [not important]. def TEMP-TABLE w-data1 NO-UNDO like w-data.
Note the 1 appended to second definition. We also add a counter (two in fact, one for each partition/set - and one for each new definition, separated by _).Now, if enough files are provided with similar named temp-tables the following collision will occur:
w-data<N>_<K> with w-data1<N2>_<K2>, if N>10 and N2<10.To fix this, I guess we need to add another separator ( '_' should be enough ) in to separate the first counter from the table converted name.
And this:
The fix seems to be working. I used the example in #3324-1, and did the following change:
./rules/schema/p2o_pre.xml:222 - tmpName = sprintf('%s%d', tmpName, nextTTSuffix) + tmpName = sprintf('%s_%d', tmpName, nextTTSuffix)and hocus-pocus, the generated files do compile!
#3 Updated by Greg Shah almost 7 years ago
This fix is checked in to branch 3330a which we intend to put through testing starting this weekend. It will be merged to trunk as soon as it passes testing.
#5 Updated by Ovidiu Maxiniuc almost 7 years ago
This is not a bullet-prof solution. There are still cases when this kind of name collisions can occur. We need to think of a better way to name the temp-tables and their interfaces. The old naming scheme was just a TT<n>. It was safe from this POV, but it was too cryptic.
The solution to add the procedure name, as Jaroslaw suggested, is not perfect: the name is generated while doing a cross-file analysis, when SHARED and tables with same structure are matched and a common interface name is selected for a set of compatible TEMP-TABLE definitions. I think it would be very strange to work with ttN_procA in procB file.
As a last resort, a temporary storage might be used to collect required data (as opposed to local annotations) and make the information available globally. However, I think there might be a way to solve this by using smarter annotations.
#6 Updated by Greg Shah almost 7 years ago
- Assignee set to Ovidiu Maxiniuc
- % Done changed from 0 to 100
- Status changed from New to Closed
Branch 3330a was merged to trunk as revision 11164. It is expected to be released as FWD v3.1 in the next few days.
Ovidiu: Please create a new task for the remaining work described in #3324-5. If you have a testcase for a scenario that can still fail, please post it there.
#7 Updated by Greg Shah over 6 years ago
The fix for this task is included in FWD v3.1 which was recently released. Download it from https://proj.goldencode.com/projects/p2j/wiki/FWD_v3_1_0
#8 Updated by Greg Shah over 4 years ago
- Project changed from FWD to Database
#9 Updated by Greg Shah over 4 years ago
- Related to Feature #4418: improve/fix temp-table naming approach added