Bug importing config from scripts where value is NULL #208

Closed
SchumacherFM opened this Issue Jun 8, 2016 · 7 comments

Comments

Projects
None yet
3 participants
@SchumacherFM

Hi there,

my config script contains some exported database NULLs as you can see below:

config:set "web/unsecure/base_media_url"    NULL
config:set "web/unsecure/base_static_url"   NULL

Importing those scripts will result NOT in a real database NULL, we get a string NULL.

select * from core_config_data where value = 'NULL';
+-----------+---------+----------+------------------------------------------------------+-------+
| config_id | scope   | scope_id | path                                                 | value |
+-----------+---------+----------+------------------------------------------------------+-------+
|        65 | default |        0 | crontab/default/jobs/catalog_product_alert/run/model | NULL  |
|        78 | default |        0 | general/store_information/merchant_vat_number        | NULL  |
|        89 | default |        0 | web/cookie/cookie_path                               | NULL  |
|        92 | default |        0 | web/secure/base_media_url                            | NULL  |
|        93 | default |        0 | web/secure/base_static_url                           | NULL  |
|       104 | default |        0 | web/unsecure/base_media_url                          | NULL  |
|       105 | default |        0 | web/unsecure/base_static_url                         | NULL  |
+-----------+---------+----------+------------------------------------------------------+-------+
7 rows in set (0.00 sec)

I suggests that the value NULL in a text file gets interpreted as a database NULL.

What do you think?

@tkn98

This comment has been minimized.

Show comment
Hide comment
@tkn98

tkn98 Jun 8, 2016

Contributor

To prevent ambiguities, this needs a flag or even multiple. Quick sketch:

--null     # do not accept value, set path with database null-value
--null-as-null   # interpret the value string "NULL" as database null-value

Otherwise if that is possible currently to have NULL values, we should have it.

Contributor

tkn98 commented Jun 8, 2016

To prevent ambiguities, this needs a flag or even multiple. Quick sketch:

--null     # do not accept value, set path with database null-value
--null-as-null   # interpret the value string "NULL" as database null-value

Otherwise if that is possible currently to have NULL values, we should have it.

@SchumacherFM

This comment has been minimized.

Show comment
Hide comment
@SchumacherFM

SchumacherFM Sep 15, 2016

Any updates here?

Any updates here?

@ktomk

This comment has been minimized.

Show comment
Hide comment
@ktomk

ktomk Sep 15, 2016

Contributor

Not yet, but I think there might be some use to invert the flag. Using the string "NULL" (all caps) should be treated as NULL value unless --no-null (or similar) is used. That would allow to set an entry to NULL while passing data from a textfile into the command (e.g. hsccd or other scripts).

Contributor

ktomk commented Sep 15, 2016

Not yet, but I think there might be some use to invert the flag. Using the string "NULL" (all caps) should be treated as NULL value unless --no-null (or similar) is used. That would allow to set an entry to NULL while passing data from a textfile into the command (e.g. hsccd or other scripts).

@SchumacherFM

This comment has been minimized.

Show comment
Hide comment
@SchumacherFM

SchumacherFM Jan 24, 2017

I would have needed that today ... ;-)

I would have needed that today ... ;-)

@ktomk ktomk added the enhancement label Jan 25, 2017

ktomk added a commit to ktomk/n98-magerun2 that referenced this issue Jan 25, 2017

Support setting config values to NULL, tasks #208
It was previously not possible to use config:set with NULL/unkown values
which are supported in the Magento config database store like so:

    config:set "web/unsecure/base_media_url"    NULL
    config:set "web/unsecure/base_static_url"   NULL

This commit adds support to set the null value unless --no-null is used
which will insert the string "NULL" which is the old behavior.

Additionally NULL values are properly reflected when displaying config data
via config:get making them more visible and even the concrete type in
JSON format.

Refs:

- #208

- Command: config:set

- Command: config:get

ktomk added a commit that referenced this issue Jan 25, 2017

Support setting config values to NULL, tasks #208
It was previously not possible to use config:set with NULL/unkown values
which are supported in the Magento config database store like so:

    config:set "web/unsecure/base_media_url"    NULL
    config:set "web/unsecure/base_static_url"   NULL

This commit adds support to set the null value unless --no-null is used
which will insert the string "NULL" which is the old behavior.

Additionally NULL values are properly reflected when displaying config data
via config:get making them more visible and even the concrete type in
JSON format.

Refs:

- #208

- Command: config:set

- Command: config:get
@ktomk

This comment has been minimized.

Show comment
Hide comment
@ktomk

ktomk Jan 25, 2017

Contributor

But you can only have it tomorrow ... ;-). It's not the prettiest code and quite quickly written as in the end the output formats needed to be dealt with (config:get). I therefore covered it with tests for the scenarios which came to my mind.

You can use --no-null with config:set for the old behaviour if you need to have NULL as string. I think this is unlikely, but it is supported.


We believe that the enhancement asked for is available in the development version now. It can be upgraded to it using the self-update command with the --unstable switch.

Contributor

ktomk commented Jan 25, 2017

But you can only have it tomorrow ... ;-). It's not the prettiest code and quite quickly written as in the end the output formats needed to be dealt with (config:get). I therefore covered it with tests for the scenarios which came to my mind.

You can use --no-null with config:set for the old behaviour if you need to have NULL as string. I think this is unlikely, but it is supported.


We believe that the enhancement asked for is available in the development version now. It can be upgraded to it using the self-update command with the --unstable switch.

@ktomk ktomk closed this Jan 25, 2017

@SchumacherFM

This comment has been minimized.

Show comment
Hide comment
@SchumacherFM

SchumacherFM Jan 25, 2017

You're awesome! Thank you very much!

You're awesome! Thank you very much!

@ktomk

This comment has been minimized.

Show comment
Hide comment
@ktomk

ktomk Feb 11, 2017

Contributor

@SchumacherFM: For those versions of Magento 1 which do support NULL in config table values, this feature has been backported to Magerun 1 (netz98/n98-magerun@cb1bd42). Just FYI.

Contributor

ktomk commented Feb 11, 2017

@SchumacherFM: For those versions of Magento 1 which do support NULL in config table values, this feature has been backported to Magerun 1 (netz98/n98-magerun@cb1bd42). Just FYI.

cmuench added a commit to cmuench/n98-magerun2 that referenced this issue Oct 11, 2017

Support setting config values to NULL, tasks #208
It was previously not possible to use config:set with NULL/unkown values
which are supported in the Magento config database store like so:

    config:set "web/unsecure/base_media_url"    NULL
    config:set "web/unsecure/base_static_url"   NULL

This commit adds support to set the null value unless --no-null is used
which will insert the string "NULL" which is the old behavior.

Additionally NULL values are properly reflected when displaying config data
via config:get making them more visible and even the concrete type in
JSON format.

Refs:

- #208

- Command: config:set

- Command: config:get
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment