search

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

Get rid of PHP to communicate with the database during designtime #559

Open hanjoosten opened 8 years ago

hanjoosten commented 8 years ago

As has been shown in issue #557 , --validate causes problems when there are enough violations in a single rule. These problems are due to the fact that we have php as an intermediate way to run SQL queries.

PHP is only used to build and querie a temporary database in the case of --validate. Other uses of PHP are already replaced by using JSON.

There are several database interfaces possible. We have to remind, that we want to prove that the SQL that we generate (and hand over using JSON) is correct. Therefore, HaskellDB is probably NOT suitable, for it abstracts away from SQL. (Very cool, by the way). Currently, at first sight I think that mysql-simple might be the right choice for this single requirement. It doesn't contain a way to construct queries, but we allready do that using simple-sql-parser.

Michiel-s commented 3 years ago

Intro

I've done a analysis to check if this database query validation using php is still working. I did except it would not work anymore, because the php scripts embedded in haskell code were very outdated and it required a connection to a MySQL database, but we don't have that enabled in our CI runner.

Analysis

When running the tests they all pass!

Registering library for ampersand-4.5.0..
ampersand> test (suite: ampersand-test)

Starting Quickcheck tests.
✅ Passed: Prettyprint/Parser roundtrip.
+++ OK, passed 100 tests.
All tests succeeded.
✅ Passed.
Starting regression test.
>> 1: /home/runner/work/Ampersand/Ampersand/testing
>> 2: /home/runner/work/Ampersand/Ampersand/testing/Sentinel
>> 3: /home/runner/work/Ampersand/Ampersand/testing/Sentinel/Tests
>> 4: /home/runner/work/Ampersand/Ampersand/testing/Sentinel/Tests/ShouldFail
>> 4.1: Start: ampersand validate --verbose Session.adl
>> 4.1: ✅ Passed.
>> 4.2: Start: ampersand proto --verbose --no-frontend Session.adl
>> 4.2: ✅ Passed.
>> 4.3: Start: ampersand validate --verbose singletontest.adl
>> 4.3: ✅ Passed.
>> 4.4: Start: ampersand proto --verbose --no-frontend singletontest.adl
>> 4.4: ✅ Passed.
>> 4.5: Start: ampersand validate --verbose Issue335.adl
>> 4.5: ✅ Passed.
>> 4.6: Start: ampersand proto --verbose --no-frontend Issue335.adl
>> 4.6: ✅ Passed.
>> 4.7: Start: ampersand validate --verbose Issue406.adl
>> 4.7: ✅ Passed.
>> 4.8: Start: ampersand proto --verbose --no-frontend Issue406.adl
>> 4.8: ✅ Passed.
>> 4.9: Start: ampersand validate --verbose Issue298.adl
>> 4.9: ✅ Passed.

But what is actually happening here?

I've changed the haskell code a bit to also print out the stdout and stderr of the test runs. In each test the ampersand compiler is executed as sub process. With the script below, all output is logged as warning. I used warning because running with --verbose doesn't effect the output of the tests.

...
    passHandler :: (HasLogFunc env) => (ExitCode, BL.ByteString, BL.ByteString) -> RIO env ()
    passHandler (_, out, err) = do
        mapM_ (logWarn . indnt) . toUtf8Builders $ out
        mapM_ (logWarn . indnt) . toUtf8Builders $ err
        logInfo $ indent<>"✅ Passed."
...

Current/original code: https://github.com/AmpersandTarski/Ampersand/blob/cbee06acb8e9294d090bf8ac5481840601f6eae8/src/Ampersand/Test/Regression.hs#L211-L213

Now looking at the output of running the tests we see way more details on what's happening.

First of all, all warnings shown by the compiler are visible (e.g. about ending without BOM) but also PHP warnings and error results that result from the command ampersand validate --verbose <script>

...
>> 7.10: ✅ Passed.
>> 7.11: Start: ampersand validate --verbose Ticket580.adl
    2021-11-17 17:46:22.871058: [debug] Ampersand-v4.5.0 [main:cbee06acb*] runs with the following settings:
    2021-11-17 17:46:22.965150: [debug] --[no-]backend True
    2021-11-17 17:46:25.900554: [debug] --[no-]check-compiler-version True
    2021-11-17 17:46:25.900636: [debug] --[no-]frontend True
    2021-11-17 17:46:25.900682: [debug] --[no-]metamodel False
    2021-11-17 17:46:25.900735: [debug] --[no-]terminal False
    2021-11-17 17:46:25.900787: [debug] --[no-]time-in-log True
    2021-11-17 17:46:25.900838: [debug] --[no-]trim-cellvalues True
    2021-11-17 17:46:25.900890: [debug] --build-recipe Prototype
    2021-11-17 17:46:25.900941: [debug] --crud-defaults CRUD
    2021-11-17 17:46:25.901018: [debug] --ignore-invariant-violations False
    2021-11-17 17:46:25.901069: [debug] --interfaces False
    2021-11-17 17:46:25.901120: [debug] --language Nothing
    2021-11-17 17:46:25.901171: [debug] --namespace ""
    2021-11-17 17:46:25.901221: [debug] --output-dir "."
    2021-11-17 17:46:25.901272: [debug] --proto-dir ".proto"
    2021-11-17 17:46:25.901333: [debug] --sql-bin-tables False
    2021-11-17 17:46:25.901384: [debug] --terminal-width Nothing
    2021-11-17 17:46:25.901434: [debug] --verbosity debug
    2021-11-17 17:46:25.901484: [debug] AMPERSAND_SCRIPT Ticket580.adl
    2021-11-17 17:46:25.913990: [debug] Reading file /mnt/c/dev/git/ampersand-tarski/ampersand-compiler/testing/Sentinel/Tests/ShouldSucceed/Ticket580.adl
    2021-11-17 17:46:25.937324: [debug] Reading internal file FormalAmpersand.adl
    2021-11-17 17:46:25.965489: [debug] Reading internal file Concepts.adl
    2021-11-17 17:46:25.969876: [debug] Reading internal file Contexts.adl
    2021-11-17 17:46:25.987981: [debug] Reading internal file Rules.adl
    2021-11-17 17:46:25.990643: [debug] Reading internal file Relations.adl
    2021-11-17 17:46:25.994233: [debug] Reading internal file Patterns.adl
    2021-11-17 17:46:26.007639: [debug] Reading internal file Terms.adl
    2021-11-17 17:46:26.013581: [debug] Reading internal file Documentation.adl
    2021-11-17 17:46:26.015151: [debug] Reading internal file Generics.adl
    2021-11-17 17:46:26.017301: [debug] Reading internal file Interfaces.adl
    2021-11-17 17:46:26.020137: [debug] Reading internal file PrototypeContext.adl
    2021-11-17 17:46:26.028172: [debug] Reading internal file Interfaces.adl
    2021-11-17 17:46:26.029922: [debug] Reading internal file Interfaces.ifc
    2021-11-17 17:46:26.031800: [debug] Reading internal file Navbar.adl
    2021-11-17 17:46:26.035255: [debug] Reading internal file Navbar.ifc
    2021-11-17 17:46:26.037108: [debug] Reading internal file Roles.adl
    2021-11-17 17:46:26.070303: [warn] /mnt/c/dev/git/ampersand-tarski/ampersand-compiler/testing/Sentinel/Tests/ShouldSucceed/Ticket580.adl:1:1 warning: 
      Unrecognized character in the beginning of the file.
      Is it saved with encoding UTF-8 without BOM?
    2021-11-17 17:46:26.071085: [info] Validating SQL expressions...
    2021-11-17 17:46:26.071156: [debug] Initializing temporary database (this could take a while)
    PHP Warning:  PHP Startup: Unable to load dynamic library 'mysqli' (tried: /usr/lib/php/20190902/mysqli (/usr/lib/php/20190902/mysqli: cannot open shared object file: No such file or directory), /usr/lib/php/20190902/mysqli.so (/usr/lib/php/20190902/mysqli.so: undefined symbol: mysqlnd_global_stats)) in Unknown on line 0
    PHP Warning:  mysqli_connect(): php_network_getaddresses: getaddrinfo failed: Name or service not known in /tmp/tmpPhpQueryOfAmpersand.php on line 8
    PHP Warning:  mysqli_connect(): (HY000/2002): php_network_getaddresses: getaddrinfo failed: Name or service not known in /tmp/tmpPhpQueryOfAmpersand.php on line 8
    2021-11-17 17:46:28.176103: [info] "Failed to connect to MySQL: php_network_getaddresses: getaddrinfo failed: Name or service not known"
    2021-11-17 17:46:28.176221: [info] Temp database creation failed! :
    The result:
    Failed to connect to MySQL: php_network_getaddresses: getaddrinfo failed: Name or service not known
    The statements:
    /*00001*/ // Try to connect to the MySQL server
    /*00002*/ global $DB_host,$DB_user,$DB_pass;
    /*00003*/ $DB_host='root';
    /*00004*/ $DB_user='ampersand';
    /*00005*/ $DB_pass='';
...
    ');
    /*00301*/ if($err=mysqli_error($DB_link)) { $error=true; echo $err.'<br />'; }
    2021-11-17 17:46:28.177269: [info] Error: Database creation failed. No validation could be done.
>> 7.11: ✅ Passed.

As you can see, although everything with PHP and MySQL failed, the test still passed.

What to do

As this validate functionality is not working anymore (and I think it didn't for a long long time), we should fix it. However, as this Github issue here requests to "get rid of php to communicate with the database during designtime" I think refactoring is more appropriate.

The complete Ampersand.Prototype.PHP module can be removed. It is only used by Ampersand.Prototype.ValidateSQL. In the latter we should find another way to validate the generated SQL queries.

@hanjoosten suggested to use https://hackage.haskell.org/package/mysql-simple. Let's investigate that one.

hanjoosten commented 1 year ago

This issue is quite old, and I do not know the current status. Today @Michiel-s claimed that the --validate part does not work in the CI actions, while in my view on the state of the universe we DO test the contents of the queries. So let's dig in and see if we can prove either claim.

A test to test the testing

If my claim holds, then a connection to the database is made for every regression-test that has the --validate parameter. In that case, if we deliberately generate SQL code for a query that is broken, we will see these errors in the regression test.
It is pretty easy to do such a thing: Prepend some constant string to every column name inside the SELECT queries will do.

Results of executing the test

The results don't lie: The --validate tests are working as expected in the CI actions: https://github.com/AmpersandTarski/Ampersand/actions/runs/4155847254/jobs/7189232717#step:5:14907

Michiel-s commented 1 year ago

Looks indeed that we've fixed the issue mentioned in my original issue here.

This doesn't explain yet why our release build is failing.