Bug 24912 - Generate sql from yaml installer files
Summary: Generate sql from yaml installer files
Status: ASSIGNED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 24883
Blocks:
  Show dependency treegraph
 
Reported: 2020-03-20 11:17 UTC by Jonathan Druart
Modified: 2020-05-15 11:28 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 24912: Centralize code to load YAML installer files to a sub (3.72 KB, patch)
2020-03-20 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24912: Move to a flat array with all values to an array of arrayref (2.17 KB, patch)
2020-03-20 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24912: Add a new yml2sql sub to C4::Installer (1.38 KB, patch)
2020-03-20 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24912: Move new sub from load_yaml.pl (3.49 KB, patch)
2020-03-20 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24912: Add a new yml2sql sub to C4::Installer (1.38 KB, patch)
2020-03-20 15:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24912: Add the ability to convert yaml installer files to SQL queries (1.47 KB, patch)
2020-03-20 15:29 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2020-03-20 11:17:20 UTC

    
Comment 1 Jonathan Druart 2020-03-20 11:20:09 UTC
Created attachment 101078 [details] [review]
Bug 24912: Centralize code to load YAML installer files to a sub
Comment 2 Jonathan Druart 2020-03-20 11:20:12 UTC
Created attachment 101079 [details] [review]
Bug 24912: Move to a flat array with all values to an array of arrayref

For the following SQL query:
INSERT INTO cities(city_name, city_country) VALUES ('Madrid', 'Spain'), ('Buenos Aires', 'Argentina');

We move from:
[ 'Madrid', 'Spain', 'Buenos Aires', 'Argentina' ]
to:
[ [ 'Madrid', 'Spain' ], [ 'Buenos Aires', 'Argentina' ] ]

Which make more sense to split, build and construct the queries
Comment 3 Jonathan Druart 2020-03-20 11:20:15 UTC
Created attachment 101080 [details] [review]
Bug 24912: Add a new yml2sql sub to C4::Installer

It will allow us to generate sql code from the yml files
Comment 4 Jonathan Druart 2020-03-20 11:20:18 UTC
Created attachment 101081 [details] [review]
Bug 24912: Move new sub from load_yaml.pl

Use the sub from C4::Installer to avoid dup of code. Also add an option to convert the yml file to SQL queries

Note:
We are going to modify the script and so will do more stuffs.
We may want to rename it, maybe installer_utilities.pl,
misc/installer/yaml_utility.pl, any suggestions?
Comment 5 Jonathan Druart 2020-03-20 11:21:27 UTC
From bug 24883 comment 13:
"""
Now, the conversion part. 
The problem here is that all values are printed as strings, producing
INSERT INTO marc_tag_structure ( `authorised_value`,`frameworkcode`,`liblibrarian`,`libopac`,`mandatory`,`repeatable`,`tagfield` ) VALUES 
('', '', 'LEADER', 'LEADER', '1', '0', '000'),

instead of
('', '', 'LEADER', 'LEADER', 1, 0, '000'),

A file like this will fail to load with strict mode.

This could be fixed if we declare on each table which fields are numeric, and take that into account when doing the conversion.
What do you think?
"""
Comment 6 Jonathan Druart 2020-03-20 11:57:19 UTC
Bernardo, I have no idea how to fix that, but having the datatype in the yml sounds wrong to me.
We could retrieve it from the DBIC schema, but that will make the process heavier (time)
Comment 7 Bernardo Gonzalez Kriegel 2020-03-20 12:33:08 UTC
(In reply to Jonathan Druart from comment #6)
> Bernardo, I have no idea how to fix that, but having the datatype in the yml
> sounds wrong to me.
> We could retrieve it from the DBIC schema, but that will make the process
> heavier (time)

Jonathan, yeah, sounds wrong but inevitable I think.
I was faced with the same problem a couple of years ago, when I wrote a script to build sql from xml.
I was thinking now in something like

tables:
  - marc_tag_structure:
      translatable: [ liblibrarian, libopac ]
      numeric: [ repeatable, mandatory ]
      multiline: []
      rows:
        - tagfield: "999"
          liblibrarian: "SYSTEM CONTROL NUMBERS (KOHA)"
          libopac: "SYSTEM CONTROL NUMBERS (KOHA)"
          repeatable: 1
          mandatory: 0
          authorised_value: ""
          frameworkcode: ""

added 'numeric' to list those fields.
That will be ignored everywhere except for yml2sub

Anyway, I liked your other modifications to Bug 24883.
Could you consider to re-add your patches except yml2sql sub?
Let that sub live here until we found a clean way to make it works.
Comment 8 Jonathan Druart 2020-03-20 15:29:09 UTC
Created attachment 101295 [details] [review]
Bug 24912: Add a new yml2sql sub to C4::Installer

It will allow us to generate sql code from the yml files
Comment 9 Jonathan Druart 2020-03-20 15:29:12 UTC
Created attachment 101296 [details] [review]
Bug 24912: Add the ability to convert yaml installer files to SQL queries