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

Issues with MySQL test cases #100

Closed
4 tasks done
chrdebru opened this issue Feb 25, 2024 · 10 comments
Closed
4 tasks done

Issues with MySQL test cases #100

chrdebru opened this issue Feb 25, 2024 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@chrdebru
Copy link
Contributor

chrdebru commented Feb 25, 2024

  • RMLTC0018a-MySQL The test assumes that MySQL is ANSI SQL compliant, but it is not. MySQL does trim trailing spaces. Hence the test will wrongly accuse of engines not to produce correct RDF. I would suggest removing RMLTC0018a-MySQL.
  • RMLTC0015a-MySQL The JDBC URL in the mapping does not set the sql-mode to ANSI_SQL. This test case thus assumes that this SQL mode is enabled on the server. This is not only be default not the case, but the community encourages not to do so as it can be specified at connection level. So options here are: remove the test, add the sql mode in the jdbc connection url, or explicitely state all assumptions of the MySQL server.
  • RMLTC0002f-MySQL seems to be a valid mapping. Everything matches (table name, and column names).
  • RMLTC0003b-MySQL needs to return an error (and thus no output file with triples) as the source contains both a table name and a query.
@DylanVanAssche
Copy link
Collaborator

  • RMLTC0018a-MySQL + RMLTC0015a-MySQL: maybe we change the test-case for MySQL with a note about its compliance?
  • RMLTC0002f-MySQL: I wonder here as well. There's some docs here: https://rml.io/test-cases/#rmltc0002f-mysql mentioning about 'Name' not okay for a template. But I don't see what is supposed to be wrong?
  • RMLTC0003b-MySQL: so removing the output?

@DylanVanAssche DylanVanAssche added the bug Something isn't working label Feb 29, 2024
@chrdebru
Copy link
Contributor Author

  1. Yes. Either remove the tests for MySQL. Or have something that would pass for them and write a note. RML engine should try and support PostgreSQL (which is more ANSI compliant).
  2. I'm pretty sure 2f is not wrong
  3. I proposed a fix for 3b. The output of 3b was not empty. All other failing mappings had empty output files. So, I believe that the additional rml:tableName was a copy-paste error. :-) It makes sense, as the reference relied on a derived attribute (concatenate). In other words, 3b is fixed.

@pmaria
Copy link
Collaborator

pmaria commented Mar 4, 2024

Some other issues:

rml:SQL2008TableName is used vs rml:SQL2008Table (in io spec)
rml:tableName used but not described io spec
rml:sqlVersion used but described io spec

Actions:

  • replace rml:SQL2008TableName with rml:SQL2008Table
  • remove rml:sqlVersion
  • replace rml:tableName with rml:iterator
  • replace rml:query with rml:iterator

@pmaria
Copy link
Collaborator

pmaria commented Mar 4, 2024

@pmaria
Copy link
Collaborator

pmaria commented Mar 4, 2024

  • RMLTC0012b-MySQL missing reference formulation

@pmaria
Copy link
Collaborator

pmaria commented Mar 5, 2024

RMLTC0015a-MySQL seems to have been an issue with erroneously escaped quotes in a multiline string. See 5e21fae. @chrdebru does that fix it for you as well?

@pmaria
Copy link
Collaborator

pmaria commented Mar 5, 2024

I'm leaning towards dropping RMLTC0018a entirely due to the different behavior of databases and this rather niche case. Agreed @chrdebru @DylanVanAssche ?

@DylanVanAssche
Copy link
Collaborator

+1 for dropping 18a

@chrdebru
Copy link
Contributor Author

chrdebru commented Mar 7, 2024

Agree on dropping 18a.

@bjdmeest
Copy link
Member

bjdmeest commented Mar 8, 2024

agree on dropping 18a too. But we probably want an IO test to cover "Description: Generation of triples by using CHAR datatype column, resulting RDF literal is space-padded. (specification reference)" at a later stage. Tracked at kg-construct/rml-io#41 so this issue can be closed after stuff is fixed

DylanVanAssche added a commit that referenced this issue Mar 8, 2024
Padding is SQL vendor dependent, see #100
DylanVanAssche added a commit that referenced this issue Mar 8, 2024
Padding is SQL vendor dependent, see #100
DylanVanAssche added a commit that referenced this issue Apr 12, 2024
Drop remaining files, see #100
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants