Skip to content

List Support for SQL IN Clause #434

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Sung-Heon
Copy link
Contributor

Description

This PR adds support for List encoding and resolves mapping issues when using ArrayList with SQL IN clauses.

Key Changes

Added ListEncoder Support in BinaryRowEncoder - Enables conversion of List types to MySQL length-encoded comma-separated strings
Created ListEncoder Tests - Comprehensive test suite verifying various list encoding scenarios (empty lists, single/multiple items, null filtering, etc.)
Implemented ArrayListInClause Tests - Demonstrates the issue with ArrayList in SQL IN clauses and provides two solutions:
Using correct SQL placeholders

Related Issue

#289

@Sung-Heon
Copy link
Contributor Author

@oshai can you help me with this?

Run gradle/wrapper-validation-action@e6e38bacfdf1a337459f332974bb2327a31aaf4b
  with:
    min-wrapper-count: 1
    allow-snapshots: false
  env:
    JAVA_HOME: /opt/hostedtoolcache/Java_Adopt_jdk/8.0.45[2](https://github.com/jasync-sql/jasync-sql/actions/runs/15515837938/job/43682601835#step:4:2)-9/x64

What do I have to fix?

@oshai
Copy link
Contributor

oshai commented Jun 8, 2025

@oshai can you help me with this?

Run gradle/wrapper-validation-action@e6e38bacfdf1a337459f332974bb2327a31aaf4b
  with:
    min-wrapper-count: 1
    allow-snapshots: false
  env:
    JAVA_HOME: /opt/hostedtoolcache/Java_Adopt_jdk/8.0.45[2](https://github.com/jasync-sql/jasync-sql/actions/runs/15515837938/job/43682601835#step:4:2)-9/x64

What do I have to fix?

I just rerun it, hth.

@Sung-Heon
Copy link
Contributor Author

@oshai Thx. Please review my code then.😃

@oshai
Copy link
Contributor

oshai commented Jun 18, 2025

I guess for the long run it makes more sense to support in (?) with list support?

can mysql client/server protocol pass a list object in one placeholder like that?

@Sung-Heon
Copy link
Contributor Author

@oshai You're right. In JDBC

It supports the SQL ARRAY data type. However, MySQL doesn't have an ARRAY type, unlike PostgreSQL.

So, perhaps I can adapt this code for PostgreSQL?

@oshai
Copy link
Contributor

oshai commented Jun 19, 2025

yes, I think postgres will better fit here. for mysql you can see what the jdbc mysql driver does (it still needs to support arrays) and do something similar.

@Sung-Heon
Copy link
Contributor Author

Sung-Heon commented Jun 19, 2025

Hi @oshai,

Proposal: Safe List Expansion for IN Clauses

After careful review, our most robust and safest path forward is to support list expansion for IN clauses exclusively when using named parameters, across all supported databases. This approach aligns with how Spring JDBC handles such scenarios.

Why Automatic Expansion for Positional Placeholders is Unsafe

Attempting to automatically expand positional ? placeholders is inherently unsafe due to the tight coupling it creates between the Java parameter order and the SQL query's structure.

Consider a query like:

SET status = ? WHERE id IN (?)

If the library were to automatically expand IN (?), it would have to guess the developer's intent based on the parameter order. Should the SQL ever be refactored (e.g., to WHERE id IN (?) SET status = ?), a developer would also need to remember to reorder the Java parameters accordingly. This introduces fragility and makes the code difficult to maintain.

The Advantage of Named Parameters

Named parameters, such as :ids, completely eliminate this risk by removing any dependency on parameter order. This ensures clarity and maintainability.

Final Implementation Plan

Based on this analysis, our final implementation plan is as follows:

  • For Named Parameter Queries: We will implement client-side rewriting.
    • For example, IN (:ids) will be expanded to IN (?, ?, ?) for databases like MySQL.
  • For Positional ? Queries: We will not implement any automatic expansion. Users will be expected to build the IN clause manually. This maintains the standard, predictable, and safe behavior.

This approach allows us to provide a powerful convenience feature—list expansion in IN clauses—in a predictable and safe manner, without the inherent dangers of adding "magic" to ambiguous placeholders.


What are your thoughts on this direction?

@oshai
Copy link
Contributor

oshai commented Jun 19, 2025

Currently the driver do not support named params at all. But when using with the r2dbc adapter it add such support iirc.

So I don't know if adding such support is beneficial. It will require us in the client side to parse the statement which we don't do right now (might impct performance, be fragile, etc')

So I would start with just supporting it in postgres by sending array param.

@Sung-Heon
Copy link
Contributor Author

Screenshot 2025-06-19 at 6 25 12 PM

@oshai ok~ I see.

So in postgres does it gonna change like = ANY(array['Ava', 'Dave']) ?

@oshai
Copy link
Contributor

oshai commented Jun 20, 2025

Not sure I follow.

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