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

Urgent!! Generated database queries are changed somewhere in the past few weeks/months #1261

Closed Michiel-s closed 2 years ago

Michiel-s commented 2 years ago

I was trying to update the prototype framework to use the latest version of the Ampersand compiler (v4.6.z).

The prototypes are not functioning at all, due to a change in the generated SQL code. Atom values (singletons) are put between double quotes instead of single quotes. Double quotes are for table/column names in ANSI sql.

I don't understand why this is changed. I haven't analyzed yet where the change is made so no clue now how long this issue is already in our code base.

I propose to rethink testing changes to the compiler, because the statement "Haskell is your friend here" doesn't apply when output is unintended changing due to refactoring or changing other parts of the code. Let's see how this could happen.

hanjoosten commented 2 years ago

This is probably due to the upgrade of to a newer release of ghc. This caused to upgrade some dependencies as well. The dependency of simple-sql-parser to 0.6.0 was done in commit c1b8c606a500b49528fb6e55af2e921f060384bd . This is most likely the cause. Normaly it would be enough to rollback that commit. However, unfortunately that isn't that simple now, because we formatted the file in the meantime. I will look into it further shortly.

I propose to rethink testing changes to the compiler, because the statement "Haskell is your friend here" doesn't apply when output is unintended changing due to refactoring or changing other parts of the code. Let's see how this could happen.

I fully agree!

hanjoosten commented 2 years ago

@Michiel-s I have a pull request ready that should fix this issue. I manually inspected the generated sql code in conjuncts.json. I believe the issue with quotes has been solved. If you could build a prototype and check this fix, then I suggest that you merge it into main.