Skip to content

Prepared IN queries should support bind markers #1172

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

Closed
samueldlightfoot opened this issue Sep 26, 2021 · 4 comments
Closed

Prepared IN queries should support bind markers #1172

samueldlightfoot opened this issue Sep 26, 2021 · 4 comments
Assignees
Labels
type: enhancement A general enhancement

Comments

@samueldlightfoot
Copy link
Contributor

samueldlightfoot commented Sep 26, 2021

Statements generated for IN queries do not use the bindMarker(?) for the collection of values provided for the IN clause. When using prepared statements this means we are creating a prepared statement for each IN query with different parameters. This soon bloats the prepared statement cache when you are selecting by different parameters.

This parameterization is supported in Cassandra:
https://issues.apache.org/jira/browse/CASSANDRA-4210

Expected:
SELECT * FROM Users WHERE id IN ?

Actual:
SELECT * FROM Users WHERE id IN ('id1', 'id2')

Looking in statement factory IN queries that take a collection are handled differently:

		'case IN:

			if (predicate.getValue() instanceof List
					|| (predicate.getValue() != null && predicate.getValue().getClass().isArray())) {
				**return column.in(toLiterals(predicate.getValue()));**
			}

			return column.in(factory.create(predicate.getValue()));`

Example test: 537f96b

I'm happy to have a go at fixing this if we are in agreement it is a bug.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 26, 2021
@samueldlightfoot samueldlightfoot changed the title Parameterized IN SELECT statements are incorrect Prepared IN queries are not parameterized correctly Sep 27, 2021
mp911de added a commit that referenced this issue Sep 27, 2021
@mp911de
Copy link
Member

mp911de commented Sep 27, 2021

Thanks a lot. I wasn't aware that Cassandra is able to consume collections/arrays within a single placeholder. The issue we have is a bit that the statement factory isn't aware that the statement is going to be used as a prepared statement. Maybe we we can generally provide collections as a single parameter but then the question remains how this would work with inline parameter rendering.

@samueldlightfoot
Copy link
Contributor Author

samueldlightfoot commented Sep 27, 2021

Thanks for the response. Sounds fairly involved and I don't have much familiarity with the codebase. I would really like to fix this but I think it's best I leave it to the professionals :-)

Update: Ideas making use of the BindMarker accepting InRelationBuilder::in.

https://github.com/samueldlightfoot/spring-data-cassandra/tree/1172-in-query-bindmarkers

samueldlightfoot added a commit to samueldlightfoot/spring-data-cassandra that referenced this issue Oct 11, 2021
@samueldlightfoot
Copy link
Contributor Author

Hi Mark, any thoughts on this?

Happy to help out if you have any ideas.

@mp911de
Copy link
Member

mp911de commented Oct 12, 2021

Feel free to submit a pull request. I hadn't had the bandwidth yet to wrap my head around it.

samueldlightfoot added a commit to samueldlightfoot/spring-data-cassandra that referenced this issue Oct 12, 2021
@mp911de mp911de added this to the 3.2.6 (2021.0.6) milestone Oct 13, 2021
@mp911de mp911de changed the title Prepared IN queries are not parameterized correctly Prepared IN queries should support bind markers Oct 13, 2021
@mp911de mp911de added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 13, 2021
@mp911de mp911de linked a pull request Oct 13, 2021 that will close this issue
4 tasks
mp911de added a commit that referenced this issue Oct 13, 2021
Introduce TermFactory.canBindCollection() to avoid unecessary parameter encoding.

See #1172
Original pull request: #1178.
mp911de pushed a commit that referenced this issue Oct 13, 2021
mp911de added a commit that referenced this issue Oct 13, 2021
Introduce TermFactory.canBindCollection() to avoid unecessary parameter encoding.

See #1172
Original pull request: #1178.
mp911de pushed a commit that referenced this issue Oct 13, 2021
mp911de added a commit that referenced this issue Oct 13, 2021
Introduce TermFactory.canBindCollection() to avoid unecessary parameter encoding.

See #1172
Original pull request: #1178.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants