s/vector?/sequential?/ when applying transform functions #115

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

The transform function code decides whether or not to use map by
testing with vector?, which breaks when the result set is a seq
instead, as is the case when you do a query of the form
(select foos (with bars)). As far as I can tell this could be
fixed either by replacing the vector? test with a sequential?
test, or using a mapv in apply-posts (and maybe elsewhere).

This commit implements the former, and adds a test.

s/vector?/sequential?/ when applying transform functions
The transform function code decides whether or not to use `map` by
testing with `vector?`, which breaks when the result set is a seq
instead, as is the case when you do a query of the form
`(select foos (with bars))`. As far as I can tell this could be
fixed either by replacing the `vector?` test with a `sequential?`
test, or using a `mapv` in `apply-posts` (and maybe elsewhere).

This commit implements the former, and adds a test.

After browsing the code a minimal amount I haven't seen why apply-transforms should even expect that it might have a single map instead of a sequence, but decided to not question and leave that as is, since it doesn't seem to have any tests.

Owner

AlexBaranosky commented Jan 21, 2013

Sets aren't sequential?, they are however coll? I'll have to take a look at this later tonight.

I didn't realize sets might be used. Would swapping sequential? for map?
(and swapping the clauses) work?
On Jan 21, 2013 5:51 PM, "AlexBaranosky" notifications@github.com wrote:

Sets aren't sequential?, they are however coll? I'll have to take a look
at this later tonight.


Reply to this email directly or view it on GitHubhttps://github.com/korma/Korma/pull/115#issuecomment-12523927.

Owner

AlexBaranosky commented Jan 22, 2013

Honestly, I'm not comfortable with that particular code enough yet... and
there are no tests of it, so I'll have to think about it at home some more.

On Mon, Jan 21, 2013 at 3:58 PM, fredericksgary notifications@github.comwrote:

I didn't realize sets might be used. Would swapping sequential? for map?
(and swapping the clauses) work?
On Jan 21, 2013 5:51 PM, "AlexBaranosky" notifications@github.com
wrote:

Sets aren't sequential?, they are however coll? I'll have to take a look
at this later tonight.


Reply to this email directly or view it on GitHub<
https://github.com/korma/Korma/pull/115#issuecomment-12523927>.


Reply to this email directly or view it on GitHubhttps://github.com/korma/Korma/pull/115#issuecomment-12524155.

Member

immoh commented Jun 8, 2015

This appears to have been fixed in commit 9b66180

@immoh immoh closed this Jun 8, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment