Convert SHOW FIELD KEYS to the new query engine
#5815
Conversation
|
This converts |
jsternberg
force-pushed
the
js-5579-show-field-keys
branch
2 times, most recently
from
f5a1063 to
76c3f3f
Compare
| } | ||
| return newMeasurementKeysIterator(sh, fn, opt) | ||
| } | ||
|
|
There was a problem hiding this comment.
Injecting a function into another iterator type seems like it makes the implementation harder to follow in order to reuse a few lines of code. Why not make a separate fieldKeysIterator and tagKeysIterator?
There was a problem hiding this comment.
Mostly because I was copying verbatim large portions of functionality with exactly one line change. So I decided to change both of the iterators to use the same base since they both did the same thing.
There was a problem hiding this comment.
The newMeasurementKeysIterator is pretty small. I'd prefer separate types but I'll leave it up to you.
|
Is this PR still WIP? |
|
It can be rebased now that remote exec has been merged. I'll do that and test it in a cluster now. |
jsternberg
force-pushed
the
js-5579-show-field-keys
branch
from
76c3f3f to
d7886f6
Compare
jsternberg
force-pushed
the
js-5579-show-field-keys
branch
from
d7886f6 to
aa0b603
Compare
jsternberg
changed the title
SHOW FIELD KEYS to the new query engineSHOW FIELD KEYS to the new query engine
|
This is now ready for review and it works with the remote exec and some extra fixes that had to go in for that. I've rebased on top of the needed commits. |
|
lgtm, merge on 🍏 |
Convert `SHOW FIELD KEYS` to the new query engine
Fixes #5579.