Project

General

Profile

Feature #7178

move the buffer scoping calculations to parse time

Added by Greg Shah over 3 years ago. Updated 7 days ago.

Status:
New
Priority:
Normal
Target version:
-
Start date:
Due date:
% Done:

0%

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

Related issues

Related to Base Language - Bug #10601: Resolving of unqualified fields New

History

#2 Updated by Greg Shah over 3 years ago

When we initially created the parser, SymbolResolver (SR), SchemaDictionary (SD) and the rest of our front end infrastructure, we did not yet have a proper understanding of the nature of record scoping. We did not appreciate the importance or complexity of the record scoping rules until we had already put quite a lot of infrastructure together including a patchwork of approximations built into the parser/SR/SD. Over time we added to those approximations as new issues were found. To date we have always found a way to patch our hacked approximate approach to make things work. With #7125, we've hit some problems that are not easily handled in our approximation approach. The time to layer more patches on top of this mess is over.

Of course, to implement the downstream processing we did have to figure out the rules (most of them anyway) and implement them faithfully. The problem is that these are all handled at the annotations phase (annotations/record_scoping*.rules) and this is way too late to properly resolve all schema references at parse time.

This task is meant to:

1. Update our documentation to match the full rules. We have some documentation and example scope "snippets" here (scroll down to "Record Scopes"). This is out of date, so some rules are not properly defined. On the other hand, it is a pretty good start, especially for the most complex piece which is the weak scope processing and how these scopes combine in weird ways. More recently, we've found some strange behavior with strong scopes which is not documented. Some of that behavior may be implemented at parse time only. We also may have implemented some of the rules in TRPL and/or the BufferScopeWorker without reflecting the rules in the documentation. I propose that we use the buffer scoping testcases being build by Marian's team in #6855 to ensure that we have the proper set of rules defined.

2. Implement these rules in the parser, SR and SD (see Schema Dictionary design). I expect we will remove all the propagation, promotion and other weird patches. I also expect that most (if not all) of the downstream rules will be unnecessary as well. We will still want to retain the BUFFER_SCOPE node and inserting that will be tricky during the parse itself because its position is not known until . We need to decide how to handle it.

There are real benefits to handling it no later than early annotations so that we can see all the results during reporting. Our IDE support will also benefit from early resolution of all of these scopes. I would like to display this information graphically so that the 4GL developer can see it in real time while editing the code.

The shorter term result will be to get the scoping and name resolution to match all known rules exactly.

#3 Updated by Greg Shah 6 months ago

Before we rewrite the approach, I want us to have a complete set of ABLUnit testcases. These should start with all of the original buffer scoping cases that I originally used for creating the first list of rules. You can see these testcases as described in #7178-1.

From there, we need to add the various problem cases that we have found over the years. We need to "mine" Redmine tasks for these. I would expect that many of the history entries for SchemaDictionary would trace back to buffer scoping bugs. Also, we know there are some problem cases with 2 recent projects. I think there may even some for 2 older projects, which they coded around in the 4GL.

We can assign someone to write these tests but we need them done. From those tests, I want us to analyze them and write a wiki page with a comprehensive set of buffer scoping rules. From there we can design the solution.

You suggested some information is missing to allow this to work at parse time. If we need to move some other processing back to parse time, we will do so. Make a list of anything you know about.

#5 Updated by Greg Shah 4 months ago

  • Related to Bug #10601: Resolving of unqualified fields added

#9 Updated by Dănuț Filimon 4 months ago

There are buffer scoping tests available in the testcases project, /tests/persistence/buffer_scoping/. A quick scan of the issues, I found the following ones that might be helpful in writing more tests - #10898, #10796, #10633, #10567, #10560, #10542, #10483, #10455, #10371, #10336, #10294, #10195, #9914, #9763, #8758, #9565, #9457, #9298, #9241, #9199, #9050, #9015, #9014, #9004, #8944, #8871, #8860, #8593, #8444, #8433, #8388, #8363, #8243 (not all of them, but enough to investigate and check if test cases can be made from those issues).

This wiki https://proj.goldencode.com/projects/p2j/wiki/Buffer_Scoping_Tests is where information about the tests will be written.

#10 Updated by Dănuț Filimon 4 months ago

A bit of a summary update on the current tests:
  • Added a few tests based on #7178-9 tasks where I could extract a few scenarios.
  • Added tests based on the provided wiki from #7178-2
  • Added additional tests based on the same wiki (using class methods, functions, procedures)
Next steps:
  • Add tests where temporary tables are defined for a database table (defined with the same name)
  • Add tests where multiple schemas are used and the same table is defined (defined with the same name)
  • Create tests for scenarios that are expected to fail or that can't be built, then use harness to test them (currently working on this)
  • Separate the current unit tests and use separate procedure to add another set of tests.
  • Convert and test everything.

I have a small issue with the VM, the database is not connecting anymore so I can't validate the tests I write anymore. Working on fixing this.

#11 Updated by Dănuț Filimon 4 months ago

Because of the issue with the VM database, Constantin told me to use the Procedure Editor. However, it seems I need a specific configuration to run unit tests because I can't check the results.

On a side note, I took all the tests and converted them. I found a single scenario where the code does not compile.

#12 Updated by Constantin Asofiei 4 months ago

Dănuț Filimon wrote:

Because of the issue with the VM database, Constantin told me to use the Procedure Editor. However, it seems I need a specific configuration to run unit tests because I can't check the results.

On a side note, I took all the tests and converted them. I found a single scenario where the code does not compile.

Danut, just to make sure: the tests are individual .p programs, right, and not unit tests. Because what we need here can't be easily mapped in unit tests.

#13 Updated by Dănuț Filimon 4 months ago

Constantin Asofiei wrote:

Dănuț Filimon wrote:

Because of the issue with the VM database, Constantin told me to use the Procedure Editor. However, it seems I need a specific configuration to run unit tests because I can't check the results.

On a side note, I took all the tests and converted them. I found a single scenario where the code does not compile.

Danut, just to make sure: the tests are individual .p programs, right, and not unit tests. Because what we need here can't be easily mapped in unit tests.

I created unit tests, but I can easily extract everything into a set of separate procedures.

#14 Updated by Constantin Asofiei 4 months ago

Dănuț Filimon wrote:

I created unit tests, but I can easily extract everything into a set of separate procedures.

Yes, please, we need this to be as simple as possible. The .cls approach will be a variation of the tests.

#15 Updated by Constantin Asofiei 4 months ago

Danut, this is a list of main issues (but not the only issues) which should be covered by the tests. Ideally we can find a way to automate creating these tests, after we find some rules to apply. The rules are not 'just standalone', we need combinations of these cases.
  • temp-table has same name as a permanent table
    • separate case: temp-table name is a prefix of the permanent table (like 'boo' instead of 'book')
  • a table exists in multiple schemas (same name and same fields)
  • explicit and implicit buffers
    • DEF BUFFER syntax with a name which matches both a perm and temp table name (there is the TEMP-TABLE option at this statement)
    • DEF BUFFER buf FOR buf (i.e. both names are the same)
    • buffer parameters are another kind of explicit buffer def. This includes DEF BUFFER for the parameter table.
    • tests need to cover explicit and implicit buffers for the same table, and unqualified fields - which buffer does the field belong?
  • when talking about buffers, we need combinations of temp-table and schema buffers being into scope at the same time, having either same name or fields.
    • this can be case where you bring into scope a perm buffer, and after that use a def temp-table with the same name/fields as the 'perm buffer'
  • buffers used in internal entries (procedures, functions) and external program. Buffers used in .cls are a separate case.
  • qualified and unqualified fields, qualified and unqualified tables
    • for example, bring two buffers (having fields with the same name) into scope; which buffer has the unqualified field?
      def temp-table tt1 like book.
      def temp-table tt2 like book.
      for each tt1:
         create book.
         message book-title. // what is the buffer?
         buffer-copy tt1 to tt2.
         message book-title. // what is the buffer?
      end.
      
    • DEF TEMP-TABLE ... fieldX like field1 - either a temp-table or a perm table defines the field, with the perm table in scope or not.
  • abbreviations:
    • for example: bring a buffer into a free scope, then use it via an abbreviation
    • same for fields
  • there are cases where the field reference is coupled with the defining statement.
    • relation-fields at a dataset
    • BUFFER-COPY with EXCEPT and other field-only clauses.
    • FOR EACH with unqualified fields in WHERE or BY clauses
    • these tests most likely require having a previous buffer 'in scope' which has same fields as that statement, but is not targeted by that statement.
  • unqualified fields used in frames, like this - this gets a OE compile error if tt2 is defined, but we need to expand both valid and invalid tests:
    def temp-table tt1 field fd1 as int field fd2 as int.
    // def temp-table tt2 field fd2 as int.
    
    form tt1.fd1 fd2 with frame f1.
    
    for each tt1:
       display tt1.fd1 fd2 with frame f1.
    end.
    

Look into Chapter_21_Record_Buffer_Definition_and_Scoping to get the list of statements (cases may be missing from there, so don't treat it as comprehensive).

I'll come back with more examples and cases. So I think we need two schemas, where we have:
  • tables having the name as a prefix of another table name (like 'boo' and 'book')
  • tables with different name but same field names
  • tables which exist in a single schema (no field collisions)

#16 Updated by Dănuț Filimon 4 months ago

Thank you for the list Constantin, I will look into a way to automate creating those tests. Although I do not see it now, I see simple structures that can be separated into include files.

I finished separating the unit tests into separate procedures.

#17 Updated by Dănuț Filimon 4 months ago

Constantin, I've improved the tests and made the following procedure:

define variable iCounter as integer initial 0.
define variable content as character.
define variable cVal as character.

repeat iCounter = 1 to 29:
    cVal = STRING(iCounter, "99").
    content = "~{tests/persistence/buffer_scoping/support/include/record-scope&1.i customer~}".
    content = SUBSTITUTE(content, cVal).
    OUTPUT TO VALUE(SUBSTITUTE("tests/persistence/buffer_scoping/support/generated/record-scope-persistent&1.p", cVal)).
    PUT UNFORMATTED content SKIP.
    OUTPUT CLOSE.
end.

repeat iCounter = 1 to 29:
    cVal = STRING(iCounter, "99").
    content = "~{tests/persistence/buffer_scoping/support/include/record-scope&1.i customer~}".
    content = SUBSTITUTE(content, cVal).
    OUTPUT TO VALUE(SUBSTITUTE("tests/persistence/buffer_scoping/support/generated/record-scope-temp&1.p", cVal)).
    PUT UNFORMATTED "define temp-table customer no-undo like customer." SKIP.
    PUT UNFORMATTED content SKIP.
    OUTPUT CLOSE.
end.

repeat iCounter = 1 to 29:
    cVal = STRING(iCounter, "99").
    content = "~{tests/persistence/buffer_scoping/support/include/record-scope&1.i customer~}".
    content = SUBSTITUTE(content, cVal).
    OUTPUT TO VALUE(SUBSTITUTE("tests/persistence/buffer_scoping/support/generated/record-scope-buffer&1.p", cVal)).
    PUT UNFORMATTED "define buffer customer for customer." SKIP.
    PUT UNFORMATTED content SKIP.
    OUTPUT CLOSE.
end.
This generates 29x3 procedures which are as simple as they can get :)

If there is a way to improve this, let me know.

#18 Updated by Dănuț Filimon 4 months ago

Let's see, small update. I've made tests for the following scenarios:
  • all cases mentioned in #7178-2 were made into include files and used to generate simple procedure tests
  • temp-table has same name as a permanent table expanded from the initial include files
  • temp-table name is a prefix of the permanent table ('cust' instead of 'customer')
  • DEF BUFFER syntax with a name which matches both a perm and temp table name (there is the TEMP-TABLE option at this statement)
  • DEF BUFFER buf FOR buf (i.e. both names are the same)
  • buffer parameters for procedures, functions
  • a few small tests with unqualified fields
  • combinations of temp-table and schema buffers being into scope at the same time
  • abbreviations

The good part is that those tests are generated, but not all of them are correct when we talk about syntax. The purpose of those wrong procedures is to use them to define better rules, then create a tool that does something similar to COMPILE ... LISTING so that we can confirm the buffer scoping matches 4GL.

#19 Updated by Dănuț Filimon 4 months ago

Started validating some of the tests I generated only to find most of them being actually wrong when the syntax got complicated to generate.

The following is wrong because it will expand the nested include file and use the first keywords from it as parameters:

content = "~{tests/persistence/buffer_scoping/support/include/procedure/proc-record-scope01.i ~{tests/persistence/buffer_scoping/support/include/record/record-scope&1.i customer~}~}".

This syntax is the correct one, which I am working on replacing.

content = "~{tests/persistence/buffer_scoping/support/include/procedure/proc-record-scope02.i customer customer customer &4""~{tests/persistence/buffer_scoping/support/include/record/record-scope01.i customer~}""~}".

There are exactly 3516 files being generated, I will also check if there any duplicated and remove them.

#20 Updated by Greg Shah 4 months ago

I don't want us to spend too much time trying to use obscure 4GL preprocessor usage patterns. We don't have to generate this code using the 4GL itself. We can always write some Java code to do it.

#22 Updated by Dănuț Filimon about 1 month ago

The tests were developed on the examples from #7178-2.

This is a recap of the tests that were completed or have a definite set of tests created until now:
  • Tests that use a persistent table ("customer")
  • Tests that use an abbreviated persistent table ("cust")
  • Tests that use a temporary table that has the same name as a persistent table ("customer")
  • Tests that use an abbreviated temporary table name of a persistent table ("cust")
  • Tests that use DEFINE BUFFER
  • Tests that use DEFINE BUFFER buf FOR buf
  • Tests that use DEFINE PARAMETER BUFFER
  • Tests that use combinations of temp-table and schema buffers being into scope at the same time (buffers have the same name)
  • Tests that use buffers used in internal entries (procedures, functions) and external program.
  • A few unqualified fields tests.

Currently working on wrapping up the unqualified tests work and move on to the next set of tests.

#23 Updated by Dănuț Filimon 27 days ago

I've wrapped up the FOR EACH WHERE/BY, dataset with relation-fields, lots of unqualified fields/tables tests.

I'm only left with the tests that use classes and I have to include the .df for the following tests:

The relation-field test managed to generate a lot of tests, mostly because of the sheer amount of combinations that were possible between unqualified or not tables/fields.

#24 Updated by Greg Shah 21 days ago

In the latest standard workstation, I've added a tool called bzr-issues which can be used to build a list of all tasks referenced by bzr commits for a given list of files. When I ran it with bzr-issues src/com/goldencode/p2j/schema/SchemaDictionary.java, we see this result:

#1521, #1868, #1896, #1969, #1985, #2054, #2069, #2090, #2120, #2135, #2156, #2315, #2493, #2645, #2683, #3024, #3253, #3255, #3275, #3310, #3330, #3471, #3490, #3498, #3500, #3509, #3511, #3514, #3515, #3518, #3520, #3521, #3523, #3525, #3527, #3528, #3535, #3539, #3544, #3566, #3567, #3588, #3589, #3592, #3690, #3696, #3750, #3751, #3753, #3755, #3756, #3762, #3764, #3772, #3773, #3793, #3799, #3809, #3810, #3812, #3813, #3814, #3816, #3817, #3818, #3819, #3820, #3821, #3822, #3838, #3854, #3855, #3867, #3876, #3880, #3912, #3960, #4046, #4056, #4069, #4074, #4081, #4105, #4121, #4123, #4124, #4140, #4147, #4160, #4162, #4164, #4170, #4171, #4174, #4181, #4182, #4190, #4191, #4198, #4201, #4207, #4208, #4221, #4231, #4278, #4291, #4314, #4336, #4349, #4354, #4355, #4363, #4375, #4381, #4384, #4393, #4394, #4395, #4397, #4440, #4450, #4464, #4476, #4484, #4492, #4498, #4546, #4554, #4570, #4577, #4578, #4581, #4589, #4599, #4600, #4601, #4602, #4606, #4624, #4636, #4642, #4657, #4782, #4785, #4791, #5034, #5128, #5232, #5534, #5567, #5684, #5703, #5919, #6129, #6370, #6492, #6556, #6616, #6617, #6659, #6667, #6685, #6813, #7020, #7180, #7202, #7203, #7204, #7206, #7215, #7328, #7449, #7645, #7795, #8437, #9198, #9213, #9258, #9264, #9276, #9294, #9350, #9597, #10223, #10338, #10590, #10841, #10935, #10978, #11015, #11510

I know this is a horribly large list, but theoretically it will contain a pretty complete set of the bugs that caused us to patch/edit the SchemaDictionary. Sorry for this, but please review each of those tasks and pull out (or create if needed) the testcase(s) that represent the task. Add those to our buffer scoping test set. This should go a long way to ensuring that we don't regress anything with future changes.

#25 Updated by Dănuț Filimon 20 days ago

Amazing!

I did not find any tests for #1521, #1868, #1896, #1969, #2683, #3024, #3253, #3255, #3310, #3471, #3490, #3498 (unrelated), #3500, #3509, #3511, #3514, #3515, #3518, #3520, #3521, #3523, #3525, #3527, #3528, #3535, #3544, #3566, #3588, #3589, #3592, #3696, #3753, #3755, #3756, #3762, #3764, #3772, #3793, #3810, #3812, #3813, #3817, #3818, #3819, #3820, #3821, #3822, #3854, #3855, #3867, #3876, #3880, #3912, #3960, #4046, #4056, #4069, $4081.

I integrated tests from #1985, #2054, #2069, #2090, #2120, #2156, #2315, #2493, #2645, #3275, #3330, #3539, #3567, #3690, #3750, #3751, #3799, #3809, #3814, #3816, #4074.

I can still integrate tests from #1985 (customer specific persistent tables), #2135, #2493 (extents), #2645 (workfile), #3330 (from ChUI, double check), #3567 (persistent tables), #3750 (big task, just double checking), #3751 (oo.Foo), #3773 (double check), #3814, #3838 with some additional work.

Greg, I think there's a good reason to go into the related tasks for all those tasks and check if we can find more test cases.

#26 Updated by Greg Shah 20 days ago

Greg, I think there's a good reason to go into the related tasks for all those tasks and check if we can find more test cases.

100% agree.

You can use bzr-issues with additional source files to help here as well.

#27 Updated by Dănuț Filimon 19 days ago

Second round.

I did not find any tests for #4123, #4124, #4147, #4160, #4162, #4170, #4171, #4174, #4181, #4182, #4190, #4191, #4198, #4201, #4221, #4278, #4291, #4336, #4349, #4354, #4355, #4375, #4381, #4393, #4394, #4395, #4464, #4498, #4546, #4570, #4577, #4578, #4581, #4589, #4599, #4600, #4601, #4606, #4624, #4636, #4642, #4657, #4785, #5034, #5232, #5534, #5567, #5684, 5703, #5919, #6492, #6556, #6616, #6617, #6685, #7180, #7202, #7203, #7215, #7449, #7645, #8347, #9198, #9213, #9264, #9276, #9294, #9350, #9597, #10223

I integrated tests from #4121, #4140, #4207, #4208, #4231, #4314, #4363, #4384, #4397, #4450, #4476, #4484, #4554, #4602, #4782, #5128, #6129, #6370, #6659, #6813, #7020, #7204, #7206, #7795, #9258, #10338, #10590, #10841, #10935, #10978, #11015

I can still integrate tests from #4105, #4121, #4207, #4208, #4231 (double check), #4440, #4492, #4782, #4791, #6659, #6667, #7020, #7328 with some additional work.

#28 Updated by Greg Shah 19 days ago

I can still integrate tests from #4105, #4121, #4207, #4208, #4231 (double check), #4440, #4492, #4782, #4791, #6659, #6667, #7020, #7328 with some additional work.

Yes, please do.

#29 Updated by Dănuț Filimon 15 days ago

I integrated the remaining tests from #7178-25 and #7178-27.

#30 Updated by Dănuț Filimon 8 days ago

Greg Shah wrote:

In the latest standard workstation, I've added a tool called bzr-issues which can be used to build a list of all tasks referenced by bzr commits for a given list of files. When I ran it with bzr-issues src/com/goldencode/p2j/schema/SchemaDictionary.java, we see this result:

Greg, how do I get my hands on this tool? I tried to setup the workstation again and bzr-issues is not available. I want to dig up more tests.

#31 Updated by Dănuț Filimon 8 days ago

Dănuț Filimon wrote:

Greg Shah wrote:

In the latest standard workstation, I've added a tool called bzr-issues which can be used to build a list of all tasks referenced by bzr commits for a given list of files. When I ran it with bzr-issues src/com/goldencode/p2j/schema/SchemaDictionary.java, we see this result:

Greg, how do I get my hands on this tool? I tried to setup the workstation again and bzr-issues is not available. I want to dig up more tests.

Fixed the issue. I thought the bootstrap script will do all those things for me. I found bzr-issues and it works properly.

#32 Updated by Dănuț Filimon 7 days ago

I went over more and more related issue and added another 100+ tests from multiple issues, had to rewrite most of them to use temp-tables/tables already defined in tstcasesdb.

Also available in: Atom PDF