Resolve #64 (Allow to discover available fields for a document)

[fbilhaut]

@ostafen, just wondering if what I assume are "reserved" fields like "__id" should be included by GetKeys(). Currently they are, but I think they might preferably not. In such case, is it clearly documented that field names starting with "_" are supposed to be reserved ? Let me know

[ostafen]

I think this can be implemented without the need of the extra prefix parameter, by concatenating key + "." to each element of result

[fbilhaut]

I think communicating this information to sub-calls is mandatory in such a recursive implementation. I might be wrong, but I'm afraid this is "mathematically" unavoidable. Otherwise level n call has no way to know about the parent nodes that have been visited during level 0..n-1 calls.

That being said, I made it basic and recursive for the sake of simplicity, and because the depth of the traversal will always be marginal which doesn't justify the overhaul of an iterative version. But of course we could make it iterative and avoid an extra parameter.

BTW I moved it to utils.go, although it could be made more generic (specify separator, avoid having to pass the empty prefix at first call, etc.)

[ostafen]

I think communicating this information to sub-calls is mandatory in such a recursive implementation. I might be wrong, but I'm afraid this is "mathematically" unavoidable. Otherwise level n call has no way to know about the parent nodes that have been visited during level 0..n-1 calls.

Communicating this information is mandatory in your implementation, because you want each subcall to return the "full" field name, also considering its parent. However, when you solve a recursive sub-problem, you should avoid to embed information of the whole problem inside the sub-problem.

Give a look at this:

func getKeys(fields map[string]interface{}) []string {
	result := []string{}
	for key, value := range fields {
		subMap, isMap := value.(map[string]interface{})
		if isMap {
			subFields := getKeys(subMap)
			for _, subKey := range subFields {
				result = append(result, key+"."+subKey)
			}
		} else {
			result = append(result, key)
		}
	}
	return result
}

Please, simply call the function getKeys(), also.

[fbilhaut]

Hi Stefano,

It works, but this is pretty ineffective to re-scan each sub-map. Transferring the prefix just takes a bit on the stack but has no additional computational time. That being said, that's your project, so you're the boss :) But to me performance is key in such a lib.

As for the name, you already have a getKeys() function in compare.go with the same signature. This one doesn't deep-traverse the map, and sorts the result. I'm not fond of the "Recursive" prefix as well but...

[ostafen]

If we were to be so concerned on the performance of this, I think we would't go for a recursive implementation at all, so it is a bit overkilled to me thinking at optimization at this level. Moreover, we are not talking of a function affecting query execution time, so it's fine to trade some cpu cycle for simplicity.

Regarding the function name, you could call it geAllKeys().

[fbilhaut]

OK Stefano, let's not enter a battle of opinions about this. That's just not my way of designing code and algorithms, but I'll push a last commit this way to close the subject. (BTW recursive implementations are not ineffective per se, but that's another story.)

[ostafen]

Hi, @fbilhaut, thank you for contributing. I requested you some changes. I think that special attributes, such as "_id" should be included

[ostafen]

Thank you! Just one more thing, and we're done

[ostafen]

Thank you!