feat: search with multiple values #132
Conversation
sjvans
changed the title
sjvans
requested review from
BobdenOs,
David-Kunz,
patricebender,
danjoa,
gregorwolf,
johannes-vogel,
vobu,
stewsk and
sebastianesch
as code owners
| args = (args || []).map(e => { | ||
| if (e === '*') return e | ||
| const arg = { __proto__: e, toString: (x = e) => this.expr(x) } | ||
| if ('xpr' in arg) | ||
| arg.xpr = arg.xpr.map(e => (typeof e === 'string' ? e : { __proto__: e, toString: (x = e) => this.expr(x) })) | ||
| return arg | ||
| }) |
There was a problem hiding this comment.
Why do we need that in addition to the {xpr} handling in Functions.search()?
There was a problem hiding this comment.
this.tolower(arg) called for xpr element. becomes [object Object] instead of ? in the sql.
| if (!('val' in arg)) throw `SQLite only supports single value arguments for $search` | ||
| if ('xpr' in arg) { | ||
| let xpr = '' | ||
| for (const each of arg.xpr) xpr += typeof each === 'string' ? ` ${each} ` : this.search(ref, each) |
There was a problem hiding this comment.
Don't we need an 'and' for strings?
There was a problem hiding this comment.
$search=foo bar (= $search=foo AND bar)
--> xpr = [{ val: 'foo' }, 'and', { val: 'bar' }]
--> sql snippet x = ? and x = ?, where x = ? is the result of this.search(ref, each)
|
Would be cool to have this, whats speaking against it? |
|
We need to converge all related discussions and decisions. One of that was in a recent spec meeting: pass through We don't have to — and likely shouldn't try to — support full-fledged search on non-HANA dbs; we just should not crash. |
|
@danjoa @sjvans any news on this PR? With @cap-js/hana now being released this "regression" is now also introduced that search strings with whitespace in Fiori elements no longer work. In my opinion at least for HANA /prod scenarios this needs to be enabled by default to not have this regression, which causes frustration on user side. |
possible solution for #127