Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query operator contains #337

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

koriwi
Copy link
Contributor

@koriwi koriwi commented Sep 17, 2019

had this changing lying around in my fork for quite some time.
I need to use this every day :)

@jorgebay
Copy link
Contributor

oh good one, thanks for the patch!

I've created a ticket to track this: https://datastax-oss.atlassian.net/browse/NODEJS-553

I would like to include it in the upcoming release (in the next few days / week), we should add a unit test for this feature in test/unit/mapping/model-mapper-select-tests.js.

Could you add one or two tests? Otherwise, I'll try to do it myself.

@jorgebay
Copy link
Contributor

Now that I think about it (after your post in the mailing list), I think supporting CONTAINS operator without secondary indexes doesn't make much sense, the only other way to use CONTAINS in CQL outside secondary indexes is using ALLOW FILTERING queries that we explicitly don't want to support.

From the user perspective, I would recommend to use custom queries on the mapper.

About the contribution, I would say we can focus in supporting indexes in the Mapper w/ this operators. As I mentioned in the mailing list, its a meaty feature that requires several unit and integration tests to make sure we properly support it.

@koriwi
Copy link
Contributor Author

koriwi commented Sep 18, 2019

Cool. As i said in the mailing list, i will try to implement the needed test for secondary indexes. I can just update this pull request so we have a pull request which contains CONTAINS, CONTAINS KEY and secondary index support :)
Got some open source time from my company

@jorgebay
Copy link
Contributor

glad to hear!

yeah, let's keep this one open.

@koriwi koriwi force-pushed the queryOperator_contains branch from 91ecb11 to c54053b Compare September 26, 2019 11:12
@koriwi koriwi force-pushed the queryOperator_contains branch from c54053b to cf45f18 Compare September 26, 2019 11:16
@jorgebay jorgebay changed the base branch from master to main June 16, 2020 08:14
@jorgebay jorgebay changed the base branch from main to master June 16, 2020 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants