FWD » Core Development » Database

The following discussion between Constantin and myself is copied from another task...

Constantin wrote:

Eric, did you notice the time spent during temp-table output parameter copying, during profiling? I wonder if instead of iterating and copying each row, why not just do 'insert into select from'? Aren't the source and destination temp-tables already supposed to be type compatible? Do we really need to validate each record being copied into the destination table - can this have different unique indexes which may be violated during copy?

In any case, if the source and destination unique indexes match, and the copying is OUTPUT (so the target table is empty), then 'insert into select from' should have no problem working.

Eric wrote:

Constantin Asofiei wrote:

Eric, did you notice the time spent during temp-table output parameter copying, during profiling? I wonder if instead of iterating and copying each row, why not just do 'insert into select from'? Aren't the source and destination temp-tables already supposed to be type compatible? Do we really need to validate each record being copied into the destination table - can this have different unique indexes which may be violated during copy?

In any case, if the source and destination unique indexes match, and the copying is OUTPUT (so the target table is empty), then 'insert into select from' should have no problem working.

I had considered this, but I haven't moved forward for a few reasons:

• It is valid and seems common to have different temp-tables for source and destination. This is why we have the simpleCopy flag in TemporaryBuffer.copyAllRows.
• Pre-validation of each record is necessary. If we leave it to the database to reject unique constraint violations on insert, we lose the connection, which destroys all temp-tables in the user's session and is catastrophic. By pre-validating, we detect the error without breaking the connection and the error is recoverable.

We could get past the first issue with some clever insert statement construction, but the second is a showstopper.

Constantin wrote:

Eric Faulhaber wrote:

We could get past the first issue with some clever insert statement construction, but the second is a showstopper.

For OUTPUT case, the target will be emptied before records will be copied; and even if we are not in 'simpleCopy' mode, if the source and target unique indexes are the same (match on the same columns, in the same order), then what else remains to validate, if the target is empty?

And for undo processing, we can add a ReversibleBulkInsert.

Eric wrote:

Constantin Asofiei wrote:

Eric Faulhaber wrote:

We could get past the first issue with some clever insert statement construction, but the second is a showstopper.

For OUTPUT case, the target will be emptied before records will be copied; and even if we are not in 'simpleCopy' mode, if the source and target unique indexes are the same (match on the same columns, in the same order), then what else remains to validate, if the target is empty?

Mandatory (non-null) fields, but if that property matches between source and destination field, it should be ok to use this optimization. If we allow temp-tables to be implemented in SQL Server in the future (I'm considering enabling their use in the "main" database), we'll have index length checking to consider as well.

And for undo processing, we can add a ReversibleBulkInsert.

Hm, this brings up a good point. It seems reversibility isn't taken into account in this area at all today. Also, a full transaction commit is applied immediately, which doesn't seem right either :-(

This idea warrants further investigation.

Eric Faulhaber wrote:

• Pre-validation of each record is necessary. If we leave it to the database to reject unique constraint violations on insert, we lose the connection, which destroys all temp-tables in the user's session and is catastrophic. By pre-validating, we detect the error without breaking the connection and the error is recoverable.

Is this still valid with 4011a? If we create a savepoint before the insert and commit/rollback after it, we should be OK, right?

I'm thinking to always use INSERT INTO SELECT FROM, and if it fails, fallback to the previous record-by-record mode.

BTW, for temp-tables AFAIK there is no way to specify a mandatory field, correct?

Constantin Asofiei wrote:

Eric Faulhaber wrote:

• Pre-validation of each record is necessary. If we leave it to the database to reject unique constraint violations on insert, we lose the connection, which destroys all temp-tables in the user's session and is catastrophic. By pre-validating, we detect the error without breaking the connection and the error is recoverable.

Is this still valid with 4011a? If we create a savepoint before the insert and commit/rollback after it, we should be OK, right?

I think so.

In 4011a, we use both methods. In many cases, we use the savepoint mechanism, but we might instead use the unique index query method, if we detect that 2 or fewer unique indices are fully dirty (this heuristic may change), or if we detect that saving the record would fail because a 'not null' column (other than the column currently being updated) contains null.

I'm thinking to always use INSERT INTO SELECT FROM, and if it fails, fallback to the previous record-by-record mode.

I like this idea.

Theoretically, this could even work for disparate tables, as long as the target table has a super-set of the columns of the source table, along with compatible indices and mandatory fields. However, this is a significant complication to the implementation and probably is better attempted in a separate pass.

BTW, for temp-tables AFAIK there is no way to specify a mandatory field, correct?

A temp-table created LIKE a table with mandatory fields will have those mandatory fields.

Eric Faulhaber wrote:

Constantin, I linked this with issue #3958 because it seems we should apply some of the same ideas to both problems, if possible. This also is related to #4011-638. Have you made any changes to the code related to any of the ideas we've been discussing in any of these issues?

No, I haven't looked into it yet.

Eric, a problem INSERT INTO ... SELECT FROM is the primary key. How important is to use the 'reclaimed keys' feature? Why can't we use the H2's auto_increment for PKs?

1. DELETED rows from the source before table
2. before records from source to destination, but only for records which have a non-null _rowState.
3. extent tables

1. before the INSERT INTO ... SELECT FROM, set the sequence to nextPrimaryKey()
2. use nextval('seq_table_name') in the SELECT
3. if completed successfully, set the DMOIFace's next PK to the current sequence value
4. if the insert fails, reclaim all used keys

This works, but I'm having trouble when the insert fails because of an unique index or mandatory field. The issue here is that on the row-by-row copy, we are relying on FWD to validate the record.

But, if the persistence.executeSql fails, then the FWD orm.Session will be invalidated (which is not OK), plus, if we are in a FULL tx, I don't think we can rollback. If this is not easy to fix, then I'll use the INSERT INTO SELECT FROM only when the destination table doesn't have any unique indexes or mandatory fields.

For now, I'll look into adding support for the before-table records and extent tables.

Constantin Asofiei wrote:

Eric, a problem INSERT INTO ... SELECT FROM is the primary key. How important is to use the 'reclaimed keys' feature?

It is important in certain cases which delete records while iterating results. I don't remember the exact use case, but without reclaiming the keys, we were getting incorrect sorting results (infinite loop, IIRC), in cases where the explicit sorting was not exact enough, and we had to rely on the order of the primary keys.

What is the exact problem in the INSERT INTO ... SELECT FROM case? Maybe we can disable reclaimed keys for specific cases?

Why can't we use the H2's auto_increment for PKs?

See above.

Eric Faulhaber wrote:

What is the exact problem in the INSERT INTO ... SELECT FROM case? Maybe we can disable reclaimed keys for specific cases?

The problem is not reclaiming the keys, but if there is a mandatory field problem or unique index collision: in this case, the tx needs to be aborted, and I think even the JDBC connection needs to be re-acquired. This trashes the orm.Session and we lose the 'sessionUseCount', which can lead to aggressive JDBC connection acquire/release. Maybe it works doing this in a separate savepoint, and we commit/rollback that, but I haven't reached this point yet. I want to finish the before table and extent tables support first.

Also, it may make sense to refactor TemporaryBuffer.copyAllRows to use the RecordMeta and PropertyMeta support. And get rid of all the HashMap usage.

A small note about the reclaimed keys - for the INSERT INTO SELECT FROM approach to work, I need consecutive, increasing PKs, as I'm using some formulas to map the records in the child extent tables to their parent IDs. So I can't use the reclaimed keys in this improvement.

Constantin Asofiei wrote:

A small note about the reclaimed keys - for the INSERT INTO SELECT FROM approach to work, I need consecutive, increasing PKs, as I'm using some formulas to map the records in the child extent tables to their parent IDs. So I can't use the reclaimed keys in this improvement.

This seems appropriate. I did not mean to imply we should use reclaimed keys for this case. If they can be conditionally disabled for this case, they should be. They are only needed to prevent incorrect behavior in a very limited use case.

Argh, I haven't realized before that the sequences are global - and if we define it as seq_tt1, any user working on a session-local temp-table tt1 will have the same sequence, seq_tt1. These need to be either session-local in H2 (which requires changing H2) or their name need to be unique, for all FWD user contexts.

Constantin Asofiei wrote:

Argh, I haven't realized before that the sequences are global - and if we define it as seq_tt1, any user working on a session-local temp-table tt1 will have the same sequence, seq_tt1. These need to be either session-local in H2 (which requires changing H2) or their name need to be unique, for all FWD user contexts.

The correct approach for this is to have only one 'global' sequence per each FWD context (created and destroyed with the context), as at one point in time only a sequence can be active.

Ovidiu, please remind me - what's the default index for the before-table? I recall something like row-state being part of it...

There's an issue with using nextVal in a SELECT. Looks like the sequence values are retrieved FIRST (I don't know what order it uses here - but I hope is the PK) and only after that the records are sorted. If the PK is not used for the list of records, when nextval is used, then my approach at using sequences will not work even for the after-table.

I think the before-table records need to be recreated following an row-state, recid index - in 4GL, the PKs (after the table copy) for the before-table records are 'incremental' following this index. And in this case, as nextval is not executed on the final sorted records. I hope this can be fixed by adding an explicit index on the _rowState, recid columns.

New question: why don't we have a multiplex, recid default index for temp-tables?

Constantin Asofiei wrote:

New question: why don't we have a multiplex, recid default index for temp-tables?

We haven't needed one before now, as far as I'm aware, and additional indices cost update performance. An index on the primary key alone is enough to give us a unique, deterministic sort for any virtual temp-table.

We do add these to the end of temp-table indices, however, when the most specific component is not enough to give us a deterministic sort.

Eric Faulhaber wrote:

Constantin Asofiei wrote:

New question: why don't we have a multiplex, recid default index for temp-tables?

We haven't needed one before now, as far as I'm aware, and additional indices cost update performance. An index on the primary key alone is enough to give us a unique, deterministic sort for any virtual temp-table.

We do add these to the end of temp-table indices, however, when the most specific component is not enough to give us a deterministic sort.

Even for the select ... where _multiplex = ? order by recid, (where the table has no explicit indexes, other than the one we add for _multiplex), H2 will use the _multiplex index and after that sort the records by recid.

This is a problem - the nextval will be computed as the records are walked on the _multiplex index. I'll try using USE INDEX at the FROM <table> USE INDEX <idx_mpid_recid> to force it to use a _multiplex, recid index.

Same for the other case, when I need to walk by the _rowState - I'll need an index on the _rowState, _multiplex, recid. But for this, I think is enough to add the _rowState index (at the 4GL table definition, in FWD conversion rules, as we already augment the index with the _multiplex, recid columns) and force it via FROM <before-table> USE INDEX <idx_rowstate>.

Constantin Asofiei wrote:

Ovidiu, please remind me - what's the default index for the before-table? I recall something like row-state being part of it...

Yes, from my tests I noticed that the queries without an explicit sorting order will output the records in the order of row-state, that is, unmodified first, deleted, modified, and lastly the new created records. This is totally empirical result, based on multiple testcases. Not sure whether there is a solid ground for this.

Constantin Asofiei wrote:

H2 will use the _multiplex index and after that sort the records by recid.

This is a problem - the nextval will be computed as the records are walked on the _multiplex index.

I don't follow. Isn't this what we want? Each _multiplex value represents a distinct temp-table instance, so sorting by _multiplex, then recid seems appropriate. If your WHERE clause filters by a particular _multiplex ID, then the sort is naturally by recid, within one virtual temp-table. What sort order do you need?

Eric Faulhaber wrote:

I don't follow. Isn't this what we want? Each _multiplex value represents a distinct temp-table instance, so sorting by _multiplex, then recid seems appropriate. If your WHERE clause filters by a particular _multiplex ID, then the sort is naturally by recid, within one virtual temp-table. What sort order do you need?

1. retrieve the records using the _multiplex index - and compute the nextval for each row.
2. sort the records (explicitly, without an index) by the recid.

The row order given by the _multiplex index and the row order after the recid sort may not be the same. And this may result in the nextval sequence values to not be incremental - that's why I need a _multiplex, recid index.

For the before-table, the same - I need an index on _rowstate, _multiplex, recid.

In both cases, I need to force the SELECT to use that index.

Ovidiu Maxiniuc wrote:

Constantin Asofiei wrote:

Ovidiu, please remind me - what's the default index for the before-table? I recall something like row-state being part of it...

Yes, from my tests I noticed that the queries without an explicit sorting order will output the records in the order of row-state, that is, unmodified first, deleted, modified, and lastly the new created records. This is totally empirical result, based on multiple testcases. Not sure whether there is a solid ground for this.

From a write-xml (on the before-table for a def temp-table tt1 before-table btt1 field f1 as int) schema I get this schema:

  <xsd:schema xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns="" xmlns:prodata="urn:schemas-progress-com:xml-prodata:0001">
    <xsd:element name="btt1" prodata:proTempTable="true" prodata:undo="true">
      <xsd:complexType>
        <xsd:sequence>
          <xsd:element name="btt1Row" minOccurs="0" maxOccurs="unbounded">
            <xsd:complexType>
              <xsd:sequence>
                <xsd:element name="f1" type="xsd:int" nillable="true"/>
                <xsd:element name="__error-flag__" type="xsd:int" nillable="true" prodata:initial="prodata:unknown"/>
                <xsd:element name="__origin-rowid__" type="xsd:base64Binary" nillable="true" prodata:dataType="prodata:rowid"/>
                <xsd:element name="__error-string__" type="xsd:string" nillable="true" prodata:initial="prodata:unknown"/>
                <xsd:element name="__after-rowid__" type="xsd:base64Binary" nillable="true" prodata:dataType="prodata:rowid"/>
                <xsd:element name="__row-state__" type="xsd:int" nillable="true" prodata:initial="prodata:unknown"/>
              </xsd:sequence>
            </xsd:complexType>
          </xsd:element>
        </xsd:sequence>
      </xsd:complexType>
    </xsd:element>
    <xsd:annotation>
      <xsd:appinfo>
        <prodata:index name="rowState" prodata:primaryIndex="true">
          <prodata:table name="btt1"/>
          <prodata:field name="__row-state__"/>
        </prodata:index>
      </xsd:appinfo>
    </xsd:annotation>
  </xsd:schema>

So yes,

Well, I remember now. I had the same epiphany when I implemented write-xml and compare the xml output with information I extracted using various testcases.

Given an sql index name, how can I get its component fields, using the new RecordMeta/DmoMeta/etc support?

Or better said, how can I access the index meta for a DMO?

Constantin Asofiei wrote:

Eric Faulhaber wrote:

I don't follow. Isn't this what we want? Each _multiplex value represents a distinct temp-table instance, so sorting by _multiplex, then recid seems appropriate. If your WHERE clause filters by a particular _multiplex ID, then the sort is naturally by recid, within one virtual temp-table. What sort order do you need?

My problem is how H2 computes the nextval(seqname) for each row; it does it like this:

1. retrieve the records using the _multiplex index - and compute the nextval for each row.
2. sort the records (explicitly, without an index) by the recid.

The row order given by the _multiplex index and the row order after the recid sort may not be the same. And this may result in the nextval sequence values to not be incremental - that's why I need a _multiplex, recid index.

For the before-table, the same - I need an index on _rowstate, _multiplex, recid.

In both cases, I need to force the SELECT to use that index.

I don't know of a way to force the SELECT to use a particular index with H2. That is up to the query planner, usually based on the WHERE clause. Have you found syntax for this?

But let me take a step back, because I am becoming a bit lost in what you are trying to accomplish. I see the details, but not the big picture. Sorry, I wasn't directly involved in the temp-table copy code that was written for data set support or for output copying, so please bear with me...

I understand you are using INSERT INTO ... SELECT FROM ... However, I don't understand why the order of the new primary keys for the destination table is important. As long as the sorting of the destination table records is deterministic when issuing a query against that table with a proper ORDER BY clause, isn't that enough? Or does the "unspecified" or "natural" order of the copied records have to match those of the original table exactly for some functional reason?

Can you give me a full example of an INSERT INTO ... SELECT FROM statement? Maybe that will help me better understand why this is a problem.

Also, please point me at the code in the current implementation which you are replacing.

Eric Faulhaber wrote:

I don't know of a way to force the SELECT to use a particular index with H2. That is up to the query planner, usually based on the WHERE clause. Have you found syntax for this?

I'm using FROM <table> USE INDEX <sql-index-name> - AFAICT this forces H2 to use that index.

I understand you are using INSERT INTO ... SELECT FROM ... However, I don't understand why the order of the new primary keys for the destination table is important. As long as the sorting of the destination table records is deterministic when issuing a query against that table with a proper ORDER BY clause, isn't that enough?

1. fetches the records using the determined index (from the WHERE clause)
2. for each fetched record, evaluates all expressions in the SELECT list (like 'nextval')
3. if the used index from the WHERE clause doesn't match the ORDER BY, then it will sort the records by the ORDER BY criteria, explicitly.

If there is any inconsistency between record order as given by the index and the ORDER BY, then the computed PKs (using the nextval(sequence)) will not be incremental. I need them to be incremental so I can make some assumptions; for example, when determining the parent__id for a record in a child extent table, I can use a formula like firstDstPK + offset, where offset is the currently processed record.

And some other assumptions which only work if the SELECT gives me always incremental values for the PK.

Or does the "unspecified" or "natural" order of the copied records have to match those of the original table exactly for some functional reason?

Actually FWD has a bug in copyAllRows - 4GL follows the recid or the first index (alphabetically), when copying the records from source to destination. I'm trying to fix this, too.

Can you give me a full example of an INSERT INTO ... SELECT FROM statement? Maybe that will help me better understand why this is a problem.

The statement looks like this now (for a before-table):

insert into tt2 (recid,_multiplex,_errorFlag,_originRowid,_errorString,_peerRowid,_rowState,f_g_5,f_g_6,f_g_6_offset,f_g_1) 
select nextval('seq_1'),2,_errorFlag,_originRowid,_errorString,_peerRowid,_rowState,g_c_5,g_c_6,g_c_6_offset,g_c_1 
from tt4 
use index (idx__tt4_row_state__1) 
where _multiplex = 4 order by _rowState,_multiplex,recid

Also, please point me at the code in the current implementation which you are replacing.

See TemporaryBuffer.copyAllRows, on line 3416, the part after:

         if (!srcBuf.isTableDefinitelyEmpty())
         {

Note the the existing code will still be executed if there are unique indexes, mandatory fields, etc.

Constantin Asofiei wrote:

I need them to be incremental so I can make some assumptions;...

OK, that's what I was missing. Thanks.

The INSERT INTO ... SELECT FROM ... syntax documented for H2 suggests that the SELECT portion follows the normal SELECT syntax. Is USE INDEX the only option? Doesn't the SELECT FROM ... portion allow an ORDER BY clause?

Eric Faulhaber wrote:

The INSERT INTO ... SELECT FROM ... syntax documented for H2 suggests that the SELECT portion follows the normal SELECT syntax. Is USE INDEX the only option? Doesn't the SELECT FROM ... portion allow an ORDER BY clause?

Yes, I'm using the ORDER BY. In any case, I'm working on adding the (proper) ORDER BY clause for sorting the records (recid or first index alphabetically). Once I finish this, I'll re-test and see how it behaves without the USE INDEX.

Since we currently use an internal counter to assign the next primary key for temp-tables (or the reclaimed keys), how are you managing the potential conflict introduced by using a sequence for this case?

I originally chose the internal counter approach for performance, since using a sequence externally is slower (extra database round trip and query execution). The auto-increment sequence is not an option for the common case of creating a new record, because we potentially need to use the recid in the persistence runtime before the new record is flushed/inserted.

Eric Faulhaber wrote:

Since we currently use an internal counter to assign the next primary key for temp-tables (or the reclaimed keys), how are you managing the potential conflict introduced by using a sequence for this case?

I originally chose the internal counter approach for performance, since using a sequence externally is slower (extra database round trip and query execution). The auto-increment sequence is not an option for the common case of creating a new record, because we potentially need to use the recid in the persistence runtime before the new record is flushed/inserted.

The sequence is used like this:

      Long firstDstPK = dstBuf.nextPrimaryKey();
      persistence.executeSQL("alter sequence " + sequenceName + " restart with " + firstDstPK);
      String sqlInsert = "insert into " + dstTable + " (" + columnListDst + ")" + 
                         " select " + columnListSrc + 
                         " from " + srcTable + 
                         " use index (" + sqlIndexName + ")" +
                         " where " + MULTIPLEX_FIELD_NAME + " = " + srcBuf.getMultiplexID() + 
                         " order by " + sort;

Is used only for this specific INSERT INTO ... SELECT FROM; normal temp-table records use the internal counter, they do not rely on the H2 sequence.

1. improve RecordBuffer.copyDMO (see #4011-773)
2. refactor TemporaryBuffer.copyAllRows to use RecordMeta and others
3. TemporaryBuffer.copyAllRows - 'slow way' must use the proper index for both after and before table (bug exists in trunk).
4. TemporaryBuffer.removeRecords - before-table records are not being created (bug exists in trunk).

I'll create different tasks for the last two.

Constantin Asofiei wrote:

At this point, what is left is:

1. improve RecordBuffer.copyDMO (see #4011-773)

I need some advice on this one, so I can commit my current changes, and leave the part bellow for a second iteration.

1. refactor TemporaryBuffer.copyAllRows to use RecordMeta and others

This one I think should require refactoring all the Map<String, Method> maps to use the PropertyMeta and DataHandler instead of maps and reflection. Also, I wonder if the normal invocation in RecordBuffer.handler (for the setter/getter method invoke, at the DMO's property) could be replaced with direct access via the PropertyMeta instance and DataHandler.

Some thoughts about table copying. I know of at least 3 different copy scenarios:

1. source and target tables are the same DMO
2. source and target tables are different DMOs but they are structurally compatible (same number, type, field order and extents)
3. source and target tables are different DMOs and they are only name compatible (the order of the fields is different but the copying of fields by name works), this is the #4011-770 case)

In regard to case 3, I know it is not possible with table parameters of methods, so I wonder if it is only possible for buffer-copy?

I'm thinking that the signature string idea (#4011-774) can identify case 2.

For case 3, I wonder if a given pair of DMOs just needs a mapping of source indexes to target indexes. I guess this could be a simple int[] idxmap where each element is the target index. This means that idxmap[0] is the target element index for copying source element 0 and idxmap[n] is the target element index for copying source element n. The idea is to only have to calculate this once and then cache it as a fast path.

Some questions:

• Are there more cases?
• Which cases need to consider unique constraints?
• Which cases can possibly fire assign triggers?
• Is it correct that write triggers never fire?

Greg Shah wrote:

In regard to case 3, I know it is not possible with table parameters of methods, so I wonder if it is only possible for buffer-copy?

There are the DATASET-related cases, for FILL and such - I haven't looked at how these can be improved. Plus the loose-copy-mode in COPY-TEMP-TABLE - I need to test this.

For case 3, I wonder if a given pair of DMOs just needs a mapping of source indexes to target indexes. I guess this could be a simple int[] idxmap where each element is the target index. This means that idxmap[0] is the target element index for copying source element 0 and idxmap[n] is the target element index for copying source element n. The idea is to only have to calculate this once and then cache it as a fast path.

This mapping would be kept for each (source, target) DMO pair.

• Are there more cases?

Mandatory fields.

• Which cases need to consider unique constraints?

All of them.

• Which cases can possibly fire assign triggers?

BUFFER-COPY does fire the ASSIGN trigger.

• Is it correct that write triggers never fire?

I can't make BUFFER-COPY fire the WRITE trigger.

There are the DATASET-related cases, for FILL and such - I haven't looked at how these can be improved.

Do the DATASET scenarios have a different functionality than cases 1 - 3? Or is it just that we haven't checked?

Plus the loose-copy-mode in COPY-TEMP-TABLE - I need to test this.

Is this different than case 3? What is meant by "loose-copy-mode"?

Greg Shah wrote:

There are the DATASET-related cases, for FILL and such - I haven't looked at how these can be improved.

Do the DATASET scenarios have a different functionality than cases 1 - 3? Or is it just that we haven't checked?

I haven't checked these FILL/DATA-SOURCE/etc cases. OTOH, DATASET copy when is a parameter, will still rely on the TemporaryBuffer.copyAllRows.

Is this different than case 3? What is meant by "loose-copy-mode"?

From the docs for COPY-TEMP-TABLE and COPY-DATASET methods:

loose-copy-mode
An optional logical expression where TRUE indicates that the AVM copy the temp-table object in a loose-copy mode. That is, it relaxes the requirement that the metaschema for the source and target temp-tables be the same.
When TRUE, the AVM copies the source temp-table object to the target temp-table object based on a field mapping between the source and target temp-table buffers. If there is an attached data source with a field mapping, the AVM uses that field mapping to copy fields from the source temp-table buffer to its target temp-table buffer. If there are fields in either buffer that do not exist in the other, they are ignored. If there is no field mapping with the attached data source, or there is no attached data source, the AVM copies only those fields with the same name that appear in both the source and target temp-table metaschema.

loose-copy-mode sounds like a case 4 in which the index mapping for the source/target pair is not calculated by field names but is provided by the data source.

If there is an attached data source with a field mapping

I'm not clear on where the field mapping comes from. The DEFINE DATA-SOURCE syntax doesn't have anthing that is an obvious match.

I have some changes which enhance RecordBuffer$DatumAccess to provide more data (like the PropertyMeta, Method instance, etc), and the difference for 1 million iterations of BUFFER-COPY (on the same record) is 0.876 seconds using 'direct indexed access' to the Record.datum vs 4.562 seconds the old way.

So, bypassing the reflection and RecordBuffer$Handler is more than ~5x faster.

Now, this bypass can be done only if the two properties have the same data type and the destination property is not mandatory, indexed or has an ASSIGN trigger. I plan to find a way to cache the DatumAccess instances, too.

Note that these changes will affect the cases where the temp-table is used as a parameter and the 'bulk copy' can not be used, thus we copy 'row-by-row'.

There is a bug in dataset-handle parameter copy, when there is a before-table, too - the before-table DMO iface (backed by a dynamic temp-table) will be missing any reserved properties (like row-state and others).

I don't know if this was supposed to work in trunk. I'm working on fixing it.

3821c rev 11565 contains another pass at improving the 'field-by-field' copy. RecordBuffer$DatumAccess was enhanced to provide info about the property meta and the accessor. If that property can be 'directly accessed', it will use the property's data handler to access the record's datum (or directly via OrmUtils.set/getField).

I haven't looked yet at a 'bulk copy' of a full record to another full record, by copying the datum directly. But, from real-life apps, most tables have at least an index on it. And the approach selectively uses direct access vs reflection and RecordBuffer$Handler, depending on if the property is indexed, mandatory, with ASSIGN trigger or if the data types for the source and destinations exactly match.

Adrian: please review the RecordBuffer$ArgumentHandler change - I would have expected for buffer() method call on a proxy to give me the real RecordBuffer instance, and not another proxy.

Ovidiu/Eric: please review the other changes, too.

Adrian, thanks for the explanation. We can't make the buffer() and definition() methods non-final, as this will break other cases.

I need to fix my issue in some other way.

Review of 3821c rev 11565.

The code looks good to me. I cannot say I fully get all of it but I did a double pass over all of changes to minimize any eventual issue. But I spot none.

One of my testcases surfaced some issues related to denormalized properties, but this was out of scope of this task. I fixed it in 3821c/r11572.

Ovidiu Maxiniuc wrote:

Review of 3821c rev 11565.

The code looks good to me. I cannot say I fully get all of it but I did a double pass over all of changes to minimize any eventual issue. But I spot none.

Thanks for the review - the idea behind the changes is that for properties which have no side-effects, we can 'move' them from the source to the destination bypassing the RecordBuffer$Handler and reflection - just copy the datum from the source property to the destination property.

For properties which have side effects (mandatory, indexed, source/dest type not the same, ASSIGN trigger), we can't bypass the RecordBuffer$Handler proxy, so the old way remains. That's the DatumAccess.isDirectAccess check.

Adrian, I've backed out my ArgumentBuffer change in 3821c rev 11577 - the solution was to use getters for RecordBuffer fields, instead of direct access, to allow the proxy to dereference the proper bound buffer.

Please check with your testcases.

Constantin, the testcases work now. Unfortunately, almost every member of RecordBuffer should be retrieved through getters in order to have the proxy work. I can't find a way to enforce this through standard Java syntax, so I guess we have to live with this.

Adrian Lungu wrote:

Unfortunately, almost every member of RecordBuffer should be retrieved through getters in order to have the proxy work. I can't find a way to enforce this through standard Java syntax, so I guess we have to live with this.

Yes, that's the downside of proxying the RecordBuffer class... any instance field access outside of the 'this' instance needs to be done via getters, and not direct.

I have experimented some scenarios both in 4GL and FWD and I wanted to point out some hot-spots and maybe some optimizations for temp-table copy.

Unique constraints: The temp-table copy will never be in bulkInsert mode if the destination has at least one unique constraint, which is not present in the source. This is because some records may violate the constraint after copy. In this case, 4GL shows an error (** tt already exists with 1. (132)) and halts the copying process. The target table is filled partially. This doesn't happen in FWD: all records are copied excepting the duplicates and no error is shown.

After locally fixing FWD in order to show the error and commit partially, the time difference on a large-sized generated test is 16.1sec 4GL ~ 47.2sec FWD. Maybe we can design a smart SQL which inserts all records until the first violating record is reached. Of course, we can query the source to check if the records are already satisfying the constraint (select count(distinct f1), count(f1) from tt;) in order to allow bulkInsert, but I should test how expensive is this query.

Finally, I think that we shouldn't focus on optimizing cases which result in errors - as these are rare in real scenarios. Therefore, I recommend extending the use of bulkInsert for as many correct scenarios (without errors) as possible.

Append: I think that is a bit harsh to completely ignore the possibility of a bulkInsert in append mode. In a copy without constraints I guess that the INSERT INTO SELECT FROM should work in the same way it works in a no-append mode - maybe some issues may appear in the before and after tables? In a medium-size generated test I get this time difference: 16.9sec 4GL ~ 22.1sec FWD. I suspect that this difference can be higher on larger tests.

Insert into select from: H2 seems to allow two keywords when using INSERT INTO SELECT FROM: DIRECT and SORTED. I couldn't find a very strong documentation regarding those. Inside the H2 sources I found:

if (insertFromSelect) { // this is set when the DIRECT keyword is in place
    query.query(0, this);
} else {
    ResultInterface rows = query.query(0);
    while (rows.next()) {
        Value[] r = rows.currentRow();
        try {
            addRow(r);
        } catch (DbException de) {
        // ...
        }
    }
    rows.close();
}

if (sortedInsertMode) { // this is set when the SORTED keyword is in place
    if (!session.getDatabase().isMVStore()) {
        table.lock(session, /* exclusive */true, /* forceLockEvenInMvcc */true);
    }
    index = table.getScanIndex(session);
    index.setSortedInsertMode(true);
}

I suspect that the DIRECT keyword short-circuits the record query and insert. This is safe in our case, as we desire inserts as fast as possible - the validation is already done. However, this may have a greater impact if the database is persistent and the DIRECT keyword avoids some additional IO. But I think is worth trying it out anyways.
Also, I think that the SORTED keyword allows a more efficient way to update the current indexes - but this works only if the incoming data is already sorted. I think this is the case for FWD, but I need a confirmation on this aspect. For these keywords, I will try to find some real case scenarios, as the generated ones may be too "plain".

Eric, regarding #4011-774, I don't understand the motivation for a String representation of the table meta-structure. Is this done in order to replace the current dstBuf.getDMOImplementationClass() == srcBuf.getDMOImplementationClass()?

Continuing with finding new ways to make temp-table copy more efficient.

Insofar as this affects your design decisions, please note that we do not actually create unique indices for temp-tables at the database level. So a unique constraint cannot be enforced by H2 for legacy temp-tables, because there are no such constraints, even if the corresponding, legacy index is unique.

Each legacy temp-table index, whether it is unique or not in the legacy schema, is created without the UNIQUE keyword in H2. This is done to enable the early flushing of partially updated temp-table records to the database, in order to emulate the way such records can be found in the legacy environment with certain queries. Any legacy API which reports on index state will still treat unique indices as unique, even though they are not created that way in the H2 database.

The net of this is that we ONLY enforce legacy temp-table uniqueness through explicit record validation (i.e., by persist.orm.Validation); the database will not report an error if a duplicate (according to a legacy index) record is added to a table outside of that validation.

None of the above applies to permanent tables, which are created with proper unique indices at the database level.

Eric Faulhaber wrote:

Each legacy temp-table index, whether it is unique or not in the legacy schema, is created without the UNIQUE keyword in H2. This is done to enable the early flushing of partially updated temp-table records to the database, in order to emulate the way such records can be found in the legacy environment with certain queries. Any legacy API which reports on index state will still treat unique indices as unique, even though they are not created that way in the H2 database.

Right now I am looking into TempTableDuplicator - so only temp-table copy. I am aware of the fact that there is no UNIQUE constraints at database level. However, doing an INSERT INTO SELECT FROM, even if application-level unique constrains are in place, is wrong. The database can allow it (as no unique constrains are defined), but the FWD API shouldn't - an error needs to be displayed (** tt already exists with 1. (132)).

The net of this is that we ONLY enforce legacy temp-table uniqueness through explicit record validation (i.e., by persist.orm.Validation); the database will not report an error if a duplicate (according to a legacy index) record is added to a table outside of that validation.

This is why we need a faster validation than record-by-record validation - in order to use the power of a bulkInsert. select count(distinct f1), count(f1) from tt; is an example - taking in consideration that f1 is marked as unique in the destination temp-table. If the extracted numbers are equal, we can avoid record-by-record validation and trigger bulkInsert.

I will ignore #4055-60 for now, it doesn't fit my understanding of how copy-temp-table is working. I will document soon some new findings.

Extent: When in insertBulk mode, the extents fields are copied one by one. This can be also optimized with a insert into select from. The only motivation for record-by-record insert is the generation of PARENT__ID. I tested out a patch which generated the new PARENT__ID as PARENT__ID + firstDstPK - firstID (PARENT__ID is the parent id of the source record, firstDstPK is the primary key of the first destination parent and firstID is the primary key of the first source parent, all computed in respect to the multiplex). I am not sure if I am missing something out of this, but my generated testcases pass this. Also, some real scenario temp-table copies are working as expected. The only worry is that the source records do not have consecutive recid in some cases.

• 3821c/rev. 11753: 55.61s(large), 27.67s(medium), 22.31s(small)
• patched 3821c/rev. 11753 with insert into select from for extent fields: 51.52s(large), 28.34s(medium), 24.63s(small)
• patched 3821c/rev. 11753 and direct + sorted keywords: 43.39s(large), 26.03s(medium), 22.75s(small)
• 4GL: 27.35s(large), 12.45s(medium), 11.24s(small)

It looks like the insert into select from for extent fields optimization is really worth only when using the direct and sorted keywords. However, the times are not even near the 4GL time. This means there is still room for improvement in this section, but the first step is clearly the use of insert into select from. Unless a test which breaks the implementation shows up, I will commit the changes.

• 3821c/rev. 11753: 10.31s(large), 4.63s(medium), 3.65s(small)
• 3821c/rev. 11753 with direct keyword: 10.16s(large), 4.67s(medium), 3.65s(small)
• 3821c/rev. 11753 with sorted keyword: 10.38s(large), 4.56s(medium), 3.61s(small)
• 3821c/rev. 11753 with both keywords: 9.95s(large), 4.52s(medium), 3.65s(small)
• 4GL: 18.78s(large), 11.43s(medium), 9.96s(small)

• 3821c/rev. 11753: 26.19s(large), 20.03s(medium), 20.03s(small)
• 3821c/rev. 11753 with direct keyword: 24.87s(large), 18.35s(medium), 17s(small)
• 3821c/rev. 11753 with sorted keyword: 23.95s(large), 18.1s(medium), 17.17s(small)
• 3821c/rev. 11753 with both keywords: 24.48s(large), 18.87s(medium), 17.78s(small)
• 4GL: 26.61s(large), 19.00s(medium), 17.35s(small)

In almost all cases, using the keywords result in faster SQLs. The best example is in extent, where the keywords bring a 16% time improvement, even if in these plain testcases the optimization is up only for 7%. I will commit soon a change which adds these two keywords.

In a short time I will make a list in this thread to keep track of all possible optimizations and directions found.

• extend the use of insert into select from for some cases of append mode
• extend the use of insert into select from for tables which have unique constrains and can be validated faster
• test and add the direct and sorted keywords
• optimize extent bulk copy using insert into select from
• check #4055-60 example and understand why 3-fields.p is more permissive than 5-fields.p
• disable indexes before insert and rebuild after
  - H2 does not support index rebuild - it must be simulated by drop and create (seem to work in the same time as before in the best case). Unless a low-level index rebuild is implemented, this idea should be abandoned.
• use SET LOCK_MODE 0 to temporarily disable locking
  - On generated tests, inserting without lock works 4 times faster. In FWD this doesn't seem to bring any change. This should be investigated further.
• integrate hash indexes for fields which are often queried for equality (#4996)
  - Tested hash index for recid, but didn't observe much improvement
• optimize inserts (#4055-65)
• optimize sequences (#5003)
  - Increased DEFAULT_CACHE_SIZE
  - Used method representation for faster parsing
• extend the use of database-level pk mapping for before-table bulk copy (and other scenarios eventually)
  - Already implemented for extent fields
• do the temp-table compatibility for fast-copy checking using DMO signatures
• make a difference between normalized and denormalized extent fields
• add support for loose-copy-mode in fast-copy
• add support for replace-mode in fast-copy #4397
  - Was this fixed separately?

Regarding optimize extent bulk copy using insert into select from, I found an optimization which doesn't rely anymore on the assumption that the records with the same _multiplex are incremental. The extent fields can be bulk copied like:

-- temp-table bulkInsert
alter sequence seq_2 restart with 11;

insert into tt2 (recid, _multiplex, data) select nextval('seq_2'), 2, data from tt1 use index (idx_mpid__tt1__2) where _multiplex = ? order by _multiplex, recid {1: 1};

-- extent bulkInsert
alter sequence seq_2 restart with 11;

create local temporary table TMP__TABLE as select nextval('seq_2') as "SRC__ID", parent__id from tt1__3 join tt1 on parent__id = recid where _multiplex = ? group by parent__id order by parent__id {1: 1};

insert into tt2__3 (parent__id, list__index, extent_data) select SRC__ID, list__index, extent_data from tt1__3 join TMP__TABLE on tt1__3.parent__id = TMP__TABLE.parent__id order by tt1__3.parent__id, list__index;

drop table TMP__TABLE;

This example presumes that a temp-table copy should be done from tt1 to tt2 (and from the extent tt1__3 to tt2__3). The statements are simplified, so tt1 and tt2 contain only the reserved recid, _multiplex fields and a custom data field. The copy should be done for _multiplex = 1. TMP__TABLE and SRC__ID are some reserved names.

The flow shows how the key mapping (between source recid and destination recid) is stored in a temporary TMP__TABLE. This will be used in order to correctly associate the copied extent records to the parent records. Finally, the temporary table will be dropped.

• 3821c/rev. 11786: 59.20s(large), 29.56s(medium), 23.45s(small)
• patched 3821c/rev. 11786 with insert into select from for extent fields: 47.37s(large), 29.79s(medium), 25.37s(small)
• patched 3821c/rev. 11786 and direct + sorted keywords: 48.33s(large), 30.62s(medium), 27.30s(small)
• 4GL: 27.35s(large), 12.45s(medium), 11.24s(small)

The times are similar as the ones in #4055-62, but now the implementation is independent of records' order from _multiplex point of view. Even though the time on the large tests decreases by 20%, the time on the small tests grows by 15%. This is not a really good thing, as in practice, the small testcases (5 fields x 10 records) are more often encountered than large testcases (125 fields x 10000 records). These time changes may be justified by the temporary table creation time.

• check "computed columns"
• check if a default expression should be used instead of a null value in the row
• check if the value should be casted in order to match column's type
• check column constraints
• check if a table defined sequence should be updated

• stopped going through all columns to find the "computed ones"; cached them in a table-level list of computed columns
• avoided synchronizing defaultExpression each time a validation is triggered; synchronize it only if the value is null
• stopped trying to convert a value to column's type if the value types match already (short-circuit check)
• stopped trying to check the precision if the column's type has infinite precision (Integer.MAX_VALUE)
• introduced VAL_STRING_CACHE used for statically caching TypeInfo instances. This is because ValueString types are dependent only upon value's length, therefore I cached MAX_VAL_STRING_CACHE_SIZE = 100 instances of TypeInfo with sizes from 0 to 99. For each string with length of at least 100 a new TypeInfo instance will be created. The same was done for ValueStringFixed and ValueStringIgnoreCase.
• increased Sequence.DEFAULT_CACHE_SIZE to 1048576 (#5003)
• avoided computing a row's memory if added to a non persistent RowList (rev. 5)

• small test: 2.85sec (rev 5), 2.90sec (rev 4), 3.30sec (rev 3)
• medium test: 3.35sec (rev 5), 3.48sec (rev 4), 3.96sec (rev 3)
• large test: 6.34sec (rev 5), 7.23sec (rev 4), 8.35sec (rev 3)

Is this safe to push to our testing environments?

Code review fwd-h2 revions 4 & 5:

The changes seem functionally ok to me, though admittedly, I am less familiar with the internals of H2.

I do think we should be consistent with H2's conventions, however, in case we decide to ask for any of these improvements to be accepted back into the project. For example:

• Let's not replace their explicit listing of imported classes with the wildcard (e.g., Column class).
• Opening brace for code blocks should remain at the end of a line instead of the way we open new blocks in a new line in FWD. The latest changes mix the two styles (e.g., Column class).
• Indents should be 4 spaces per soft tab. This was done in some places but not in others.
• Instance variables should be grouped at top of file without blank lines between them (e.g., ValueString.hash). I'm not sure if this is consistent throughout H2, but it seems to be the case in the files I've visited.

Greg Shah wrote:

Is this safe to push to our testing environments?

I have not done exhaustive testing, but I have tested this some in a large customer application, without obvious error.

I have committed FWD 3821c/11817, which updates the dependency in build.gradle to use the new H2 jar file, and I have uploaded the binary accordingly.

Eric Faulhaber wrote:

I do think we should be consistent with H2's conventions, however, in case we decide to ask for any of these improvements to be accepted back into the project.

I will take care of being consistent with the conventions. I will fix this in the next revision - as I am still going around some slow methods in H2.

Greg Shah wrote:

Is this safe to push to our testing environments?

The changes shouldn't affect the behavior of H2, only the performance. Therefore, I think that H2 is safe for our testing environments.

Committed as 3821c/rev. 11824. the modifications partially described in #4055-64. This also contains the #5003-2 optimization, but only in the case of a bulk insert. I used the following statement to store the pk mapping into H2, in order to be able to use INSERT INTO SELECT FROM for extent fields:

alter sequence seq_2 restart with 11; -- the restart number is the first destination primary key
create local temporary table TMP__TABLE transactional as select seq_2.nextval as "SRC__ID", recid from tt1 where _multiplex = ? use index (idx_tt1) order by _multiplex, recid; -- the sorted clause or index name may differ

• 3821c/rev 11823 + fwd-h2/rev 5: 46.53s(large), 24.77s(medium), 20.40s(small)
• 3821c/rev 11824 + fwd-h2/rev 5: 33.54s(large), 20.37s(medium), 17.66s(small)

We are now more than half-way towards the original 4GL times. However, I don't have yet a next direction for extent field fast copying. Further, I think this H2 mapping can help in the bulk copy of before records (as now a client-side mapping is used: srcToDstPks). I added this matter in the #4055-63 list.

Adrian, have you tested to ensure all of your recent changes around extent field copying work with both normalized (default) and denormalized (#2134) extent fields?

If I understand correctly, the de-normalization feature in #2134 adds the extent fields (eventually with custom names) to the master table, while the normalized fields are stored separately and linked with a foreign key to the master table. If I understood correctly, we may have some issues.

I didn't test with de-normalized extent fields, but I think that there can be a problem when doing a temp-table copy from a table with normalized extent fields to a table with de-normalized extent fields (as I understand that the normalization is done per-table through hints?). This is because the source can have a dedicated extent table (srcTable + "__" + extent.getKey()), while the destination doesn't. Right now, a consistent part of temp-table copy through bulkCopyAllRows is based on direct SQL execution (without FQL preprocessing). This scenario is rather a top-level issue; it is not only about the extent field copying, but the whole temp-table copy and field mapping. Further, I don't think that the old implementation (before bulkInsert method) had such case particularly handled anyways - Constantin can clarify this matter.

If the source and destination have normalized extent fields, the current implementation works. If they both have de-normalized fields, this should work (however, I will do a test on this matter). If they have different normalization, I expect some weird behavior.

I might misunderstood. Only persistent tables can be de-normalized through schema hints individually. For temporary tables, they are either all de-normalized or not (is there a flag which sets this? according to #2134-89, the temporary tables were excluded from de-normalization). Anyways, this means that the case I was talking about before can't happen. Correct me if I am right.

Never mind, according to trunk/rev. 10743:

** 049 SVL 20150205          Fields of temporary tables should not be denormalized.

Adrian Lungu wrote:

Never mind, according to trunk/rev. 10743:
[...]
I guess this answers the questions around de-normalized temp-table extent fields :)

Revision 10743 is quite old and I'm not sure to what that comment refers.

Extent fields can be denormalized by hint down to the field level. Right now, we don't have any applications which define static temp-tables at that level of granularity, BUT it can be defined at that level. Also, a temp-table can be defined LIKE a persistent table. So, the possibility of denormalized extent fields needs to be supported at the temp-table level.

IIRC, we made a change not long ago for all dynamically prepared temp-tables to use denormalization for performance. At least, we discussed it and I think it was implemented. So, there definitely could be cases where the source and destination do not match. If that optimization is no longer appropriate, given the recent improvements in bulk copying, we can consider rolling it back (if indeed it was implemented). In any event, denormalization needs to be part of the decision process when choosing bulk copy vs. the original, record-by-record copy.

Note that we had some ideas on how to make the analysis of source and destination more performant (see #4055-40, #4011-774). AFAIK, this idea has not been taken further yet. We need to revisit that idea and probably expand it with the notion of whether or not an extent field is denormalized.

Eric Faulhaber wrote:

Revision 10743 is quite old and I'm not sure to what that comment refers.

After #2134, this revision was done to ignore denormalization for temp-tables and apply denormalization only for persistent schemas. Either way, I agree that this is old - but I didn't find related documentation about this technique for temp-tables.

Extent fields can be denormalized by hint down to the field level. Right now, we don't have any applications which define static temp-tables at that level of granularity, BUT it can be defined at that level. Also, a temp-table can be defined LIKE a persistent table. So, the possibility of denormalized extent fields needs to be supported at the temp-table level.

Indeed, using a "LIKE table-name" syntax will lead us back to #4055-72 and its issues. I really need to see how this works now when a table is normalized and the other is not.

IIRC, we made a change not long ago for all dynamically prepared temp-tables to use denormalization for performance. At least, we discussed it and I think it was implemented. So, there definitely could be cases where the source and destination do not match.

I found this in 3821c/11581.

If that optimization is no longer appropriate, given the recent improvements in bulk copying, we can consider rolling it back (if indeed it was implemented). In any event, denormalization needs to be part of the decision process when choosing bulk copy vs. the original, record-by-record copy.

A directory.xml setting should be true in order to trigger denormalizeDynamicTempTables. I will also test my changes with this. No need to think of any roll back yet, as by default the dynamic temp-tables are not denormalized.

Note that we had some ideas on how to make the analysis of source and destination more performant (see #4055-40, #4011-774). AFAIK, this idea has not been taken further yet. We need to revisit that idea and probably expand it with the notion of whether or not an extent field is denormalized.

I agree. My optimization rather refers to faster copy between normalized extent fields. If this is not the case, I think that another implementation should be delivered (worst-case scenario, fallback to a record-by-record copy).

Further, I think #4011-774 is worth being revisited. By now, #4055-63 focused on enhancing the performance of the current bulk copy (together with #5004 and #4996). In this process, I got a strong overview of the full picture. I think is time to make the "decision phase" more broad and allow "bulk copy" handle more cases. For this purpose, I will search for a suitable representation of a DMO signature according to #4011-774.

Adrian Lungu wrote:

These are the first steps in introducing the DMO structure signature according to #4011-774. I posted a diff containing the mentioned changes. They are completely documented and not safe yet - some features are still to be implemented: extend StringBuilder.typeMapping, flag the existence of write triggers, LOBs representation and denormalized extent fields. Done a slim testing and TemporaryBuffer.copyAllRows() works acceptable with the new changes.

Good so far. Let's make DmoMeta.signature final.

I think that this signature should be extended not to allow only "perfect matches" (so that we allow copying mandatory values to non-mandatory values for example).

Agreed. Note that for temp-tables, the MANDATORY attribute can be dropped from the signature altogether. This constraint is not honored by the 4GL in temp-tables. There is no syntax I am aware of to define a mandatory field in a temp-table. Even though a temp-table defined LIKE a persistent table with a mandatory field will inherit the mandatory field (and report it as such via the MANDATORY attribute), this constraint will be ignored at runtime (i.e., you can store unknown value in it). I think BUFFER-COPY (statement or method) to a persistent table with a mandatory field is the only case where we need to consider this constraint.

You may already take this into account in the code which generates the bulk copy SQL, but if not, we also need to consider how the SQL is affected when corresponding fields in the source and destination tables match in type and position, but not in name. AFAIK, this is legal in the 4GL.

Eric Faulhaber wrote:

Agreed. Note that for temp-tables, the MANDATORY attribute can be dropped from the signature altogether. This constraint is not honored by the 4GL in temp-tables. There is no syntax I am aware of to define a mandatory field in a temp-table. Even though a temp-table defined LIKE a persistent table with a mandatory field will inherit the mandatory field (and report it as such via the MANDATORY attribute), this constraint will be ignored at runtime (i.e., you can store unknown value in it). I think BUFFER-COPY (statement or method) to a persistent table with a mandatory field is the only case where we need to consider this constraint.

Agree, no need to store the mandatory constraint for temp-tables.

You may already take this into account in the code which generates the bulk copy SQL, but if not, we also need to consider how the SQL is affected when corresponding fields in the source and destination tables match in type and position, but not in name. AFAIK, this is legal in the 4GL.

There is already a propsMap in TemporaryBuffer.bulkCopyAllRows(), which is used in case simpleCopy is false (so when the the DMO implementation classes differ). In fact, positional and type matching is the default in 4GL for copy-temp-table.

The copy-temp-table and buffer-copy do differ on how they match the source fields with the destination fields:

• copy-temp-table is about the field type and position (and eventually constraints). This behavior can be changed by loose-copy-mode, which is rather based on a field mapping by name.
• buffer-copy is about field mapping by name. However, some fields can be ignored (they are in the source, but not in the destination / they are in the exclude list).

At this point, any copy (buffer or temp-table) based on a name mapping can't really make use of the signature. I think that the only place we can use the signature in its current state is copy-temp-table without loose-copy-mode.

I will continue with enhancing the signature with the the flags described in #4055-77 and allow a more relaxed verification of the signature in TemporaryBuffer.copyAllRows().

I committed the current implementation of DMO signature to 3821c/rev. 11835, as I reached a stable implementation, which should cover the old fast copy cases and some new ones. I have some remarks:

• Contrary to what was presumed in #4011-774, LOBs don't rule out the fast copy. In temp-table, a LOB is stored inline - so the fast copy will handle a LOB field like any other type of field. I've done a test in this direction and our current fast copy does not have any problem with LOBs. This conclusion was reached empirically; is there a normalization for those fields? I could reach #4421 which seems to be still WIP. Anyways, there is no need to have special treatment for LOBs at signature level.
• The signature can encode only schema triggers, as session triggers are created at runtime. Also, some triggers can be disabled - this can't be trivially encoded in the signature (unless we allow run-time modification of the signature).
• The signature marks the properties which are part of unique indexes (but this doesn't meant that they should be unique). This basically means that the signature is a restrictive representation. However, having such flaw will only mean we will sometimes have false negatives when matching the signatures, so will fallback to the row-to-row copy, even if we could do a fast copy instead. I will think of a fix for this: maybe store at the end of signatures the unique indexes as sorted sequences of field positions?

I committed some changes for signature building and matching to 3821c/rev. 11839:

• Enhanced unique constraints checking: encoded them at the end of the signature as a sorted sequence of encoded unique constraints (each of them as a sorted sequence of property ids). Example: CC2CCIDC2CCICmCm|1,2|1,3,10|2,10 means that there is a table with 12 fields (of type character, character extent of 2, integer or decimal), from which the last two are mandatory, and with 3 unique constraints.
• Enhanced fast-copy checking: check if the signatures are equal (meaning they are directly compatible). If they don't match, do a match on properties and afterwards on the unique constraints.
  - due to rev. 11835, if the property part of the signatures match exactly, it means they are compatible. Otherwise, try a relaxed checking: match exactly the data-type and extent, match the number of fields and check if the constrained fields in the destination are also constrained in the source (for example if they are mandatory). Example: C2ImImC2m is a suitable source for the C2IImC2m destination.
  - due to rev. 11839, if the unique constraints part of the signatures match exactly, it means they are compatible. Otherwise, try each unique constraint in the destination and find a suitable unique constraint in the source which can satisfy it. In other words, for the signatures to match, for each destination unique constraint there must be a source constraint which is a subset of the destination constraint (all the field ids in the source constraint should be in the destination constraint). Example: |1,2|1,4 is a suitable source for the |1,2,4|1,2|1,4 destination. Knowing that the unique constraint signatures are sorted and the ids inside are also sorted, this checking is relatively fast.

I am planning to tackle the append mode for copy-temp-table with fast-copy. Right now, the fast-copy implementation covers only the case when the destination table is empty (assured by not using assign mode). I am trying now to see which modifications should be done in order to allow fast-copy for append. I am after these topics:

• Master temp-table copy: The problem we are facing here is the validation of the unique constraints. While the DMO signatures should match in order to allow fast-copy, it means that both source and destination are satisfying the same unique constraints. The only real problem is when some records from the source are already in the destination (it will violate the unique constraints).

In mySQL, there is ON DUPLICATE KEY IGNORE which can easily solve our current problem. Unfortunately, standard H2 does not allow such syntax yet. Even if it had such syntax, we don't store unique indexes at H2 level; this solution is already far from becoming feasible.

For this matter, I am thinking of a syntax similar to the one below:

insert into tt_destination select * from tt_source where (
   not exists (
      select 0 from tt_destination where tt_source.unique_field = tt_destination.unique_field
      -- equals clause for each field, part of the unique_constraint
   )
   -- not exists clause for each unique constraint
)

• Normalized extent fields copy: These can't be part of an index, but can be part of a record which was not copied (because of the previous filtering). However, our current extent table copying is based on a H2-level pk mapping. As long as this mapping is correct, the extent field copying will be correct. For the append mode, a rethinking of the pk mapping should be done. This is work in progress.

• Before table copy: In the first part, we can do the copying the same as we did in the first topic ("master temp-table copy") and the pk remapping will be done using a mapping alike in the previous topic ("Normalized extent fields copy") - or even reuse it if already created. For this matter, I need to investigate a bit more how these before tables work.

These three are the only matters we deal with currently in the fast-copy.

I committed the changes proposed in #4055-82 regarding append mode support as 3821c/rev. 11851. These were build over the refactorization done in 3821c/rev 11846, which allows a FastCopyHelper to deal with all operations regarding fast temp-table copy (which use INSERT INTO SELECT FROM and database-level pk mappings to increase time performance).

For append mode, I followed the idea from #4055-82 but with slight modifications. When in append mode, a pre-copy pk mapping is done. This means that we will have an SQL like:

create local temporary table MASTER__PK__MAPPING transactional as 
select seq_2.nextval as "DST__PK", recid as "SRC__PK", _peerRowid from tt_source 
use index (idx__tt__source) 
where _multiplex = ? and 
not exists (
   select 0 from tt_destination where tt_destination._multiplex = ? and tt_source.unique_field = tt_destination.unique_field
   -- equals clause for each field, part of the unique_constraint
)
-- not exists clause for each unique constraint
order by sort_clause nulls last;

Using such master pk mapping, we can "shadow" copy the records based only on this mapping (without further validation):

insert into tt_destination (recid, _multiplex, ...) direct sorted 
select MASTER__PK__MAPPING.DST__PK, ?, ... from tt_source 
join MASTER__PK__MAPPING on tt_source.recid = MASTER__PK__MAPPING.SRC__PK 
order by sort_clause nulls last;

This technique will leave a pk mapping in the database which can be used for extent-field copying (TODO: to be used in the before table copying as well). This approach removes the overhead of scanning all records and validate them "client-side" (in the FWD server) rather than doing the validation directly server-side. Please review this approach - I am planning to move on with testing the loose-copy-mode.

There is a flaw in FastCopyHelper.java: when a copy operation is executed using a restricted field mapping, the unmapped fields remains null in database. This is not correct, those fields must be set to their default value.
In more details, if the destination table t-dest has an extra character field f-extra the sqlInsert computed in FastCopyHelper.getFastTableCopySql() will not contain the column for f-extra. The default value for character is, by default, the empty string (""), not unknown value(?), but it can be any other string the ABL programmer wishes. The new records will not be populated at insert time with these values.

Ovidiu, I see the problem. This can be appear only when loose-copy-mode is on (so we have a possible restricted field mapping). Correct me if this is not the right scenario.
I will rule out for now the loose-copy-mode from the FastCopyHelper, so it will be processed in the old way (row-by-row). I need to take a step back in order to understand how to correctly integrate the loose-copy-mode in this fast-copy technique. I am after the following considerations:

• The unmapped fields should be set on default values: need to extend processProperties so that the source and destination props strings should include the default values.
• The default values can violate unique constraints: need to extend canFastCopy in order to check if the unmapped fields are part of unique constraints.
• The default values can violate mandatory constraints: need to extend canFastCopy to see which fields are mandatory and which are set on null default.
• In general, loose-copy-mode doesn't care entirely about the quick signature. Even the unique constraints are inconsistent here, as the copied properties do not have the same order in the source as in the destination. For this matter, I think that the explicit signature (which stores the properties after they name) is more relevant.

Adrian Lungu wrote:

Ovidiu, I see the problem. This can be appear only when loose-copy-mode is on (so we have a possible restricted field mapping). Correct me if this is not the right scenario.

Yes, this is exactly the scenario where I encountered the issue. However, I am not familiar with this new code. Does DmoSignature cover them? Does it guarantee a 100% table schema compatibility including indexes? BTW, beside file-header and javadocs you may want to this immutable class implementations for a cached hashCode and specific implementation for equals() because it is used mainly in compare operations.

I will rule out for now the loose-copy-mode from the FastCopyHelper, so it will be processed in the old way (row-by-row). I need to take a step back in order to understand how to correctly integrate the loose-copy-mode in this fast-copy technique. I am after the following considerations:

• The unmapped fields should be set on default values: need to extend processProperties so that the source and destination props strings should include the default values.

Yes, the default/init values must be added to insert string, but only if their value differ from SQL default values.

• The default values can violate unique constraints: need to extend canFastCopy in order to check if the unmapped fields are part of unique constraints.

This is why I asked about the index coverage in the signature. If I understand correctly, their presence/absence does not change schema compatibility between two temp-tables, even if the actual data stores in them might be incompatible - exactly because of unique indexes.

• The default values can violate mandatory constraints: need to extend canFastCopy to see which fields are mandatory and which are set on null default.

I do not know exactly. If for a common filed this attribute (mandatory) is different, is schema compatibility unchanged? Need to test that.

• In general, loose-copy-mode doesn't care entirely about the quick signature. Even the unique constraints are inconsistent here, as the copied properties do not have the same order in the source as in the destination. For this matter, I think that the explicit signature (which stores the properties after they name) is more relevant.

Yes. However, probably all fields of the destination table must be added in the INSERT SQL statement, as noted above.

Please note: currently, the work of record initialization to default values is done in BaseRecord.initialize, which also sets some meta-state. This method can be refactored, if necessary. However, using this method of course assumes a record-by-record approach, as opposed to (or in addition to) a bulk copy. The default values for each type of DMO are stored in its associated RecordMeta object, in the initialData array.

Ovidiu Maxiniuc wrote:

Yes, this is exactly the scenario where I encountered the issue. However, I am not familiar with this new code. Does DmoSignature cover them? Does it guarantee a 100% table schema compatibility including indexes? BTW, beside file-header and javadocs you may want to this immutable class implementations for a cached hashCode and specific implementation for equals() because it is used mainly in compare operations.

• If two quick signatures are compatible, it means that a "property order based" copy is 100% safe (it checks types, extents, mandatory constraints, unique constraints) - useful for copy-temp-table
• If two explicit signatures are compatible, it means that a "name order based" copy is 100% safe (it checks names, types, extents, mandatory constraints, unique constraints) - useful for buffer-copy

I think that hashCode and custom equals() are not required - as the container is never checked for such things. The ones which should be checked are the String signatures inside.

This is why I asked about the index coverage in the signature. If I understand correctly, their presence/absence does not change schema compatibility between two temp-tables, even if the actual data stores in them might be incompatible - exactly because of unique indexes.

Well, the signature can tell if a "by property order" copy is valid for a fast-copy (even from unique constraints' point of view). Unfortunately, the loose-copy-mode can't be certified through such signature as it does a "by property name" copy of some fields. It is totally wrong to allow loose-copy just by checking this kind of signature. I tried to explain this through my last point in #4055-85. I committed 3821c/rev. 11874 which disables fast copy in loose-copy-mode for now.

Eric Faulhaber wrote:

Please note: currently, the work of record initialization to default values is done in BaseRecord.initialize, which also sets some meta-state. This method can be refactored, if necessary. However, using this method of course assumes a record-by-record approach, as opposed to (or in addition to) a bulk copy. The default values for each type of DMO are stored in its associated RecordMeta object, in the initialData array.

I will keep this in mind when searching for the initial values of each DMO property.

Overall there are two areas which should be handled: DMOSignatureHelper should check if two signatures are compatible for a loose-copy mode and FastCopyHelper should add the default values in the insert SQL. I will go ahead and implement the changes. I will be right back with a new commit on this matter.

Committed in 3821c/rev. 11881 support for fast loose-copy. Done some testing and it seems that loose-copy-mode now sets the unmapped field to the initial value (also redone my already built tests and they seem ok). I will run a bit more tests further on to see if anything obvious will pop out.

What is left to do on this task?

#4055-63 keeps track of the things left to do. The opened ones are still some possible directions, which were explored for a while. Some of them were taken in consideration and implemented. I don't thing we can push further the optimization of temp-table copy for now (taking in consideration what we already have).

The only matter to be solved here is the denormalized/normalized fields fast-copy. I don't know if this scenario can create problems, but I need to investigate to make sure (and eventually restrict fast-copy). Adding to my WIP queue.