Skip to content

Allow NamedParameterJdbcTemplate to optionally include parameters in comments of SQL #22255

Closed as not planned

Description

@benelog

NamedParameterJdbcTemplate skips parameters in comments by the logic in NamedParameterUtils.parseSqlStatement(String sql)

...
			int skipToPosition = i;
			while (i < statement.length) {
				skipToPosition = skipCommentsAndQuotes(statement, i);
				if (i == skipToPosition) {
					break;
				}
				else {
					i = skipToPosition;
				}
			}

...

I use NamedParameterJdbcTemplate to run SQLs on our own DBMS engine in the company.
I have special cases to use parameters in comment, for example, "/*@ queryCkey(:cafeId) */ SELECT ... ",

If it is allowed in NamedParameterJdbcTemplate, it will be very helpful for our system.
( For your information, MyBatis allows parameters in comments of SQL)

I'd like you to consider adding an optional flag to support this.

var template = new NamedParameterJdbcTemplate(dataSource);
template.setAllowParametersInComments(true);

Activity

  1. pashazadeh commented on Jan 17, 2019

    @pashazadeh

    Setting this kind of parameters is not supported by PreparedStatements, because they change the execution plan of the query. Cause major databases support which support prepared-statements just parse the query once, so they don't accept placeholders for what changes the execution plan.

  2. benelog commented on Jan 21, 2019

    @benelog
    ContributorAuthor

    @pashazadeh
    I understand your concerns. It is reasonable default to disable parameters in comments.

    However, if it is customizable, it will be very helpful for developers who use a special DBMS like us.
    If the JDBC driver allows it, I think it's unnatural to prevent it in the Spring framework. So, MyBatis also seems to allow this.

    Since Spring framework is traditionally highly customizable, I expect that.

  3. junoyoon commented on Jan 21, 2019

    @junoyoon

    Or.. I think it can be solved by making skipCommentsAndQuotes protected and non-static so that it can be extended in the sub-class of NamedParemterJdbcTemplate .

  4. changed the title [-] Allow NamedParemterJdbcTemplate to optionally include parameters in comments of SQL[/-] [+] Allow NamedParameterJdbcTemplate to optionally include parameters in comments of SQL[/+] on Jul 19, 2020
  5. added this to the General Backlog milestone on Sep 21, 2023
  6. snicoll commented on Jan 8, 2024

    @snicoll
    Member

    The problem is that we delegate everything to NamedParameterUtils that is static at the moment. I think we'd probably need to make it non static and offer some way to customize it.

    With the interesting feedback of @pashazadeh, I am wondering though if it would be a good idea to support it.

    @benelog can't you use simple ? for those advanced use cases, rather than named parameters?

  7. benelog commented on Jan 9, 2024

    @benelog
    ContributorAuthor

    @snicoll
    Unfortunately, all queries in our system require parameters within SQL hints.
    We use our company's own DBMS system, and this DBMS requires that the shard key be included in the hint.
    The cost of giving up the NamedParameterJdbcTemplate is high in this case.

    We are currently working around this issue by creating a class that inherited the NamedParameterJdbcTemplate.
    It is also possible to maintain this workaround.
    I raised this issue because there is an internal opinion that it would be beneficial if NamedParameterJdbcTemplate could directly support what is already allowed in MyBatis.

    If we are the only ones who want this feature, it would be okay for you to consider implementing it when other users request it too.

  8. snicoll commented on Jan 11, 2024

    @snicoll
    Member

    We are currently working around this issue by creating a class that inherited the NamedParameterJdbcTemplate.

    Can you share your workaround?

  9. benelog commented on Jan 14, 2024

    @benelog
    ContributorAuthor

    @snicoll
    I'm sharing it with you for reference to the code below.

    I think it's just workaround, not a natural way.

    import java.util.List;
    import java.util.regex.Pattern;
    
    import javax.sql.DataSource;
    
    import org.apache.commons.lang3.ObjectUtils;
    import org.springframework.core.convert.converter.Converter;
    import org.springframework.jdbc.core.PreparedStatementCreatorFactory;
    import org.springframework.jdbc.core.SqlParameter;
    import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
    import org.springframework.jdbc.core.namedparam.NamedParameterUtils;
    import org.springframework.jdbc.core.namedparam.ParsedSql;
    import org.springframework.jdbc.core.namedparam.SqlParameterSource;
    import org.springframework.lang.Nullable;
    import org.springframework.util.ConcurrentLruCache;
    
    import com.navercorp.nbaset.spring.jdbc.comment.NbaseNodeCommentGenerator;
    
    // NamedParameterJdbcTemplate parameters in comment issue
    // https://github.com/spring-projects/spring-framework/issues/22255
    public class NbaseNamedParameterJdbcTemplate extends NamedParameterJdbcTemplate
    	implements NbaseNamedParameterJdbcOperations {
    	private static final Pattern CKEY_COMMENT_PATTERN = Pattern.compile("(/\\*@.*?\\*/)",
    		Pattern.MULTILINE | Pattern.DOTALL);
    
    	@Nullable
    	private final NbaseNodeCommentGenerator commentGenerator;
    	private final Converter<String, String> sqlConverter;
    	private volatile ConcurrentLruCache<String, ParsedSql> parsedSqlCache =
    		new ConcurrentLruCache<>(DEFAULT_CACHE_LIMIT, this::createParsedSqlInstantWithComment);
    
    	public NbaseNamedParameterJdbcTemplate(DataSource dataSource) {
    		this(dataSource, null, null);
    	}
    
    	public NbaseNamedParameterJdbcTemplate(DataSource dataSource,
    		@Nullable NbaseNodeCommentGenerator commentGenerator) {
    		this(dataSource, commentGenerator, null);
    	}
    
    	public NbaseNamedParameterJdbcTemplate(
    		DataSource dataSource,
    		@Nullable NbaseNodeCommentGenerator commentGenerator,
    		Converter<String, String> sqlConverter) {
    
    		super(dataSource);
    		this.commentGenerator = commentGenerator;
    		this.sqlConverter = ObjectUtils.defaultIfNull(sqlConverter, sql -> sql);
    	}
    
    	@Override
    	public int getCacheLimit() {
    		return this.parsedSqlCache.capacity();
    	}
    
    	@Override
    	public void setCacheLimit(int cacheLimit) {
    		this.parsedSqlCache = new ConcurrentLruCache<>(cacheLimit, this::createParsedSqlInstantWithComment);
    	}
    
    	@Override
    	protected ParsedSql getParsedSql(String sql) {
    		return this.parsedSqlCache.get(sql);
    	}
    
    	@Override
    	protected PreparedStatementCreatorFactory getPreparedStatementCreatorFactory(
    		ParsedSql parsedSql, SqlParameterSource paramSource) {
    
    		String sqlToUse = NamedParameterUtils.substituteNamedParameters(parsedSql, paramSource);
    		sqlToUse = this.sqlConverter.convert(sqlToUse);
    		List<SqlParameter> declaredParameters = NamedParameterUtils.buildSqlParameterList(parsedSql, paramSource);
    		return new PreparedStatementCreatorFactory(sqlToUse, declaredParameters);
    	}
    
    	private ParsedSql createParsedSqlInstantWithComment(String sql) {
    		if (this.commentGenerator != null && !CKEY_COMMENT_PATTERN.matcher(sql).find()) {
    			sql = this.commentGenerator.queryComment() + sql;
    		}
    
    		return NbaseNamedParameterUtils.createParsedSqlInstantWithComment(sql);
    	}
    }
    import static java.util.Comparator.comparingInt;
    import static java.util.Objects.requireNonNull;
    import static org.springframework.util.ReflectionUtils.accessibleConstructor;
    import static org.springframework.util.ReflectionUtils.findField;
    import static org.springframework.util.ReflectionUtils.getField;
    import static org.springframework.util.ReflectionUtils.setField;
    
    import java.lang.reflect.Field;
    import java.lang.reflect.InvocationTargetException;
    import java.util.ArrayList;
    import java.util.List;
    import java.util.regex.Matcher;
    import java.util.regex.Pattern;
    
    import org.apache.commons.lang3.StringUtils;
    import org.springframework.jdbc.core.namedparam.NamedParameterUtils;
    import org.springframework.jdbc.core.namedparam.ParsedSql;
    
    public class NbaseNamedParameterUtils {
    	private static final Field PARSED_SQL_PARAMETER_INDEXES = findAndMakeAccessible("parameterIndexes");
    	private static final Field PARSED_SQL_PARAMETER_NAMES = findAndMakeAccessible("parameterNames");
    	private static final Field PARSED_SQL_NAMED_PARAMETER_COUNT = findAndMakeAccessible("namedParameterCount");
    	private static final Field PARSED_SQL_UNNAMED_PARAMETER_COUNT = findAndMakeAccessible("unnamedParameterCount");
    	private static final Field PARSED_SQL_TOTAL_PARAMETER_COUNT = findAndMakeAccessible("totalParameterCount");
    
    	public static ParsedSql createParsedSqlInstantWithComment(String sql) {
    		ParsedSql parsedSql = NamedParameterUtils.parseSqlStatement(sql);
    		List<String> commentParameterNames = getFieldValue(parsedSql, PARSED_SQL_PARAMETER_NAMES);
    		List<int[]> commentParameterIndexes = getFieldValue(parsedSql, PARSED_SQL_PARAMETER_INDEXES);
    		int totalCount = getFieldValue(parsedSql, PARSED_SQL_TOTAL_PARAMETER_COUNT);
    		int namedCount = getFieldValue(parsedSql, PARSED_SQL_NAMED_PARAMETER_COUNT);
    		int unnamedCount = getFieldValue(parsedSql, PARSED_SQL_UNNAMED_PARAMETER_COUNT);
    
    		for (MatchedComment each : MatchedComment.findComments(sql)) {
    			ParsedSql parsedComment = NamedParameterUtils.parseSqlStatement(each.getUncommented());
    			commentParameterNames.addAll(getFieldValue(parsedComment, PARSED_SQL_PARAMETER_NAMES));
    			commentParameterIndexes.addAll(getFieldValue(parsedComment, PARSED_SQL_PARAMETER_INDEXES));
    			int commentTotalParamCount = getFieldValue(parsedComment, PARSED_SQL_TOTAL_PARAMETER_COUNT);
    			int commentNamedParamCount = getFieldValue(parsedComment, PARSED_SQL_NAMED_PARAMETER_COUNT);
    			int commentUnnamedParamCount = getFieldValue(parsedComment, PARSED_SQL_UNNAMED_PARAMETER_COUNT);
    			totalCount += commentTotalParamCount;
    			namedCount += commentNamedParamCount;
    			unnamedCount += commentUnnamedParamCount;
    		}
    		return createParsedSqlInstance(sql, commentParameterNames, commentParameterIndexes, totalCount, namedCount, unnamedCount);
    	}
    
    	private static ParsedSql createParsedSqlInstance(
    		String originalSql,
    		List<String> commentParameterNames,
    		List<int[]> commentParameterIndexes,
    		int totalCount,
    		int namedCount,
    		int unnamedCount) {
    
    		try {
    			ParsedSql parsedSql = accessibleConstructor(ParsedSql.class, String.class).newInstance(originalSql);
    
    			List<CommentPair> sorted = sortCommentParameter(requireNonNull(commentParameterNames), commentParameterIndexes);
    			List<String> newCommentParameterNames = new ArrayList<>();
    			List<int[]> newCommentParameterIndexes = new ArrayList<>();
    
    			for (CommentPair commentPair : sorted) {
    				newCommentParameterNames.add(commentPair.getName());
    				newCommentParameterIndexes.add(commentPair.getIndex());
    			}
    
    			setField(PARSED_SQL_PARAMETER_NAMES, parsedSql, newCommentParameterNames);
    			setField(PARSED_SQL_PARAMETER_INDEXES, parsedSql, newCommentParameterIndexes);
    			setField(PARSED_SQL_TOTAL_PARAMETER_COUNT, parsedSql, totalCount);
    			setField(PARSED_SQL_NAMED_PARAMETER_COUNT, parsedSql, namedCount);
    			setField(PARSED_SQL_UNNAMED_PARAMETER_COUNT, parsedSql, unnamedCount);
    			return parsedSql;
    		} catch (NoSuchMethodException | InstantiationException | IllegalAccessException | InvocationTargetException e) {
    			throw new NbaseParseSqlException("Parse sql reflection exception. sql: " + originalSql, e);
    		}
    	}
    
    	private static List<CommentPair> sortCommentParameter(
    		List<String> commentParameterNames, List<int[]> commentParameterIndexes) {
    
    		List<CommentPair> commentParameters = new ArrayList<>();
    		for (int i = 0, size = commentParameterNames.size(); i < size; i++) {
    			int[] index = commentParameterIndexes.get(i);
    			commentParameters.add(CommentPair.of(commentParameterNames.get(i), index));
    		}
    		commentParameters.sort(comparingInt(l -> l.getIndex()[0]));
    		return commentParameters;
    	}
    
    	private static Field findAndMakeAccessible(String fieldName) {
    		Field field = requireNonNull(findField(ParsedSql.class, fieldName));
    		field.setAccessible(true);
    		return field;
    	}
    
    	@SuppressWarnings("unchecked")
    	private static <T> T getFieldValue(ParsedSql parsedSql, Field field) {
    		return (T)requireNonNull(getField(field, parsedSql));
    	}
    
    
    	static class CommentPair {
    		private String name;
    		private int[] index;
    
    		static CommentPair of(String name, int[] index) {
    			CommentPair pair = new CommentPair();
    			pair.name = name;
    			pair.index = index;
    			return pair;
    		}
    
    		public String getName() {
    			return this.name;
    		}
    
    		public int[] getIndex() {
    			return this.index;
    		}
    	}
    
    	static class MatchedComment {
    		private static final Pattern COMMENT_PATTERN = Pattern.compile("(/\\*@.*?\\*/)", Pattern.MULTILINE | Pattern.DOTALL);
    
    		String original;
    		String text;
    		int start;
    		int end;
    
    		static List<MatchedComment> findComments(String sql) {
    			List<MatchedComment> matches = new ArrayList<>();
    			Matcher matcher = MatchedComment.COMMENT_PATTERN.matcher(sql);
    			while (matcher.find()) {
    				MatchedComment match = new MatchedComment();
    				match.original = sql;
    				match.text = matcher.group();
    				match.start = matcher.start();
    				match.end = matcher.end();
    				matches.add(match);
    			}
    			return matches;
    		}
    
    		String getUncommented() {
    			return StringUtils.repeat(" ", start) + "   " + text.substring(3, text.length() - 2) + "  ";
    		}
    	}
    }
  10. added a commit that references this issue on Jul 26, 2026
    e18980e
  11. bclozel commented on Oct 23, 2026

    @bclozel
    Member

    Superseded by ##37088

  12. removed this from the General Backlog milestone on Oct 23, 2026
  13. added
    status: supersededAn issue that has been superseded by another
    and removed on Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    in: dataIssues in data modules (jdbc, orm, oxm, tx)status: supersededAn issue that has been superseded by anothertype: enhancementA general enhancement

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions