AmpersandTarski / Ampersand

Build database applications faster than anyone else, and keep your data pollution free as a bonus.
http://ampersandtarski.github.io/
GNU General Public License v3.0
40 stars 8 forks source link

Strange warning: Ampersand is case-sensitive. you might have meant that the following are equal #1416

Open stefjoosten opened 1 year ago

stefjoosten commented 1 year ago

What happened

I got the following warning from the Ampersand compiler vs. Ampersand-v4.7.1:

Unknown origin warning: 
  Ampersand is case-sensitive. you might have meant that the following are equal:
      Relation `Aanvrager` and `aanvrager`.

The parser keeps Aanvrager and aanvrager apart in my source code, because Aanvrager is a concept identifier and aanvrager is a relation identifier. I got no parser errors, so the ADL code is fine. I traced the problem to a .xlsx-file. It contains the following data:

afbeelding

It is relevant to know that the source code defines

CLASSIFY Aanvrager ISA Persoon

The second column contains the concept set of Aanvrager. If I remove this column, the warning goes away. So, the identifier Aanvrager from cell B1 is mistakenly added to the set of relations, which results in the shown warning.

What I expected

I did not expect this warning, since everything is fine.

stefjoosten commented 1 year ago

Observations

  1. The Ampersand compiler parses a row that starts with [ <Some Concept> ] as a row of one concept followed by an arbitrary number of relations. That explains why it expects a relation in cell B1.
  2. The spreadsheet, especially the first two rows in the screenshot, were generated by the command ampersand population. Therefore, I expected this to work. In fact, we want a round trip over ampersand population and ampersand check to work in all cases, but we know that this does not work yet.
  3. The warning yields a very confusing experience, because I expected it to work. (That is why this issue was created as a bug).

Solutions

It is a reasonable requirement for this example to work, especially since it is consistent with the excel-sheet produced by ampersand population. This will also fit nicely in a future round trip, so it is an improvement rather than a bug.

hanjoosten commented 1 year ago

In your model, I expect CLASSIFY Aanvrager ISA Persoon. The parser cannot have knowledge about that. To be able to populate the isa-relation, we need a syntactical convention. The .xlsx parser seems to accept the Conceptname as the name of the relation, which seems to me a good convention. However, in the current implementation, I am pretty sure that under-the-hood a relation with an invalid name is created and populated, not being the isa-relation you intended.

A related question in this respect: How should the isa-relation be populated in the initial population, using an .adl file only? I think that currently, this isn't possible as well.