Add query context #22
Conversation
tsmacdonald
force-pushed
the
parts-of-speech
branch
from
60f2858 to
45173d3
Compare
There was a problem hiding this comment.
Please enjoy my post hoc peanut throwing! Looks good overall, but I do wonder whether you won't need to disambiguate which sub-select etc at some stage. We can wait and see I guess, and digging down into the visitedItem for this kind of thing is still possible in the Clojure layer.
| private void pushContext(QueryContext c) { | ||
| this.contextStack.push(c.toString()); | ||
| } |
There was a problem hiding this comment.
| WHERE; | ||
|
|
||
| public String toString() { | ||
| return name().toUpperCase(); |
There was a problem hiding this comment.
nit: A trivial memory optimization is to pre-calculate these strings to a field in a a constructor.
| @@ -252,10 +272,19 @@ public void invokeCallback(CallbackKey key, Object visitedItem) { | |||
| IFn callback = this.callbacks.get(key); | |||
| if (callback != null) { | |||
| //noinspection unchecked | |||
| this.acc = (Acc) callback.invoke(acc, visitedItem); | |||
| this.acc = (Acc) callback.invoke(acc, visitedItem, this.contextStack.toArray()); | |||
There was a problem hiding this comment.
It doesn't seem like you're using the "two headed" nature of the deque, but you're copying twice for each callback (.toArray and then vec). Just using a cons list to avoid (at least) the first copy seems simpler.
On this topic, the order of the contexts in the vector is the opposite of what I'd expect - I'd expect it to go from outside in, roughly reflecting the lexical structure. Is there a practical reason they're listed in the opposite direction?
Noisy diff since everything is a
{:component original-thing, :context [...]}now, but I think the actual semantics of the changes are pretty straighforward.JSqlParser is inventing some phantom elements (especially JOINs) and I decided it was better to just roll with it for now and let the clients deal with it. Better to have too much info than not enough.
Things were getting hairy so I added a CHANGELOG. Keeping the version numbers correct is a bit of a pain (if a PR merges before this one I'll have to increment the version)