#492 verify parameters during setup #719
Conversation
…l supported ML versions
…ported ML versions
…ibute-pair indexes
…es, word lexicons
…of try/catch and eval for path-namespaces
replace eval with xdmp:value
|
Also removed recursion in some cases. Some of the admin functions take a sequence of things to add, and instead of passing in a sequence, we were calling the function multiple times. I simplified the code to just FLWOR over the inputs and remove the recursion, where it made sense to do so. |
| $index/db:scalar-type, | ||
| $index/db:field-name, | ||
| $index/db:collation, | ||
| $index/db:range-value-positions |
There was a problem hiding this comment.
Should this also include $index/db:invalid-values in ML versions after 6?
There was a problem hiding this comment.
it does (line 2344); the if section is versions 6+.
There was a problem hiding this comment.
@dmcassel Wow, I totally misread that one. Deleted a lot of my other comments. Thanks.
| let $existing := admin:database-get-range-field-indexes($admin-config, $database) | ||
| for $expected in $db-config/db:range-field-indexes/db:range-field-index | ||
| return | ||
| if ($existing[fn:deep-equal(., $expected)]) then () |
There was a problem hiding this comment.
Should we also be turning $expected into admin:database-range-field-index before comparison? deep-equal is pretty strict and I would be concerned of formatting/typo issues in ml-config which Roxy ignored but could cause it to fail here.
There was a problem hiding this comment.
I was thinking that since this is for validation, not creating indexes, it would be okay, but the fn:deep-equal would be thrown off by comments or ordering. Good catch, I'll change this.
There was a problem hiding this comment.
I like that calling admin:database-range-field-indexes protects from comments and ordering differences, but I see a different problem. Consider this from ml-config:
<range-field-index>
<scalar-type>dateTime</scalar-type>
<field-name>field-test</field-name>
<range-value-positions>false</range-value-positions>
<invalid>reject</invalid>
</range-field-index>
I've incorrectly specified "invalid" instead of "invalid-values" as the element name. The validate code doesn't catch it, but fn:deep-equal would. Likewise, in any case where the value is optional or an empty string is okay, constructing an element won't work for the comparison, because it will allow the same mistake that the index-creating code makes.
I've also noticed that the validate code isn't actually getting called. setup:validate-install calls setup:validate-databases-indexes, which calls setup:validate-range-field-indexes, but nothing calls setup:validate-install (seems like setup:do-setup should).
In light of the first observation, I propose we keep the comparisons as they are. Based on the second, I suggest we open another issue to address that.
There was a problem hiding this comment.
Agreed. I'm okay with merging this as-is
|
Outside of the (minor) comments I made this looks very good and appears to work as expected. Self-test appears to be working too. |
This ended up being bigger than I expected, but I think the changes are all things we've discussed before. Wherever we were previously relying on having the correct structure in ml-config.xml, the code now calls the appropriate admin function to build a proper structure.
While I was at it, I replaced several try/catch and xdmp:evals with xdmp:value, as @RobertSzkutak and I discussed previously.