FWD » Core Development » Database

• converting the statement at runtime into some representation of a temp table definition that can be used by the runtime;
• updating or replacing the Hibernate SessionFactory instance which is used for temp table support, in order to include the new temp-table definition;
• creating the temp table in the H2, in-memory, temp-table database;
• generating a new DMO class (and instance) for the created temp table on the fly (or some representation of a DMO that will act sufficiently like one behind the RecordBuffer dynamic proxy, and play nicely with Hibernate if necessary).

Code review 20130129b:

I see some problems with the DynamicTempTable interface. I believe all of the getXXX methods belong in the parent (Buffer) interface, since AFAIK, they all apply to buffers in general, not just temp-table buffers [LATER CORRECTION: getDefaultBufferHandle is only about temp-tables]. Also, we cannot use the JavaBean-style method naming convention for any methods in the Buffer interface (or its sub-interfaces), because these have the potential to collide with names in the generated DMO interfaces. So, for example, getField should simply be field, because there might be a database table with a field column, which would result in a conflicting getField method in the DMO interface.

The changes to attributes_methods.rules must be adjusted accordingly. BTW, please follow the convention in this rule set of adding new ftype == ... rules in alphabetical order by token type name.

Why is AbstractDynamicTempTable an abstract class? There are neither abstract methods nor unimplemented methods from the DynamicTempTable interface. What is supposed to extend it? Since it is emitted into converted business logic, I would prefer a shorter name that doesn't imply abstractness.

If I understand correctly, the flow of creating a dynamic temp-table and using it involves calling CREATE TEMP-TABLE, invoking one or more ADD/CREATE methods on it, then invoking the TEMP-TABLE-PREPARE method. Evidently, the NAME attribute can be set as well, but I'm not clear when, based on the documentation. Is the idea that we define the DMO proxy (and dynamically add the definition to Hibernate's config) during the TEMP-TABLE-PREPARE step? I know this is a runtime-related question, but I want to get my head around the intended design.

I have moved the getField() to Buffer interface and renamed it to simply field() so there would be no collisions with DMOs.
I have sorted the attributes alphabetically.

Regarding the class that implements DynamicTempTable. At moment of naming it, it did make sense to me, but now, it does't. Maybe DynamicTempTableImpl, but, again, it's rather long :(. Maybe rename the interface into DynamicTempTableInterface and naming the class DynamicTempTable ? I need help here.

• create it
• add fields/indexes (just store these internally)
• prepare (this is where temp-tables get truly created)
• naming it: The NAME attribute of the Temp-table object handle is writeable for dynamic and AVM-generated temp-tables. A new temp-table name cannot be assigned until after the TEMP-TABLE-PREPARE method has been executed.

Ovidiu Maxiniuc wrote:

Regarding the class that implements DynamicTempTable. At moment of naming it, it did make sense to me, but now, it does't. Maybe DynamicTempTableImpl, but, again, it's rather long :(. Maybe rename the interface into DynamicTempTableInterface and naming the class DynamicTempTable ? I need help here.

How about TempTable for the interface and TempTableFactory for the class? Is "factory" appropriate? Does the class need to do more than act as a mechanism to define and configure dynamic temp tables?

Eric Faulhaber wrote:

How about TempTable for the interface and TempTableFactory for the class? Is "factory" appropriate? Does the class need to do more than act as a mechanism to define and configure dynamic temp tables?

If we design this class with only the overloaded methods that create dynamically temp-tables then yes, I suppose it should be a factory. But I was thinking that this class will also handle all the other specific methods and attributes, like defining structure and populating the table. In this case, "builder" would be more appropriate.

Ovidiu Maxiniuc wrote:

In this case, "builder" would be more appropriate.

Works for me.

Does this update include all of the changes you rolled into your last (combined) update for #1668, or do I need to merge?

Code review 20130208a:

I reviewed this update in combination with your 20130201a update, which was necessary to make sure I had everything, since it appears this update is not a superset of 20130201a. It turned out to be quite confusing.

• In progress.g (0201a), you added a comment in the header that you had "changed KW_BUF_VAL's type to METH_POLY". However, that token's type is ATTR_POLY. Is the comment wrong, or is the code wrong?
• The two versions of methods_attributes.rules were very difficult to reconcile with the version currently in bzr. The version of this ruleset in the older update (0201a) seems more recent in some respects than the version in 0208a, except for the dynamic temp table content. There is a lot of content in 0201a that seems unrelated to this issue: basic handle processing, XML processing, etc. Why all this additional code? Also, the additions were still largely out of alphabetical order, making review considerably more difficult than it needed to be.
• We cannot use JavaBean-style method names in Buffer[Impl].java, as they have the potential to conflict with DMO method names.
• In discussion with Vadim, we decided that buffer field attributes and methods belong in FieldInterface/FieldReference, not Buffer/BufferImpl.
• bufferRelease belongs in Buffer, not P2JQuery.
• BufferImpl.java (0201a) has a number of new, static, create methods (there is no version of this file in 0208a). These methods are emitted by buffer_definitions.rules. However, there should be no public static methods in BufferImpl.java. It is meant to be an internal class that is never used directly from converted code, and only exists to implement methods declared in Buffer.java. In other words, there should be no methods in BufferImpl that are not in Buffer, other than possibly some private helper methods, if necessary. These static methods should be in RecordBuffer.java.

Because of all the above issues, I was unable to reconcile your 0201a and 0208a updates with each other and with the current version of the code in bzr.

As your top priority, please provide a single update which includes all of your updates related to all the issues under #1651. It should be built on top of the current version of code in bzr, plus my merged version of Vadim's changes for #1947 and #1668, which is named vmn_upd20130207b.zip (and is attached to #1947). Do not include any unrelated code which is not checked into bzr. Also, there is, unfortunately, considerable, conflicting feature overlap between what Vadim has done and what you added. I already reviewed/merged/approved Vadim's changes, so please do not replace any of that new work with yours where there are conflicts.

We need this update as soon as possible. Vadim is continuing to make changes for #1668, using vmn_20130207b.zip as a baseline, and he will need to merge with your changes very soon.

Ovidiu Maxiniuc wrote:

I did that. Please see the attached update. I removed any unrelated code, only bzr + vmn_upd20130207b + things related to creating temporary tables.

This update seems much cleaner and easier to review, thank you. However, please note that I asked for all updates related to all the issues under #1651 (the parent issue for all the dynamic database work you have been doing), not just the things related to temporary tables. This must include all CREATE QUERY work, all CREATE BUFFER work, all CREATE TEMP-TABLE work, and all the methods and attributes you had to implement to for these (except where they overlap/conflict with work already in vmn_upd20130207b.zip). I'm sorry if this was not clear.

Ovidiu Maxiniuc wrote:

I merged the changes from all CREATE TEMP-TABLE / BUFFER / QUERY tasks using the last vmn_upd20130211a.zip as base. However, this update seems not to be complete, some of the rules refer some classes that do not exist in the package. I hope you have them all in order to obtain a compilable project.

Yes, please see #1947, note 44 -- I added a correction shortly after posting that entry.

Code review 20130212a:

Everything here looks good, except we are missing updated versions of AbstractQuery and QueryWrapper to match the expanded P2JQuery interface. Right now, it won't compile without these classes (even with Costin's update). Please add these missing files and resubmit the update ASAP.

That's strange, the files I sent DefaultDelegate inner class had the new method implemented (they were returning nulls). I compiled and converted the testcases. Might be lost in the merging process !?...

However, there is a small issue with the implementation of BUFFER-COPY (KW_BUF_COPY). It's implemented only in the basic form, without optional parameters. I believe the method should be overloaded to support the additional parameters, not only the first one: BUFFER-COPY ( source-buffer-handle [ , except-list [ , pairs-list [ , no-lobs ] ] ] )

[LaterEdit: There are two minor warnings, javadoc will complain in a couple of places that the string parameter does not match the predicate form javadoc comment.]

Ovidiu Maxiniuc wrote:

That's strange, the files I sent DefaultDelegate inner class had the new method implemented (they were returning nulls). I compiled and converted the testcases. Might be lost in the merging process !?...

You are right! My mistake during the merge over here, sorry.

BTW, please don't return null from DefaultDelegate methods. Just return a safe value of the correct data type which would make sense for an uninitialized query (i.e., one that was declared, but not properly defined/prepared yet). Generally, that will be false for logical return types that indicate success, or unknown value or some other sensible no-op result for other return types.

However, there is a small issue with the implementation of BUFFER-COPY (KW_BUF_COPY). It's implemented only in the basic form, without optional parameters. I believe the method should be overloaded to support the additional parameters, not only the first one: BUFFER-COPY ( source-buffer-handle [ , except-list [ , pairs-list [ , no-lobs ] ] ] )

[LaterEdit: There are two minor warnings, javadoc will complain in a couple of places that the string parameter does not match the predicate form javadoc comment.]

Agreed, please fix these issues and resubmit only the affected files, as a separate update. We are going to try to get the existing update into regression testing as is. I don't want to hold it back for another day.

Note that the defaultBufferHandle API I think it needs to be added to the Buffer interface too, because there are compile errors in the server projects, as it can not find the TempTablex.Buf.defaultBufferHandle method

Update has passed conversion regression test and is committed to bzr rev. 10228.

• name conversion, to ensure table, field, and index names are legal/valid in the P2J environment;
• the generation of a .dict AST, such that dynamic query conversion can incrementally load new temp-table definitions into a SchemaDictionary instance when converting WHERE clauses for dynamically prepared queries.

I'm not sure yet as to how we deal with the second piece. It depends on the scope/lifetime of a dynamically prepared temp-table. If it cannot survive the current, external procedure, then we can store it in memory, and a dynamically prepared query in the same procedure can share access to this data structure. Otherwise, we will need to persist it (is there an existing metadata table for this?), and clean it up at an appropriate time. This will require some investigation.

In order to use a dynamically prepared temp-table with the existing infrastructure to execute queries and extract data from the results, we will need to dynamically generate a DMO interface (for dynamic proxy creation) and a DMO implementation class (for Hibernate's use).

We will want to remove the dynamically generated DMO interface and implementation class from the associated class loader when they are no longer in use, to prevent an ongoing memory leak. I've looked at the com.goldencode.p2j.classloader package, and I don't think these loaders are suited for this, so we will need something new.

One concern I have is whether we can properly clean up a temp-table that is no longer in use from Hibernate. Can we remove a class-table mapping from the session factory? Also, Hibernate instruments the implementation class -- what are the class unloading implications of this?

It looks like the single way to update ORM at runtime is to create/update Configuration object and build a new SessionFactory. Besides that creation of a SessionFactory is an expensive operation, we will have difficulties with switching to the new factory. So if we want true ORM for dynamic tables (theoretically we can use pure JDBC or store data as "field name" -> "value" map etc.), we should modify Hibernate SessionFactoryImpl or implement our own factory. Any better ideas?

Some thoughts:
1. About dynamic addition / removal of Hibernate entities: Eric, please see my previous note and tell what do you think is the most correct way to solve this problem?
2. As the inner representation of a temp table definition we can use an inner class-container inside TempTableBuilder for fields and can re-use P2JIndex and P2JIndexComponent for indexes.
3. In order to get the information about tables/fields passed to addFieldLike/addIndexLike/etc. we probably should use Hibernate Configuration object.

Stanislav Lomany wrote:

It looks like the single way to update ORM at runtime is to create/update Configuration object and build a new SessionFactory. Besides that creation of a SessionFactory is an expensive operation, we will have difficulties with switching to the new factory. So if we want true ORM for dynamic tables (theoretically we can use pure JDBC or store data as "field name" -> "value" map etc.), we should modify Hibernate SessionFactoryImpl or implement our own factory. Any better ideas?

After researching for a while, no better ideas come to mind. Upon closer inspection, the org.hibernate.cfg.Mappings class I was placing my hope in seems to be about dynamically changing the Configuration, but I guess you still have to create a SessionFactory after making runtime changes. As you say, switching to a new factory while other threads may still be using the old one appears to come with some problems.

How do you see the pure JDBC or name/value mapping working, specifically with regards to cooperating with all the 4GL-compatibility machinery already in the P2J persistence runtime?

If we were to go the SessionFactoryImpl modification or implementation route, what would our version do differently, specifically, to solve this problem?

How do you see the pure JDBC or name/value mapping working, specifically with regards to cooperating with all the 4GL-compatibility machinery already in the P2J persistence runtime?

1. Pure JDBC mapping probably will not work - if we want to coordinate with Hibernate state, we need to attach the entity to a session, which is not possible if we do not have a mapping for the entity.
2. For name/value mapping - we have to implement proper getting of field handles, buffer copying / comparison, changing dynamic queries on fly and other things. Doesn't seem to be an elegant solution.

If we were to go the SessionFactoryImpl modification or implementation route, what would our version do differently, specifically, to solve this problem?

First of all, the fact that worries me the most that there can be some reason why was session factory has been made immutable.
Specifically - it's hard to say without practice, but at least we have to update Hibernate mappings, persisters and identifier generators when a new DMO class is registered or removed.

It looks like re-implementing/replacing SessionFactoryImpl is a tricky job, but there are some factors that limit our exposure.

Let's look at this problem as replacing the single, immutable SessionFactoryImpl with a subclass that has minimal changes, keeping the "singleton-ish" aspect of SessionFactory (i.e., one SessionFactory per database, never to be replaced at runtime). Rather than replacing the SessionFactory instance on the fly, we want to make a new implementation that is "partially mutable". This would only be used for the temporary table database; we want to continue to use the standard SessionFactoryImpl for permanent databases.

We only care about adding (and later deleting) dynamically generated, temporary tables that exist in a single user's context. These will never be seen by other sessions, so we should add a facility to our SessionFactoryImpl subclass that keeps track of per-context, "extra" mappings, in addition to the standard, static mappings.

Hibernate uses the SessionFactoryImplementor interface to maintain an internal contract among Hibernate packages as to what SessionFactory implementations must do, but it is not consistently used throughout Hibernate. It appears that although they went through the trouble of creating this interface and using it for most internal references, they did not make the choice of implementation configurable.

And while most places in Hibernate reference this interface instead of the concrete SessionFactoryImpl class, there are still 45 direct references to SessionFactoryImpl in Hibernate 4.1.8. Some of these are JavaDoc references, others are casts, and some are more problematic. For example, Configuration.buildSessionFactory actually instantiates SessionFactoryImpl directly. We would need to change this to make SessionFactoryImpl the default, but allow it to be configurable. There are probably other references that will be similarly problematic.

SessionFactoryImpl currently is final. I suggest subclassing it, to avoid having to change all the places in Hibernate which cast to a SessionFactoryImpl instead of SessionFactoryImplementor. However, if we are to offer our changes back to the Hibernate project, a more acceptable approach is probably to fix up all these casts to use the interface instead (unless they specifically require SessionFactoryImpl methods which are not part of the SessionFactoryImplementor interface -- I haven't looked at them that closely).

It will be unavoidable to make changes in Hibernate code, like removing the final modifier from SessionFactoryImpl and making the creation of a SessionFactoryImplementor concrete class configurable in Configuration.buildSessionFactory. However, we want to minimize these changes.

Please investigate the feasibility of this approach and post your findings as you go, rather than waiting until you have all the answers. Be specific, pointing me at the things you find. I want this to be a running design discussion.

Another factor that may simplify making our partially-mutable SessionFactoryImplementor is the fact that we know the limits of the dynamic temp tables that we will create. We don't have to handle every possible association between tables. The only association we will support is the "normalization" of extent fields into a separate table. We do this as a Hibernate list association, using an internal composite-element. For example, in the p2j_test database, a Person DMO has such an association:

    <!-- List of composite elements -->
    <list access="field" cascade="evict" lazy="true" name="composite5" table="person__5">
      <!-- Second level cache -->
      <cache usage="nonstrict-read-write"/>
      <key>
        <column index="person__5_fkey" name="parent__id"/>
      </key>
      <index column="list__index"/>
      <composite-element class="PersonImpl$Composite5">
        <property column="schedule" name="schedule" type="p2j_character"/>
        <!-- Hours -->
        <property column="hours" name="hours" type="p2j_integer"/>
      </composite-element>
    </list>

It appears a lot of the up-front work SessionFactoryImpl does is the compilation of the many types of associations Hibernate handles. Hopefully, limiting those we need to support in our context-local "extension" mechanism to the above will limit our pain.

Note also that the current project calls for replacing this association with an array column, so please give some consideration to that as an additional, not-too-distant future mapping requirement.

To clarify my last post, there will be no ORM associations allowed between dynamically generated temp-tables and existing, statically defined temp-tables. So, hopefully we can avoid whatever compilation/sweep Hibernate would normally do to figure out those cross-table associations, when we add new, context-local table mappings.

It will be unavoidable to make changes in Hibernate code, like removing the final modifier from SessionFactoryImpl and making the creation of a SessionFactoryImplementor concrete class configurable in Configuration.buildSessionFactory. However, we want to minimize these changes.

Please investigate the feasibility of this approach and post your findings as you go, rather than waiting until you have all the answers. Be specific, pointing me at the things you find. I want this to be a running design discussion.

For the most part references to SessionFactoryImpl can be replaced with references to SessionFactoryImplementor with the following exceptions:
1. Static function SessionFactoryImpl.deserialize. It doesn't use the internal state of SessionFactoryImpl and we can leave it as is.
2. Hard-coded SessionFactoryImpl instantiation in Configuration and SessionFactoryBuilderImpl. We can define a configuration property which specifies the factory class and change the instantiation code appropriately. SessionFactoryBuilderImpl is not used in our case.
3. SessionFactoryImpl.getTransactionEnvironment used in AbstractSessionImpl.getTransactionEnvironment - it uses internal state of the factory and I suggest to add this function to the SessionFactoryImplementor interface.

This is a good start. The following lines in the test app concern me:

// add entity to the already created session factory
configuration.addFile(new File("Tt.hbm.xml"));
configuration.buildMappings();
PersistentClass pc = configuration.getClassMapping("Tt");
((MutableSessionFactoryImpl) sessionFactory).add(pc, configuration);

Specifically, I imagine the configuration.buildMappings() call can be quite expensive with a large number of existing mappings. Does this recompile all mappings -- old and new -- each time? I guess it would, since an incremental use case was not envisioned during the original design. Can you please test this with a scenario which adds a single table atop a large number of existing mappings? We need to know the cost of this operation in a realistic environment.

What about removing a mapping, once we're done with it? Since we will be adding many mappings over the uptime of the server, it would be very good to be able to purge them when they're no longer needed.

Specifically, I imagine the configuration.buildMappings() call can be quite expensive with a large number of existing mappings. Does this recompile all mappings -- old and new -- each time? I guess it would, since an incremental use case was not envisioned during the original design. Can you please test this with a scenario which adds a single table atop a large number of existing mappings? We need to know the cost of this operation in a realistic environment.

Regarding entities definitions Configuration is designed for incremental adding (it has a queue inside). Of course there is some overhead for additional structures. In the test project initial builtSessionFactory takes 600 ms and subsequent buildMappings calls - 60 ms. I'll let you know the results with a large number of tables later.

What about removing a mapping, once we're done with it? Since we will be adding many mappings over the uptime of the server, it would be very good to be able to purge them when they're no longer needed.

I'm working on that.

For 100 tables initial builtSessionFactory takes 1500 ms and subsequent buildMappings calls - 15 ms.

I'm not so concerned about the initial time, this will be part of server startup. It is good that the incremental calls are relatively fast, but strange that the time went down from 60ms with a small number of tables to 15ms with more tables. We need it to scale well with several thousand tables.

What class of hardware were you using for these measurements?

For 5000 tables addition of a new one takes about 50 ms.

What class of hardware were you using for these measurements?

Mobile i5 @ 2.3 GHz.

Eric, how we a going to implement classes of entities for dynamic tables? I gravitate towards Javassist.

I want to be careful about adding another bytecode manipulation library to the project. We have a few options already: ASM is already in use for the com.goldencode.proxy project. Greg suggested using the same technique as he uses in the test harness project, which is to generate Java source in memory and compile it using javax.tools.JavaCompiler.

Note that we need both a DMO interface and implementation class. We also need a custom class loading and unloading strategy.

What in your opinion makes Javassist a different/better solution for our needs?

What in your opinion makes Javassist a different/better solution for our needs?

I didn't notice that we are already using ASM so I picked the one that seemed the most friendly to me.

I like Greg's idea.

Check out the harness project using bzr and look in src/com/goldencode/compile/ for the backing code. Also look at:

src/com/goldencode/harness/test/CodeTemplate.java (this actually emits the source code for a class than implements DynamicCode)
src/com/goldencode/harness/test/DynamicCode.java (the interface being implemented dynamically)
src/com/goldencode/harness/test/RunCode.java (this executes the instance of the DynamicCode class that is generated)

To see how we encode the Java inside the baselines, see the docs/harness.html#run-code in the harness project.

Let me know if you have any questions. The result should be very easy to create using this approach. It is very readable (in my opinion) and can also be debugged easily if we allow caching of the generated source code.

"The dynamic temp-table object is scoped like the buffer object. It is created in a widget pool and ends when the widget pool ends or when it is deleted with the DELETE OBJECT statement."
Do we support widget pools? DELETE OBJECT?
What will be our strategy of dropping unused dynamic tables and mappings? I suggest to delete them when a user session is finished.

We will be supporting both widget pools and the DELETE OBJECT. That work is not yet complete. See #1983.

Constantin can provide guidance on how to hook into the widget pool infrastructure. We should plan to exactly duplicate the 4GL scoping of these dynamic resources.

Since table names can be the same across multiple or even one user context, I think we should use <table name>___<unique id> format for table and entity class names, e.g. "person___1234". The only drawback I see is that we should take this into consideration when converting dynamic queries like "for each person" to HQL at runtime.

Stanislav Lomany wrote:

Since table names can be the same across multiple or even one user context, I think we should use <table name>___<unique id> format for table and entity class names, e.g. "person___1234". The only drawback I see is that we should take this into consideration when converting dynamic queries like "for each person" to HQL at runtime.

My initial thought was that our subclass/extension of SessionFactoryImpl would be context-aware (i.e., would exist in a P2J package which could use ContextLocal to contain the dynamic temp-table mappings), but after seeing your proof of concept, it seems that approach would require a lot more rework of the SessionFactory internals. So, I agree with your naming proposal.

I was working on the code generation for dynamic DMOs. At first I started with the approach that re-uses hibernate/java templates using graft function, however later I switched to the approach that completely re-uses DMO generation phase from the conversion process, i.e. runs pattern engine on the rules which:
1. generate hbm.xml
2. generate DMO class and interface JASTs
3. convert JASTs to java code
At this point it works, but I have to make the following modifications in the conversion engine:
1. Do not read configuration from cfg folder.
2. Produce in-memory ASTs or code instead of writing it to files.

Also, definition of a DMO in dmo_index.xml looks like this:

<schema impl="_temp.impl" name="_temp">
  <class interface="TempRecord1">
    <unique>
      <component name="f1"/>
      <component name="f2"/>
    </unique>
    <index name="idx__tt1_index1" unique="true">
      <column name="f1"/>
      <column descend="true" name="f2"/>
    </index>
  </class>
</schema>

This all makes sense to me, except:

3. there is no direct replacement for index section, so it looks like another bit Hibernate code modifications is required.

Today, we don't rely on the Hibernate mapping for anything related to an index, because it (at least v3.0.5) does a horrible job with multi-column indexes (which are most of our migrated indexes). Please explain this requirement in more detail.

Eric, I wasn't sure if the definitions of indexes in Hibernate are used by P2J. If they are not used - great!

I have to take a deeper look and dig into the details, but so far, nice work getting this going at runtime! Have you run a conversion-only regression test to make sure this doesn't break normal conversion?

I am a bit concerned about some of the singleton pattern changes -- for example, are there issues using ContextLocal at conversion time?

It doesn't look like you did anything with the places we write messages directly to the console during conversion. Do you just let these messages get redirected to the log?

I have to take a deeper look and dig into the details, but so far, nice work getting this going at runtime! Have you run a conversion-only regression test to make sure this doesn't break normal conversion?

Not yet, but conversion of testcases works fine

I am a bit concerned about some of the singleton pattern changes -- for example, are there issues using ContextLocal at conversion time?

At conversion time ContextLocal switches MODE_STANDALONE. Again, didn't test it completely, but works fine so far.

It doesn't look like you did anything with the places we write messages directly to the console during conversion. Do you just let these messages get redirected to the log?

Yes.

About field name validation for dynamic tables.
1. Basically you can create a field with any name, and this field can be used later thru field handle.
2. There are some limitations for names of fields which can be referenced in QUERY-PREPARE.
3. There are more limitations implied by Hibernate.
I think we should use "field1", "field2" as Hibernate names and parse queries passed to QUERY-PREPARE at run time. Eric, do you agree?

About field format validation: I've used
new NumberFormat(format);
for numeric types and
new date().toString(format);
for dates (this functions display validation errors). That doesn't completely reproduce 4GL behavior, but that seems to be all we have. Also, I haven't found any existing code that performs validation for datetime.

Eric, I was working on DEFAULT-BUFFER-HANDLE - handle to default buffer which can be used anywhere in the program after the table has been created. And I'm not sure how to handle scoping for this handle (well, to be correct the term "scoping" probably is not applicable to a handle). How should I emulate normal openScope calls? My suggestion is to make a global buffer and emulate single openScope call for the initial external procedure.

Stanislav Lomany wrote:

...
I think we should use "field1", "field2" as Hibernate names and parse queries passed to QUERY-PREPARE at run time. Eric, do you agree?

Yes.

Stanislav Lomany wrote:

Eric, I was working on DEFAULT-BUFFER-HANDLE - handle to default buffer which can be used anywhere in the program after the table has been created. And I'm not sure how to handle scoping for this handle (well, to be correct the term "scoping" probably is not applicable to a handle). How should I emulate normal openScope calls? My suggestion is to make a global buffer and emulate single openScope call for the initial external procedure.

AFAIK, the default buffer is created within the scope in which the TEMP-TABLE-PREPARE() method is called. We probably should have that method trigger the call to openScope for the associated buffer. Please try to write test cases which use DEFAULT-BUFFER-HANDLE both within and outside the scope in which TEMP-TABLE-PREPARE() is called and see if that sheds any light.

Default buffer for dynamic tables can be used anywhere, i.e. within and outside of TEMP-TABLE-PREPARE() call scope.

Then I think your suggested approach makes sense.

Code review 20130717a/b/c:

Nice work. A very interesting update.

• What is the TODO comment in DMOIndex.classes()?
• The property types in DynamicTablesHelper will need to be updated to add int64, datetime, and datetimetz.
• Please fix the spelling of dynamicDMOClases (dynamicDMOClasses) in DatabaseManager.

Please fix these items and then do a full conversion and runtime regression test. If all goes well, check it in.

What is the TODO comment in DMOIndex.classes()

Dynamic table classes are not returned by this function. Since this function is used only in registerDatabase I'll leave it as is and update javadoc.

Eric, com.goldencode.compile.* classes are from the harness project. They are now required for P2J project too. How should I handle this in bzr?

I guess we have the same files in both projects.

Greg: can you think of a better way?

For now, that is the best way. Later on, we will start to move our utility classes into separate jars, but let's keep it simple at this time.

DEFAULT-BUFFER-HANDLE will take some more time if I'm going to implement it completely, specifically it is converted incorrectly: handle.unwrapBuffer(th).defaultBufferHandle() instead of handle.unwrapTempTable(th).defaultBufferHandle() and BufferImpl.bufferCreate doesn't manage transactions correctly.

If there are known rules (transactions, undo, etc.) for buffer handles that can help me, please let me know.

Right now I'm working on the regression (single one).

Please go ahead and fix the conversion defect. AFAIK, all the information we have regarding rules about buffers is in p2j/dist/docs/api/com/goldencode/p2j/uast/package-summary.html#record_scopes. I haven't read this in a while; don't remember if buffer handles are treated there.

Eric, please note that in order to authentically implement CREATE-LIKE and ADD-LIKE-FIELD I need to have original 4GL names for tables and fields. I suggest to store them upon conversion in dmo/dmo_name_map.xml with this format:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<name-map>
   <schema name="p2j_test">
      <dmo-mapping jname="PersAddr" pname="pers-addr">
         <field-mapping jname="SiteId" pname="site-id"/>
         <field-mapping jname="EmpNum" pname="emp-num"/>
      </dmo-mapping>
   </schema>
   <schema name="_temp">
      <dmo-mapping jname="TempRecord1" pname="tt">
         <field-mapping jname="RecId" pname="rec-id"/>
      </dmo-mapping>
   </schema>
</name-map>

These names will available via an API. The code will look something like this:

MetaSchema meta = persistence.getMetaSchema();
String jNameTable = meta.getTableName(pTableName);
String jNameField = meta.getFieldName(pTableName, pFieldName);
...

I'm still working on this piece.

When distributing update I forgot to mention that if someone wants to use dynamic temp tables, InMemoryClassLoader should be used as the system one. However we already have MultiClassLoader for TIMCO jars. In order to handle this I suggest to set java.system.class.loader to InMemoryClassLoader if we are NOT planning to use MultiClassLoader OR if are planning to use it, set java.system.class.loader to MultiClassLoader and chain classloaders MultiClassLoader -> InMemoryClassLoader:

public MultiClassLoader(ClassLoader parent)
{
   super(new InMemoryClassLoader(parent));
}

Unless there are significant downsides, I think we should implement chaining of both classloaders as our default approach. The bottom line is that we don't want to have to rely upon configuration and we don't want to have to choose between the features. But if there is some significant problems it will cause, please let us know.

I have not reviewed the code yet (will do that later today). Is there anything left to do on this task (considering sub-issues #2122 and #2128), besides integrating the MetaSchema API when it's ready?

I have not reviewed the code yet (will do that later today). Is there anything left to do on this task (considering sub-issues #2122 and #2128), besides integrating the MetaSchema API when it's ready?

That is actually the old update which you already reviewed (with minor changes). I had to update baseline and regression tests were unstable so it took long before I could distribute it.

The final update (except MetaSchema and Widget pool integration) will be ready on Monday.

Constantin, Ovidiu, is there any difference between TempTable.uniqueId() and UniqueID.getUniqueID()?

I've found that CLEAR() resets UNDO attribute to true, unless the call was performed on an unprepared table with no fields. Note that the default value of UNDO is false.

If UNDO is false then all table records and all related buffers are deleted on CLEAR. If UNDO is true then the message

CLEAR method may not be called on a static temp-table object, nor on a dynamic 
one with records; use buffer:EMPTY-TEMP-TABLE. (9085)

Example:
def var th1 as handle no-undo.

create temp-table th1.
th1:add-new-field("field1", "integer").
th1:temp-table-prepare("table1").
message string(th1:undo).    /* no */
th1:default-buffer-handle:buffer-create.

message string(th1:clear()). /* clears */

th1:add-new-field("field1", "integer").
th1:temp-table-prepare("table1").
message string(th1:undo).    /* yes */
th1:default-buffer-handle:buffer-create.
message string(th1:clear()).  /* displays error */

Hmm....

protected void RecordBuffer.deleteAll()
{
      // TODO: current table lock level is queried/stored;
      initialize();

      DataModelObject dmo = getDMOProxy();
      final PreselectQuery query = new PreselectQuery(dmo, null, null, "book.bookId asc");   <- BOOK?

My mistake, will correct.

UNDO attribute can be changed (and takes effect!) after a table has been prepared. I'll leave this "feature" for future releases. Testcase:

def var th1 as handle no-undo.

create temp-table th1.
th1:add-new-field("field1", "integer").
th1:temp-table-prepare("table1").
th1:default-buffer-handle:buffer-create.

th1:undo = true.

blk:
do transaction:
   th1:default-buffer-handle:buffer-field("field1"):buffer-value = 123.
   undo blk, leave blk.
end.

message th1:default-buffer-handle:buffer-field("field1"):buffer-value. /* output is "0" */

In regard to note 89, please create a separate task for this (and include the testcase). Make sure to make the new task "related" to this one.

Stanislav Lomany wrote:

Constantin, Ovidiu, is there any difference between TempTable.uniqueId() and UniqueID.getUniqueID()?

TempTable.uniqueID should have been removed, all resources which have UNIQUE-ID attribute should implement UniqueID.

I understand for TEMP_TABLE, but why unique id for BUFFER is also ContextLocal?

Vadim Nebogatov wrote:

I understand for TEMP_TABLE, but why unique id for BUFFER is also ContextLocal?

I don't think I understand your question. The UNIQUE-ID attribute is a value associated with the resource, but the generator used to generate the IDs should be ContextLocal; BTW, if you implement UNIQUE-ID now, does 4GL follow some kind of progression when generating the UNIQUE-ID or are they random?

Constantin, you can check com.goldencode.p2j.util.UniqueIdGenerator. Vadim, generator presumes that id's are unique across a session and kind of entities (temp-tables, buffers, etc.). So, each time you start a new session ids will be TEMP-TABLE #1, BUFFER #1, TEMP-TABLE #2 etc. That's NOT always true in 4GL, but for me for the most part - yes. If you think that this way of generation break some critical constraint in 4GL - let me know.

Stas, could some conflict between ids happen with using shared buffers?

Stas, could some conflict between ids happen with using shared buffers?

UniqueIdGenerator only hands out new ids. In my tasks I'm only responsible for assigning these values to dynamic buffers and so far I've reached only default-buffer-handle. So the one who is responsible for handling ids for all other buffers should take into consideration that shared buffers keep their ids.

These names will available via an API. The code will look something like this:

MetaSchema meta = persistence.getMetaSchema();
String jNameTable = meta.getTableName(pTableName);
String jNameField = meta.getFieldName(pTableName, pFieldName);

Eric, in order to make ADD-*-LIKE work I need to know the following 4GL definitions for a field:
1. 4GL name;
2. data type;
3. extent size;
4. initial value;
5. 4GL format;
6. label;
7. column label.

Which of these attributes will be available thru MetaSchema? Obviously I can get 2, 3 and 4 using other means.

All of these will be available via the MetaSchema API, though the actual metadata tables (e.g., _file, _field, etc.) only support permanent tables. We will have to come up with an alternate implementation to store and expose this information for temp-tables, but I still expect to use the MetaSchema API for this.

And what about index information - list of indexes with attributes: primary, word (if we need it), unique?

We will have to come up with an alternate implementation to store and expose this information for temp-tables

Does that mean that I can leave stubs that are supposed to use MetaSchema for both permanent and temp tables?

If I could get MetaSchema interface that would allow me to implement ADD-*-LIKE functions almost completely.

Stanislav Lomany wrote:

And what about index information - list of indexes with attributes: primary, word (if we need it), unique?

Yes, this will be available via MetaSchema as well. However, again, only permanent tables will be supported by the metadata tables (e.g., _index, _index-field). We will need a different implementation for temp-tables.

We will have to come up with an alternate implementation to store and expose this information for temp-tables

Does that mean that I can leave stubs that are supposed to use MetaSchema for both permanent and temp tables?

Yes. Though the backing implementation will differ, I think it makes sense to support both through this interface.

Have you run this update through regression testing yet? If not, please do. I will review in parallel, but I don't want to hold up a first pass of testing.

• Please add a simple javadoc comment for the MutableSessionFactoryImpl constructor, and add a standard GCD file header and the standard Hibernate license/copyright text to this file. If/when we attempt to contribute this back to the Hibernate project, we will need to replace our file header with an explicit GCD copyright notice and reformat the code to be consistent with Hibernate's code style.
• Also in MutableSessionFactoryImpl, there are several TODOs. What needs to be done for these, and what is the implication of leaving them undone?
• You changed the SessionFactoryImpl internal maps we need to access dynamically from type HashMap to ConcurrentHashMap, rather than synchronizing access to all of them together in the add and remove methods in MutableSessionFactoryImpl (and wherever else they already are accessed today). I assume this approach was used to ensure existing access to these maps in SessionFactoryImpl was protected. While this approach also makes the access to these resources from multiple threads more granular and probably a bit faster, but with this granularity comes the potential for these resources to be updated out of step with one another from different threads. Have you analyzed the potential for problematic race conditions that might result from this approach? It seems it would be safer to have less granular synchronization to ensure these resources were always updated consistently. Do you foresee a performance problem with this alternate approach?

Eric Faulhaber wrote:

Code review 20130924b:

• Please add a simple javadoc comment for the MutableSessionFactoryImpl constructor, and add a standard GCD file header and the standard Hibernate license/copyright text to this file. If/when we attempt to contribute this back to the Hibernate project, we will need to replace our file header with an explicit GCD copyright notice and reformat the code to be consistent with Hibernate's code style.
• Also in MutableSessionFactoryImpl, there are several TODOs. What needs to be done for these, and what is the implication of leaving them undone?
• You changed the SessionFactoryImpl internal maps we need to access dynamically from type HashMap to ConcurrentHashMap, rather than synchronizing access to all of them together in the add and remove methods in MutableSessionFactoryImpl (and wherever else they already are accessed today). I assume this approach was used to ensure existing access to these maps in SessionFactoryImpl was protected. While this approach also makes the access to these resources from multiple threads more granular and probably a bit faster, with this granularity comes the potential for these resources to be updated out of step with one another from different threads. Have you analyzed the potential for problematic race conditions that might result from this approach? It seems it would be safer to have less granular synchronization to ensure these resources were always updated consistently. Do you foresee a performance problem with this alternate approach?

• Also in MutableSessionFactoryImpl, there are several TODOs. What needs to be done for these, and what is the implication of leaving them undone?

We do not use natural ids, second-level cache for temp-tables and concurrent access for temp-tables, so these parameters are correctly null. But potentially for a general-purpose mutable session factory they shouldn't be null.

• You changed the SessionFactoryImpl internal maps we need to access dynamically from type HashMap to ConcurrentHashMap, rather than synchronizing access to all of them together in the add and remove methods in MutableSessionFactoryImpl (and wherever else they already are accessed today). I assume this approach was used to ensure existing access to these maps in SessionFactoryImpl was protected. While this approach also makes the access to these resources from multiple threads more granular and probably a bit faster, but with this granularity comes the potential for these resources to be updated out of step with one another from different threads. Have you analyzed the potential for problematic race conditions that might result from this approach? It seems it would be safer to have less granular synchronization to ensure these resources were always updated consistently. Do you foresee a performance problem with this alternate approach?

I didn't want to completely stop Hibernate processing while a new entity is added. Besides concurrent maps, addition of new entity is synchronized externally in DynamicTablesHelper. If you think that full synchronization is the safe approach, I can fully synchronize access to the updated maps. Hibernate code is complicated and it's hard to predict all dependencies, but new entity classes are not accessed until they were completely added and removed only after all references to them have been cleaned up.

OK, let's leave the synchronization as is.

Code review 20130924a:

Wow, this is an exciting update and a hugely important area of functionality. Nice work! I would like to see your test cases. Are these checked into the testcases project? Please provide a list of names, or upload them here.

• Suggestion for a minor improvement in UniqueIdGenerator: in the context anonymous inner class' initialValue implementation, iterate the IdKind enum instead of the explicit put calls, as in for(IdKind kind : IdKind.values()) { map.put(kind, new integer(1)); }. Then, we only need to update the IdKind enum as we add new types.
• It appears the only class from which MetaSchema will be used is TempTableBuilder. Please list here the signatures you would like to see added to the MetaSchema interface to get the data you need, for each such pending dependency.
• Please merge with the latest version of BufferManager, which has been committed in the meantime (please fix the header numbering as part of this change -- I missed the double use of 71 in my last review of this class).
• The singleton implementation in InMemoryClassLoader is not threadsafe. Won't the constructor and getSystemInMemoryClassLoader method can be called on multiple threads?

How did the first round of regression testing turn out?

I would like to see your test cases. Are these checked into the testcases project? Please provide a list of names, or upload them here.

I've committed them in bzr: dt-*.p

• Suggestion for a minor improvement in UniqueIdGenerator: in the context anonymous inner class' initialValue implementation, iterate the IdKind enum instead of the explicit put calls, as in for(IdKind kind : IdKind.values()) { map.put(kind, new integer(1)); }. Then, we only need to update the IdKind enum as we add new types.

OK

• It appears the only class from which MetaSchema will be used is TempTableBuilder. Please list here the signatures you would like to see added to the MetaSchema interface to get the data you need, for each such pending dependency.

boolean tableExists(String tableName4GL)

String get4GLTableName(Class dmoIface)
String get4GLTableName(String tableName4GL) //get table name in original letter case

P2JField getExistingField(Class dmoIface, String fieldNameHibernate)
P2JField getExistingField(String tableName4GL, String fieldName4GL)

P2JIndex getExistingIndex(Class dmoIface, String indexName4GL)
P2JIndex getExistingIndex(String tableName4GL, String indexName4GL)

Collection<P2JIndex> getExistingIndexes(Class dmoIface)
Collection<P2JIndex> getExistingIndexes(String tableName4GL)

Collection<P2JField> getExistingFields(Class dmoIface)
Collection<P2JField> getExistingFields(String tableName4GL)

Note that 4GL table/field/index name parameters can be in arbitrary form (arbitrary case, may have extra trailing spaces).

• Please merge with the latest version of BufferManager, which has been committed in the meantime (please fix the header numbering as part of this change -- I missed the double use of 71 in my last review of this class).

OK

• The singleton implementation in InMemoryClassLoader is not threadsafe. Won't the constructor and getSystemInMemoryClassLoader method can be called on multiple threads?

InMemoryClassLoader(ClassLoader parent) is called only if InMemoryClassLoader is in the chain of system class loaders, i.e. it is called before normal classes are loaded.

How did the first round of regression testing turn out?

Successfully.

OK, please run it through another round of regression testing after you've made the above changes. Since they are pretty straightforward, I don't need to review it again, unless you end up changing more than anticipated. If all goes well, please commit and distribute it.

20131002a changes look good. Please commit and distribute if testing passes.

I was pretty sure that extents work normally for dynamic tables, there are several checked in testcases for them. However I've encountered two problems:
1. Composite elements are not loaded when they are iterated by dynamic queries. I've fixed it by modifying Hibernate jar and post the update shortly.
2. Extent fields cannot be referenced in where clause, see testcase dbuf-query-extent1-dt.p in bzr. Error text:

[01/23/2014 19:54:50 TMT] (com.goldencode.p2j.persist.Persistence:WARNING) [00000001:00000007:syman-->local/_temp/primary] error executing query [select tt__1.id from DynamicRecord1Impl as tt__1 where ((tt__1._multiplex = ?) and (tt__1.field1[2] = 3)) order by tt__1._multiplex asc, tt__1.id asc]

Is there a stack trace that goes with this error that shows the problem at the Hibernate level?

Hibernate cannot deal with the above HQL directly. We re-write the where clauses for such queries (i.e., those that reference specific extent fields) in the HQLPreprocessor, into HQL that Hibernate can deal with. Are dynamic queries possibly short-circuiting the HQLPreprocessor?

Is there a stack trace that goes with this error that shows the problem at the Hibernate level?

Caused by: java.lang.NullPointerException
        at org.hibernate.persister.collection.AbstractCollectionPersister.initialize(AbstractCollectionPersister.java:678)
        at org.hibernate.event.internal.DefaultInitializeCollectionEventListener.onInitializeCollection(DefaultInitializeCollectionEventListener.java:80)
        at org.hibernate.internal.SessionImpl.initializeCollection(SessionImpl.java:1804)
        at org.hibernate.collection.internal.AbstractPersistentCollection$4.doWork(AbstractPersistentCollection.java:549)
        at org.hibernate.collection.internal.AbstractPersistentCollection.withTemporarySessionIfNeeded(AbstractPersistentCollection.java:234)
        at org.hibernate.collection.internal.AbstractPersistentCollection.initialize(AbstractPersistentCollection.java:545)
        at org.hibernate.collection.internal.AbstractPersistentCollection.read(AbstractPersistentCollection.java:124)
        at org.hibernate.collection.internal.AbstractPersistentCollection$1ExtraLazyElementByIndexReader.doWork(AbstractPersistentCollection.java:339)
        at org.hibernate.collection.internal.AbstractPersistentCollection.withTemporarySessionIfNeeded(AbstractPersistentCollection.java:234)
        at org.hibernate.collection.internal.AbstractPersistentCollection.readElementByIndex(AbstractPersistentCollection.java:347)
        at org.hibernate.collection.internal.PersistentList.get(PersistentList.java:293)
        at com.goldencode.p2j.persist.dynamic._temp.impl.DynamicRecord1Impl.getField1(DynamicRecord1Impl.java:116)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:601)
        at com.goldencode.p2j.persist.RecordBuffer$Handler.invoke(RecordBuffer.java:9606)
        at com.goldencode.p2j.persist.$__Proxy2.getField1(Unknown Source)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:601)
        at com.goldencode.p2j.persist.FieldReference.getObject(FieldReference.java:842)
        at com.goldencode.p2j.persist.FieldReference.get(FieldReference.java:568)
        at com.goldencode.p2j.persist.FieldReference.value(FieldReference.java:1090)
        at com.goldencode.p2j.persist.FieldReference.value(FieldReference.java:1038)
        at com.goldencode.p2j.persist.FieldReference.value(FieldReference.java:1020)
        at com.goldencode.testcases.Test3$1.body(Test3.java:46)

The fix is:

java.util.Set<CollectionPersister> collectionPersistersToProcess = new HashSet<CollectionPersister>(); //ADDED
while (iter.hasNext())
{
     Property property = iter.next();
     if (property.getType().isCollectionType())
     {
        Collection collection = (Collection) property.getValue();
        CollectionPersister persister = persisterFactory.createCollectionPersister(
                                                                 configuration,
                                                            collection,
                                                        null, //TODO accessStrategy?
                                                    this);
        collectionPersistersToProcess.add(persister);    //ADDED
        collectionPersisters.put(collection.getRole(), persister);
     }
   }

   for (CollectionPersister persister : collectionPersistersToProcess)    //ADDED
   {
      persister.postInstantiate();
   }

Hibernate cannot deal with the above HQL directly. We re-write the where clauses for such queries (i.e., those that reference specific extent fields) in the HQLPreprocessor, into HQL that Hibernate can deal with. Are dynamic queries possibly short-circuiting the HQLPreprocessor?

The second error doesn't relate to the first error and probably needs a fix in DynamicQueriesHelper. The cause is:

org.hibernate.QueryException: could not resolve property: field1 of: com.goldencode.p2j.persist.dynamic._temp.impl.DynamicRecord1Impl [select tt__1.id from com.goldencode.p2j.persist.dynamic._temp.impl.DynamicRecord1Impl as tt__1 where ((tt__1._multiplex = ?) and (tt__1.field1[2] = 3)) order by tt__1._multiplex asc, tt__1.id asc]

Regarding the second error, this query is generated by the conversion rules for historical reasons, when we handled some extent fields as simple Lists (for which Hibernate parses and handles the array dereferencing syntax field[N]):

select tt__1.id
from com.goldencode.p2j.persist.dynamic._temp.impl.DynamicRecord1Impl as tt__1
where ((tt__1._multiplex = ?) and (tt__1.field1[2] = 3))
order by tt__1._multiplex asc, tt__1.id asc

The above HQL should never reach Hibernate in this form. Clearly it has, since the error we are getting is org.hibernate.QueryException.

We seem to be bypassing the HQLPreprocessor. I don't see a dbuf-query-extent1-dt.p program in the testcases project. But assuming field1 has extent N, the HQLPreprocessor would rewrite the query as follows before passing it on to Hibernate:

select tt__1.id from com.goldencode.p2j.persist.dynamic._temp.impl.DynamicRecord1Impl as tt__1
join.tt__1.compositeN as tt__1_compositeN
where ((tt__1._multiplex = ?) and (tt__1_compositeN.field1 = 3 and index(tt__1_compositeN) = 2))
order by tt__1._multiplex asc, tt__1.id asc

If we are bypassing the HQLPreprocessor, we are going to have other problems too, since we do a lot of other required fix-ups in this class.

• there may be dependencies inside this class on static data structures which have not been updated with dynamic table data (needs investigation);
• we may want to avoid bloating the HQLPreprocessor cache with instances for dynamic queries, due to their transient nature.

Regarding the first error, is it actually necessary to modify Hibernate? I don't know where the code you modified above resides in Hibernate, but I'm concerned this will always load collections when the main record is loaded, which is what I wanted to avoid by normalizing extent fields in the first place.

Can't we accomplish the same thing if we do Session.buildLockRequest(LockOptions.NONE).lock(record) at the appropriate time (i.e., at the moment we know we need to access the extent field value)? In fact, I tried to do something like this already in the RecordBuffer.Handler.invoke method (around line 9418), but I don't think it's working as I expected, because the cascade attribute for the list of composite-elements in the entity's Hibernate mapping would have to be changed from "evict" to "lock, evict".

• Is the import com.goldencode.p2j.convert.*; statement needed in TemporaryBuffer?
• The TODO comment in DynamicTablesHelper.get4GLTableName is a bit misleading, in that it still references MetaSchema. More likely, we will store a case-sensitive, original table name annotation during conversion and carry it forward as a DMO Java annotation for use here, or change the existing legacy name annotation to be mixed case and fix up the places that rely on it being lowercased today.
• I did not cross-reference with Constantin's interim fixes for TempTableBuilder, but it looks from memory like you included those, correct?

Looks good so far. So the remaining items are the related-buffer TODOs in hasRecords and forceClear, correct?

Is there anything left to do in terms of assuring proper field order in a CREATE table LIKE other-table scenario? I saw you added the support method TempTableBuilder.getOrderedPropertyNames and use that from TemporaryBuffer.copyAllRows now. Is there anything left to do for this item?

Please address all remaining items, then regression test ASAP.

Regarding the first error, is it actually necessary to modify Hibernate? I don't know where the code you modified above resides in Hibernate, but I'm concerned this will always load collections when the main record is loaded, which is what I wanted to avoid by normalizing extent fields in the first place.

That code is from MutableSessionFactoryImpl.add(PersistentClass persistentClass, Configuration configuration). Our session factory mimics the addition of a persistent class in the original Hibernate session factory. And postInstantiate is the call in the original factory which I've missed. And it does exactly what I needed - creates a collection initializer which absence causes NPE (obviously static tables get their collection initializers from the very beginning).

OK, makes sense. I was understanding this to be a different problem.

• Is the import com.goldencode.p2j.convert.*; statement needed in TemporaryBuffer?

That is required for ParmType.

• I did not cross-reference with Constantin's interim fixes for TempTableBuilder, but it looks from memory like you included those, correct?

Yes.

Looks good so far. So the remaining items are the related-buffer TODOs in hasRecords and forceClear, correct?

Not quite. What is left:
1. COPY-TEMP-TABLE: error handling
2. COPY-TEMP-TABLE: implement methods for static temp-table
3. COPY-TEMP-TABLE: copy modes: append-, replace- and loose-copy-
4. Minor TODOs in TempTableBuilder.hasRecords and forceClean.
5. Finish with no-op methods that raise error in StaticTempTable.
Except #3 that are no-problem tasks.

Is there anything left to do in terms of assuring proper field order in a CREATE table LIKE other-table scenario? I saw you added the support method TempTableBuilder.getOrderedPropertyNames and use that from TemporaryBuffer.copyAllRows now. Is there anything left to do for this item?

Stanislav Lomany wrote:

3. COPY-TEMP-TABLE: copy modes: append-, replace- and loose-copy-

I think append- is the only option in scope here, since it can be used in internal code paths. The replace- and loose-copy options are not in use. Unless it is trivial to implement them while implementing the append option, please open a separate task to track these options for future development.

I think append- is the only option in scope here, since it can be used in internal code paths. The replace- and loose-copy options are not in use. Unless it is trivial to implement them while implementing the append option, please open a separate task to track these options for future development.

Append is not THAT append that we used to. Please look and the new copyAllRows definition. I've called this append mode skipUnique for more clarity (as it seems to me).

    * @param   append
    *          true to add new records without affecting existing records;
    *          false to remove all existing records before the copy takes place.
    * @param   skipUnique
    *          Corresponds append-mode in COPY-TEMP-TABLE. If there is a unique index on the
    *          target temp-table and a row with a duplicate key is found, the row is not replaced.
    *          This parameter is ignored if replacePrimary is true.

OK, that's a bit confusing, but got it. Still, please open a separate task to track the features needed for full compatibility with the COPY-TEMP-TABLE method. Just make it a placeholder for now, you can fill in the details after we finish M7.

Stas, I looked around this task history and that of related tasks, but I can't seem to find a summary of the configuration changes needed to get dynamic temp-tables working. I've added symlinks for the cfg and data directories into my server directory (required for dynamic queries). How do you handle the need for the runtime conversion to find/access rule-sets? Did you change the classpath in server.sh, or copy directories around, or create additional symlinks, or add them to p2j.jar (I didn't see an updated build.xml), or ...?

Please provide a summary of all changes needed to the default, development, project environment to support dynamic temp-tables.

The following files and directories have to be accessed from the server classpath:

data
cfg
rules
name_map.cache
hibernate-mapping-3.0.dtd (if local dtd file is linked in hibernate template)

We have to solve this for testcases project.

As mentioned in #1651 note 5:

7. Long term, we want the standard rules to be loaded from the p2j.jar instead of from the file system.
8. Long term, we want the customer-specific rules and the customer-specific project configuration (e.g. p2j.cfg.xml) to be loaded from the customer's application jar.

Eric: please add a task to M11 to:

1. Eliminate dependencies where easily possible.
2. Build required dependencies the jars.
3. Load these dependencies from the jars.