Bug 24912

Summary: Generate SQL from YAML installer files
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Command-line UtilitiesAssignee: Jonathan Druart <jonathan.druart>
Status: ASSIGNED --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: bgkriegel, ere.maijala, robin
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13897
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 24883    
Bug Blocks:    
Attachments: Bug 24912: Centralize code to load YAML installer files to a sub
Bug 24912: Move to a flat array with all values to an array of arrayref
Bug 24912: Add a new yml2sql sub to C4::Installer
Bug 24912: Move new sub from load_yaml.pl
Bug 24912: Add a new yml2sql sub to C4::Installer
Bug 24912: Add the ability to convert yaml installer files to SQL queries

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
Comment 10 Katrin Fischer 2024-01-06 16:23:48 UTC
Is this one still needed now that we already moved all the installers to YAML?