Item10520: QuerySearch for \"SomeText\" ONLY refers to a FieldName, and no longer also a FormName.

QuerySearch is quite explicit about

SEARCH{"Sometext" type=query, in that it says this refers to the field named X, with an example of LastName

however, Fn_SEARCH::verify_formQuery2 has a test that succeeds despite there being no field of that name:

• SEARCH{"TestForm"...

The test succeeds on a topic that contains a form of that name, but this does not gel with the docco, and is really really hard to hoist to, as it means that a scalar string could refer to either a field name, or a form name.

I would prefer to stick with what is doccoed - but clearly need feedback on that. (oh, and some docco in the evaluate methods that might shed light on the issue.

trunkwork atm.

-- SvenDowideit - 22 Mar 2011

Fix it to align with the docco... sounds way too sticky otherwise. Unless there's a good reason not to (I don't know why you'd search for form usage this way).

In fact I don't think I've seen a "Blah" type="query" at all (but I only have 3 users that have ever written QuerySearch, so...

-- PaulHarvey - 22 Mar 2011

of course, I would also rather the parser would turn this into a real query, rather than leaving us hanging on another exception case.

-- SvenDowideit - 22 Mar 2011

It is a real query, roughly equivalent to form.name='TestForm'. Note that you can see this using %QUERY{"ItemTemplate"}% -> ==

To explain. All calls to evaluate return a result which is one of an array, a hash, a scalar, or undef. A form name used in isolation is spotted by getField, which maps it to an array of the fields in the form. This is context dependent behaviour, because getField depends on being able to find out the name of the form in the queried topic. In this case, TestForm is resolved by getField to an array of form fields, which is non-null and therefore true. Unfortunately the parser cannot rewrite this query because the context information is only available when evaluating the query against a specific topic, so it has no way to know if it is a form name, or a field name.

IMHO making the code fit the docco is not an option (not only is it complex to do in the code, it is inconsistent and potentially very confusing to have form names as "second class citizens" that sometimes resolve, and sometimes do not, and it also does nothing to resolve the context dependency issue). Because of the problem in resolving the context dependency during hoisting, one alternative is to entirely remove support for the form name. This is a major change that might potentially break end user applications, but without it I can't see how we can ever support full hoisting.

-- CrawfordCurrie - 06 Apr 2011

Dropping support for FooForm.field accessors.. Hmm. Perhaps there's another way. Could we geek up the syntax a little...

• MyForm: - referring to form.name='MyForm'
• MyForm:MyField - referring to fields[name='MyField' AND form='MyForm'].value

I'm quite keen to get multi dataforms support into Foswiki...

-- PaulHarvey - 07 Apr 2011

Geeking up the syntax is one option, though it's kinda like a logical next step after breaking everything as I described.

I'm going to try to write a BNF for the query language that resolves the context of a name in such a way as it is unambiguous whether it is a form name or a field name.

-- CrawfordCurrie - 07 Apr 2011

Cool!

-- PaulHarvey - 07 Apr 2011

as discussed with Crawford on IRC

It is not a real query, in that SEARCH{"Sometext" type=query is roughly equivalent to form.name='Sometext' OR field.name='Sometext'.

and that in many ways, its way too vague a 'statement' to really be allowed to be this way.

its error prone, as the user might have meant a word or regex search - without any logic, its vauge, and its not a query for a single thing - what about a form that has a field of the same name??

additionally, it makes it even harder for users to grok - as the QUERY macro does not in fact return what SEARCH is evaluating.

and lastly - this kind of vaguery makes it much more difficult for search backends to generate queries.

imo there are (again) 2 issues -

1. the query language needs to be simple and unambiguous, and attempts need to be made to recognise fat-finger-errors
2. the output parse tree should be simplified and rigourously defined, documented and tested, to ensure that writers of backends have a snowballs hope.

wrt multi-forms - eeeek

-- SvenDowideit - 07 Apr 2011

Arguing about what constitutes a "real query" isn't helpful, so I'm not going to.

After writing a BNF and giving it some serious thought, I have tightened up the rules regarding where a name can be interpreted as a form name, and where it must be a field (or subscript). This has led to pulling a bunch of functionality back into Node.pm from QueryAlgorithm.pm, and adding a getForm method to QueryAlgorithm. This way the query algorithm is in no doubt as to whether it is being asked for a form or a field.

-- CrawfordCurrie - 07 Apr 2011

well. humpf.

your commit has broken all the hoisters, including the hoistRE one - trunk.f.o is broken.

its a pain that your commit isn't just a fix for this task, but also a much bigger change to the parse tree from binary to n-ary (as that is what has broken the hoisters).

and an additional pain that i was thinking about doing this too, but didn't want to break everyones work without talking about it first.

given that the HoistRE tests are failing, and that trunk is broken, this piggback isn't complete yet - please use Item10612: make OP_and, OP_or and OP_comma parse tree nodes n-ary and commit those things to it.

(yes, I've been spending time getting mongodb to work with your deep modification, so i'm not going to revert it.)

-- SvenDowideit - 08 Apr 2011

I disabled folding in this checkin; will check it in under Item10612 when I'm sure the hoisting works with it.

-- CrawfordCurrie - 08 Apr 2011

Calling this closed

-- SvenDowideit - 07 Jun 2011

Commenting is disabled while not logged in