FWD » Core Development » Database

I created branch 8459a_h2 for this matter.

Constantin Asofiei wrote:

will make H2 use the scan index.

Constantin, is there a chance that you can point me to where this choice of using the scan index is being made?
I made a slight adjustment to H2 and I want to make sure that things actually change.

If you have two big tables and none have indexes, on join, it should do scan on both tables (nested join - N * M comparisons).

This test shows the 'unknown equality' in OpenEdge:

def temp-table tt1 field f1 as int field f2 as int index ix1 f1.
def temp-table tt2 field g1 as int field g2 as int index ix2 g1.

create tt1.
tt1.f1 = 1.
tt1.f2 = 10.
release tt1.

create tt1.
tt1.f1 = ?.
tt1.f2 = 20.
release tt1.

create tt2.
tt2.g1 = 1.
tt2.g2 = 100.
release tt2.

create tt2.
tt2.g1 = ?.
tt2.g2 = 200.
release tt2.

for each tt1, each tt2 where tt1.f1 = tt2.g1:
   message tt1.f1 tt1.f2 tt2.g1 tt2.g2.
end.

In FWD, the join will be augmented as tt1.f1 = tt2.g1 or (tt1.f1 is null and tt2.g1 is null) - we want to remove the is null augmentation and change H2 so that 'null equals null'.

Alex/Constantin: Was the phrase tt1.f1 is null added just for the H2 dialect, or is this null equality an issue in multiple dialects? What I want to know is whether the removal of is null at FQL level would impact other dialects, or is it safe to handle it there. As an alternative, when converting the FQL into SQL, I can disable the addition of is null just when dealing with H2 dialect.

Radu Apetrii wrote:

Alex/Constantin: Was the phrase tt1.f1 is null added just for the H2 dialect, or is this null equality an issue in multiple dialects? What I want to know is whether the removal of is null at FQL level would impact other dialects, or is it safe to handle it there. As an alternative, when converting the FQL into SQL, I can disable the addition of is null just when dealing with H2 dialect.

The augmentation for is null is required for all dialects, not just H2. And if we make H2 to have null = null be true, then the augmentation in FQLPreprocessor needs to be skipped only for H2 dialect.

Radu, I think we should restrict this with a flag. H2 is used with other goals in FWD and I don't what to make this change for every use-case.

Create a flag like in the H2_Database_Fork Wiki and use it to distinguish the cases where we actually need this. Add it in FWD for H2Helper connection string.

Please run the H2 tests and check which fail now - none should fail. Create a unit test in H2 for this flag (lets say USE_NULL_EQUALITY) and do extra tests.

• I don't know how database.compare works, but I presume it will return false if only one parameter is null. I really want to avoid having a NPE somewhere after your changes. I would rather suggest to treat this way more specific, like if (useNullEquality && l null && r null) return true;, without letting the null values freely inside database.compare. It wasn't made to actually treat nulls, so I expect the worst.
• The same can go for getValue
• Please check the following construct ORDER BY x ASC NULLS LAST (how is the sorting done after your changes?). Also, check for indexes (CREATE INDEX idx ON tt (f1, f2 NULLS LAST)) and insert a record with f2 or f1 as null. Are they stored ok?

Alex/Constantin: Was the phrase tt1.f1 is null added just for the H2 dialect, or is this null equality an issue in multiple dialects? What I want to know is whether the removal of is null at FQL level would impact other dialects, or is it safe to handle it there. As an alternative, when converting the FQL into SQL, I can disable the addition of is null just when dealing with H2 dialect.

It is for all dialects. The feature you introduce should be done only for H2.

Alexandru Lungu wrote:

Create a flag like in the H2_Database_Fork Wiki and use it to distinguish the cases where we actually need this. Add it in FWD for H2Helper connection string.

Should this setting be false by default? If so, then what should trigger it to become true?

Radu Apetrii wrote:

Alexandru Lungu wrote:

Create a flag like in the H2_Database_Fork Wiki and use it to distinguish the cases where we actually need this. Add it in FWD for H2Helper connection string.

Should this setting be false by default? If so, then what should trigger it to become true?

Yes, default it to false; use a JDBC URL connection parameter to set it to true.

Radu, do you have the FWD and H2 changes, for some early testing?

Please rebase both 8459a_h2 and 8459a.

Constantin Asofiei wrote:

Please rebase both 8459a_h2 and 8459a.

On it.

Who should be doing the review?

Radu Apetrii wrote:

Constantin Asofiei wrote:

Please rebase both 8459a_h2 and 8459a.

On it.

Done. 8459a rebased with trunk rev. 15069, and 8459a_h2 with fwd-trunk rev. 41.

Who should be doing the review?

I'm OK with either Alex or Constantin. I'm not sure if Alex is still up, however.

I am OK with the changes in 8459a (FWD branch).

• In compare, you need to also explicitly handle the case when only one operand is null and return false
  • in other words, if the flag is set, one operand is null and the other is not, return false. With the current changes, it will return null.
  • the same reasoning goes with NOT_NULL

• select 1 = null
• select null = null
• select null = 1
• select 1 = 1
• the above with <>

The tests above should work normally without the flag.

Also, one more thing. In Comparison.getValue, depending on the compareType value, things should be treated for both = and != signs. I'm not sure if it is enough to test compareType 0 for equality and compareType 5 for non-equality. Aside from that, I have taken on board everything said.

• Added a case for handling equality between a null component and a non-null component.
• Added TestNullEquality class responsible for null equality tests. This includes basic tests (like in #8459-19), tests with join (where both components are null), tests with ORDER BY tt.f1 NULLS LAST/FIRST, and tests with indexes that have NULLS LAST in them.

The only thing that I'm unsure about at the moment is the question raised in #8459-20. After this gets addressed, from my point of view, we will be in a good position.

I am OK with 8459a_h2 changes. Good addition with the unit test!

The getValue looks right to me. The only concern is in regard to what 0 and 5 actually mean. Can we replace these with something more suggestive. Are these EQUAL and NON_EQUAL, or something like that?

Alexandru Lungu wrote:

Can we replace these with something more suggestive. Are these EQUAL and NON_EQUAL, or something like that?

Yes, they represent EQUAL and NON_EQUAL. This was my concern as well, that maybe it is not the best practice to just check the equality with 0 or 5. Should I change these into something else?

What about simple comparison with EQUAL (instead of 0) and NON_EQUAL (instead of 5)? Am I missing something?

Alexandru Lungu wrote:

What about simple comparison with EQUAL (instead of 0) and NON_EQUAL (instead of 5)? Am I missing something?

No, I was a bit confused, because I thought these were values for a bitmask. I didn't look properly.

I replaced the values 0 and 5 with EQUAL and NOT_EQUAL in 8459a_h2 rev. 44.

Alexandru, I'm good.

Radu: if unit tests/standalone tests are OK, please build fwd-h2 and upload it to devsrv01:/tmp/. I'll upload it to the public repository. Once this and 8459a is ready, I'll do ETF testing.

Constantin Asofiei wrote:

Alexandru, I'm good.

Radu: if unit tests/standalone tests are OK, please build fwd-h2 and upload it to devsrv01:/tmp/. I'll upload it to the public repository. Once this and 8459a is ready, I'll do ETF testing.

Done. I've uploaded the jar to devsrv01:/tmp/. The name is fwd-h2-1.42-trunk.jar.

fwd-h2 1.42 is on projsrv01.

I've started merge of 8459a.

def temp-table tt1 field f1 as int index ix1 is unique primary f1.

def var i as int.
do transaction:
   create tt1.
   tt1.f1 = ?.
   release tt1.
   create tt1.
   tt1.f1 = ?.
   release tt1.
end.

for each tt1: message tt1.f1. end.

Created 8459b and committed the disable for USE_NULL_EQUALITY.

Preparing for merge with trunk.

Merged to trunk as rev. 15085.

Constantin Asofiei wrote:

The problem is this happens only when we do a unique validation using SELECT, and not on INSERT - if you remove the TRANSACTION, then it works.

This means that the error appears when performing checks at the end transaction level, right? (and not inside the transaction, even though release s happen)

Alexandru Lungu wrote:

Merged to trunk as rev. 15085.

Please post to #7847 also.

Radu Apetrii wrote:

Constantin Asofiei wrote:

The problem is this happens only when we do a unique validation using SELECT, and not on INSERT - if you remove the TRANSACTION, then it works.

This means that the error appears when performing checks at the end transaction level, right? (and not inside the transaction, even though release s happen)

I think it is the release that is causing this. RELEASE is forcefully calling the WRITE triggers according to the documentation, so it is usually associated with flush and validation. Also, I think DO TRANSACTION is redundant; you would have the same problem if you attempted to do tt1.f1 = ? the second time.

Radu, mind that you can also use VALIDATE statement to trigger validation.