Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sequel to #860 and #861: Incorrect SQL code generated for I /\ r~;r /\ s;s~ in the face of generalizations #862

Closed
RieksJ opened this issue Dec 17, 2018 · 3 comments · Fixed by #870
Assignees
Labels
bug Indicates an unexpected problem or unintended behavior component:prototype generator priority:high

Comments

@RieksJ
Copy link
Contributor

RieksJ commented Dec 17, 2018

This ticket describes the issue we have encountered in a large project, and that we tried to describe earlier in #860 and #861, which were attempts to quickly find a solution. The issue is that in our project we have the following rule (definitions of relations are given in the comments):

RULE "InsPair npFormIsClean for PolisRegForms":
   I[NPForm] 
   /\ polregNPForm~;polregNPForm                    -- RELATION polregNPForm[PolisRegForm*NPForm] [INJ,UNI]
   /\ npform_Geslachtsnaam;npform_Geslachtsnaam~    -- RELATION npform_Geslachtsnaam[NPForm*Geslachtsnaam] [UNI]
   /\ npform_Voornamen;npform_Voornamen~            -- RELATION npform_Voornamen[NPForm*Voornamen] [UNI]
   /\ npform_Geboortedatum;npform_Geboortedatum~    -- RELATION npform_Geboortedatum[NPForm*Geboortedatum] [UNI]
   /\ npform_Geboorteplaats;npform_Geboorteplaats~  -- RELATION npform_Geboorteplaats[NPForm*Geboorteplaats] [UNI]
   /\ npform_Geslacht;npform_Geslacht~              -- RELATION npform_Geslacht[NPForm*Geslachtsaanduiding] [UNI]
|-  npFormIsClean                                   -- RELATION npFormIsClean[NPForm*NPForm] [PROP]

Compiling the script with the --sqldump switch resulted in the following SQL as is shown in the file referenced in next comment.

The problem can be seen by focusing on the last lines in the SQL, and in particular the following:

                                 where ("NPForm" is not null)
                                       and ("PolisRegForm" is not null)
                                       and ("npform_Geslachtsnaam" is not null)
                                       and ("npform_Voornamen" is not null)

Notice that lines 3 and 4 show the names of relations npform_Geslachtsnaam and npform_Voornamen, whereas line 2 shows the name of a concept, PolisRegForm. I would have expected that PolisRegForm would have been a SQL text that represents polregNPForm~ (notice the ~ here).

@RieksJ RieksJ added priority:high bug Indicates an unexpected problem or unintended behavior component:prototype generator labels Dec 17, 2018
@RieksJ
Copy link
Contributor Author

RieksJ commented Dec 17, 2018

Here is a file issue862 sql code.txt that I just managed to upload. It contains the (erroneous) SQL code, but (alas) also some rubbish around it - please ignore that.

@RieksJ
Copy link
Contributor Author

RieksJ commented Dec 17, 2018

I managed to produce ADL code that demonstrates this issue, as well as another one that seems to have the same cause:

CONTEXT Issue862

--[Generic Forms]
npFormIsClean :: NPForm * NPForm [PROP]

--[RegForms and NPForms]
CLASSIFY RegForm ISA Form
CLASSIFY NPForm ISA Form
regNPForm :: RegForm * NPForm  [INJ,UNI] -- the NPForm is a subform of the RegForm
npfName   :: NPForm * Name     [UNI] -- Name of the NPForm
npfText   :: NPForm * Contents [UNI] -- Contents of the NPForm

ROLE ExecEngine MAINTAINS "InsPair npFormIsClean for RegForms"
RULE "InsPair npFormIsClean for RegForms":
   I[NPForm] /\ regNPForm~;regNPForm /\ npfName;npfName~ /\ npfText;npfText~ |- npFormIsClean
VIOLATION (TXT "{EX} InsPair;npFormIsClean;Form;", SRC I, TXT ";Form;", TGT I)

--[Default population]
POPULATION regNPForm CONTAINS [ ("Reg1", "NPForm1") ]
POPULATION npfName   CONTAINS [ ("NPForm1", "NPForm1 name") ]
--POPULATION npfText   CONTAINS [ ("NPForm1", "NPForm1 contents") ]

--[Interface that shows (the effects of) the issue]
INTERFACE "Home": V[SESSION*NPForm] CRuD BOX <SHCOLS>
   [ "Form": I cRud
   , "regNPForm~": regNPForm~ cRud
   , "Name": npfName cRUd
   , "Contents": npfText cRUd
   , "Clean?": npFormIsClean cRUd
   ]
REPRESENT Name, Contents TYPE ALPHANUMERIC

ENDCONTEXT

To reproduce, build a prototype, (re)install the database, and click on the Home-text in the menu bar.

We have two issues:

  1. Notice that the field regNPForm~ is empty, which shouldn't be the case, since its value is explicitly specified in the default population (and indeed, inspecting the database shows that it is there).
  2. Enter a value in the field Contents, which should cause the rule InsPair npFormIsClean for RegForms to be triggered, and result in npFormIsClean to be populated. Inspecting the database again shows that the rule should fire, but it does not. The reason for this can be found in the SQL code text I uploaded an hour or so ago.

The problem seems to originate from a combination of optimization code and the fact that generalizations are in play here.

@RieksJ RieksJ changed the title Sequel to #860 and #861: Incorrect SQL code generated for I /\ r~;r /\ s;s~ ... Sequel to #860 and #861: Incorrect SQL code generated for I /\ r~;r /\ s;s~ in the face of generalizations Dec 17, 2018
@hanjoosten
Copy link
Member

There is good news and bad news. While looking at the code, I found the bug. Acutally, this bug was introduced while solving issue #217 . Later on, also similar bugs were found (issue #627 and #760). It turns out that the function that decides when an expression can be found in a broad table (and in the same row!) is buggy. It needs to be rewritten. Good news is that this can be done in an elegant manner. Bad news is that this will take some more time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior component:prototype generator priority:high
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants