From 9492e7c6e2c45812efe20d23594a719b041246c5 Mon Sep 17 00:00:00 2001 From: lance Date: Thu, 23 Oct 2025 22:14:20 +0800 Subject: [PATCH 1/3] FilterExpressionTextParser treats all number literals as Integer, failing on Long values Signed-off-by: lance --- .../filter/FilterExpressionTextParser.java | 28 +++--- .../FilterExpressionTextParserTests.java | 87 +++++++++++-------- 2 files changed, 69 insertions(+), 46 deletions(-) diff --git a/spring-ai-vector-store/src/main/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParser.java b/spring-ai-vector-store/src/main/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParser.java index 77f31aaad6a..65c9e8c252f 100644 --- a/spring-ai-vector-store/src/main/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParser.java +++ b/spring-ai-vector-store/src/main/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParser.java @@ -16,12 +16,6 @@ package org.springframework.ai.vectorstore.filter; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CopyOnWriteArrayList; - import org.antlr.v4.runtime.ANTLRErrorStrategy; import org.antlr.v4.runtime.BailErrorStrategy; import org.antlr.v4.runtime.BaseErrorListener; @@ -30,7 +24,6 @@ import org.antlr.v4.runtime.RecognitionException; import org.antlr.v4.runtime.Recognizer; import org.antlr.v4.runtime.misc.ParseCancellationException; - import org.springframework.ai.vectorstore.filter.antlr4.FiltersBaseVisitor; import org.springframework.ai.vectorstore.filter.antlr4.FiltersLexer; import org.springframework.ai.vectorstore.filter.antlr4.FiltersParser; @@ -38,11 +31,16 @@ import org.springframework.core.NestedExceptionUtils; import org.springframework.util.Assert; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; + /** - * * Parse a textual, vector-store agnostic, filter expression language into * {@link Filter.Expression}. - * + *

* The vector-store agnostic, filter expression language is defined by a formal ANTLR4 * grammar (Filters.g4). The language looks and feels like a subset of the well known SQL * WHERE filter expressions. For example, you can use the parser like this: @@ -161,7 +159,9 @@ public void clearCache() { this.cache.clear(); } - /** For testing only */ + /** + * For testing only + */ Map getCache() { return this.cache; } @@ -202,7 +202,13 @@ private String removeOuterQuotes(String in) { @Override public Filter.Operand visitIntegerConstant(FiltersParser.IntegerConstantContext ctx) { - return new Filter.Value(Integer.valueOf(ctx.getText())); + String text = ctx.getText(); + try { + return new Filter.Value(Integer.parseInt(text)); + } + catch (NumberFormatException ignored) { + return new Filter.Value(Long.parseLong(text)); + } } @Override diff --git a/spring-ai-vector-store/src/test/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParserTests.java b/spring-ai-vector-store/src/test/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParserTests.java index 15f6d2b54d3..9ab18ef8c29 100644 --- a/spring-ai-vector-store/src/test/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParserTests.java +++ b/spring-ai-vector-store/src/test/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParserTests.java @@ -16,15 +16,18 @@ package org.springframework.ai.vectorstore.filter; -import java.util.List; - import org.junit.jupiter.api.Test; - +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.springframework.ai.vectorstore.filter.Filter.Expression; import org.springframework.ai.vectorstore.filter.Filter.Group; import org.springframework.ai.vectorstore.filter.Filter.Key; import org.springframework.ai.vectorstore.filter.Filter.Value; +import java.util.List; +import java.util.stream.Stream; + import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.ai.vectorstore.filter.Filter.ExpressionType.AND; import static org.springframework.ai.vectorstore.filter.Filter.ExpressionType.EQ; @@ -39,54 +42,55 @@ /** * @author Christian Tzolov * @author Sun Yuhan + * @author lance */ -public class FilterExpressionTextParserTests { +class FilterExpressionTextParserTests { FilterExpressionTextParser parser = new FilterExpressionTextParser(); @Test - public void testEQ() { + void testEQ() { // country == "BG" Expression exp = this.parser.parse("country == 'BG'"); assertThat(exp).isEqualTo(new Expression(EQ, new Key("country"), new Value("BG"))); - assertThat(this.parser.getCache().get("WHERE " + "country == 'BG'")).isEqualTo(exp); + assertThat(this.parser.getCache()).containsEntry("WHERE " + "country == 'BG'", exp); } @Test - public void tesEqAndGte() { + void tesEqAndGte() { // genre == "drama" AND year >= 2020 Expression exp = this.parser.parse("genre == 'drama' && year >= 2020"); assertThat(exp).isEqualTo(new Expression(AND, new Expression(EQ, new Key("genre"), new Value("drama")), new Expression(GTE, new Key("year"), new Value(2020)))); - assertThat(this.parser.getCache().get("WHERE " + "genre == 'drama' && year >= 2020")).isEqualTo(exp); + assertThat(this.parser.getCache()).containsEntry("WHERE " + "genre == 'drama' && year >= 2020", exp); } @Test - public void tesIn() { + void tesIn() { // genre in ["comedy", "documentary", "drama"] Expression exp = this.parser.parse("genre in ['comedy', 'documentary', 'drama']"); assertThat(exp) .isEqualTo(new Expression(IN, new Key("genre"), new Value(List.of("comedy", "documentary", "drama")))); - assertThat(this.parser.getCache().get("WHERE " + "genre in ['comedy', 'documentary', 'drama']")).isEqualTo(exp); + assertThat(this.parser.getCache()).containsEntry("WHERE " + "genre in ['comedy', 'documentary', 'drama']", exp); } @Test - public void testNe() { + void testNe() { // year >= 2020 OR country == "BG" AND city != "Sofia" Expression exp = this.parser.parse("year >= 2020 OR country == \"BG\" AND city != \"Sofia\""); assertThat(exp).isEqualTo(new Expression(OR, new Expression(GTE, new Key("year"), new Value(2020)), new Expression(AND, new Expression(EQ, new Key("country"), new Value("BG")), new Expression(NE, new Key("city"), new Value("Sofia"))))); - assertThat(this.parser.getCache().get("WHERE " + "year >= 2020 OR country == \"BG\" AND city != \"Sofia\"")) - .isEqualTo(exp); + assertThat(parser.getCache()) + .containsEntry("WHERE " + "year >= 2020 OR country == \"BG\" AND city != \"Sofia\"", exp); } @Test - public void testGroup() { + void testGroup() { // (year >= 2020 OR country == "BG") AND city NIN ["Sofia", "Plovdiv"] Expression exp = this.parser.parse("(year >= 2020 OR country == \"BG\") AND city NIN [\"Sofia\", \"Plovdiv\"]"); @@ -95,13 +99,12 @@ public void testGroup() { new Expression(EQ, new Key("country"), new Value("BG")))), new Expression(NIN, new Key("city"), new Value(List.of("Sofia", "Plovdiv"))))); - assertThat(this.parser.getCache() - .get("WHERE " + "(year >= 2020 OR country == \"BG\") AND city NIN [\"Sofia\", \"Plovdiv\"]")) - .isEqualTo(exp); + assertThat(parser.getCache()) + .containsEntry("WHERE " + "(year >= 2020 OR country == \"BG\") AND city NIN [\"Sofia\", \"Plovdiv\"]", exp); } @Test - public void tesBoolean() { + void tesBoolean() { // isOpen == true AND year >= 2020 AND country IN ["BG", "NL", "US"] Expression exp = this.parser.parse("isOpen == true AND year >= 2020 AND country IN [\"BG\", \"NL\", \"US\"]"); @@ -109,12 +112,13 @@ public void tesBoolean() { new Expression(AND, new Expression(EQ, new Key("isOpen"), new Value(true)), new Expression(GTE, new Key("year"), new Value(2020))), new Expression(IN, new Key("country"), new Value(List.of("BG", "NL", "US"))))); - assertThat(this.parser.getCache() - .get("WHERE " + "isOpen == true AND year >= 2020 AND country IN [\"BG\", \"NL\", \"US\"]")).isEqualTo(exp); + + assertThat(parser.getCache()) + .containsEntry("WHERE " + "isOpen == true AND year >= 2020 AND country IN [\"BG\", \"NL\", \"US\"]", exp); } @Test - public void tesNot() { + void tesNot() { // NOT(isOpen == true AND year >= 2020 AND country IN ["BG", "NL", "US"]) Expression exp = this.parser .parse("not(isOpen == true AND year >= 2020 AND country IN [\"BG\", \"NL\", \"US\"])"); @@ -126,13 +130,12 @@ public void tesNot() { new Expression(IN, new Key("country"), new Value(List.of("BG", "NL", "US"))))), null)); - assertThat(this.parser.getCache() - .get("WHERE " + "not(isOpen == true AND year >= 2020 AND country IN [\"BG\", \"NL\", \"US\"])")) - .isEqualTo(exp); + assertThat(parser.getCache()).containsEntry( + "WHERE " + "not(isOpen == true AND year >= 2020 AND country IN [\"BG\", \"NL\", \"US\"])", exp); } @Test - public void tesNotNin() { + void tesNotNin() { // NOT(country NOT IN ["BG", "NL", "US"]) Expression exp = this.parser.parse("not(country NOT IN [\"BG\", \"NL\", \"US\"])"); @@ -141,7 +144,7 @@ public void tesNotNin() { } @Test - public void tesNotNin2() { + void tesNotNin2() { // NOT country NOT IN ["BG", "NL", "US"] Expression exp = this.parser.parse("NOT country NOT IN [\"BG\", \"NL\", \"US\"]"); @@ -150,7 +153,7 @@ public void tesNotNin2() { } @Test - public void tesNestedNot() { + void tesNestedNot() { // NOT(isOpen == true AND year >= 2020 AND NOT(country IN ["BG", "NL", "US"])) Expression exp = this.parser .parse("not(isOpen == true AND year >= 2020 AND NOT(country IN [\"BG\", \"NL\", \"US\"]))"); @@ -164,13 +167,12 @@ public void tesNestedNot() { null))), null)); - assertThat(this.parser.getCache() - .get("WHERE " + "not(isOpen == true AND year >= 2020 AND NOT(country IN [\"BG\", \"NL\", \"US\"]))")) - .isEqualTo(exp); + assertThat(parser.getCache()).containsEntry( + "WHERE " + "not(isOpen == true AND year >= 2020 AND NOT(country IN [\"BG\", \"NL\", \"US\"]))", exp); } @Test - public void testDecimal() { + void testDecimal() { // temperature >= -15.6 && temperature <= +20.13 String expText = "temperature >= -15.6 && temperature <= +20.13"; Expression exp = this.parser.parse(expText); @@ -178,11 +180,11 @@ public void testDecimal() { assertThat(exp).isEqualTo(new Expression(AND, new Expression(GTE, new Key("temperature"), new Value(-15.6)), new Expression(LTE, new Key("temperature"), new Value(20.13)))); - assertThat(this.parser.getCache().get("WHERE " + expText)).isEqualTo(exp); + assertThat(parser.getCache()).containsEntry("WHERE " + expText, exp); } @Test - public void testLong() { + void testLong() { Expression exp2 = this.parser.parse("biz_id == 3L"); Expression exp3 = this.parser.parse("biz_id == -5L"); @@ -191,7 +193,7 @@ public void testLong() { } @Test - public void testIdentifiers() { + void testIdentifiers() { Expression exp = this.parser.parse("'country.1' == 'BG'"); assertThat(exp).isEqualTo(new Expression(EQ, new Key("'country.1'"), new Value("BG"))); @@ -203,9 +205,24 @@ public void testIdentifiers() { } @Test - public void testUnescapedIdentifierWithUnderscores() { + void testUnescapedIdentifierWithUnderscores() { Expression exp = this.parser.parse("file_name == 'medicaid-wa-faqs.pdf'"); assertThat(exp).isEqualTo(new Expression(EQ, new Key("file_name"), new Value("medicaid-wa-faqs.pdf"))); } + @MethodSource("constantConstantProvider") + @ParameterizedTest(name = "{index} => [{0}, expected={1}]") + void testConstants(String expr, Object expectedValue) { + Expression result = parser.parse(expr); + assertThat(result).isEqualTo(new Expression(EQ, new Key("id"), new Value(expectedValue))); + } + + static Stream constantConstantProvider() { + return Stream.of(Arguments.of("id==" + Integer.MAX_VALUE, Integer.MAX_VALUE), + Arguments.of("id==" + Integer.MIN_VALUE, Integer.MIN_VALUE), + Arguments.of("id==" + Long.MAX_VALUE, Long.MAX_VALUE), + Arguments.of("id==" + Long.MIN_VALUE, Long.MIN_VALUE), Arguments.of("id==" + 0x100, 0x100), + Arguments.of("id==" + 1000000000000L, 1000000000000L), Arguments.of("id==" + Math.PI, Math.PI)); + } + } From 3f973bb14f3729c3b6264213efbda71e919d322b Mon Sep 17 00:00:00 2001 From: lance Date: Thu, 23 Oct 2025 22:32:29 +0800 Subject: [PATCH 2/3] FilterExpressionTextParser treats all number literals as Integer, failing on Long values Signed-off-by: lance --- .../filter/FilterExpressionTextParser.java | 12 +++++------ .../FilterExpressionTextParserTests.java | 21 ++++++++++--------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/spring-ai-vector-store/src/main/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParser.java b/spring-ai-vector-store/src/main/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParser.java index 65c9e8c252f..af98f03a774 100644 --- a/spring-ai-vector-store/src/main/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParser.java +++ b/spring-ai-vector-store/src/main/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParser.java @@ -16,6 +16,12 @@ package org.springframework.ai.vectorstore.filter; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; + import org.antlr.v4.runtime.ANTLRErrorStrategy; import org.antlr.v4.runtime.BailErrorStrategy; import org.antlr.v4.runtime.BaseErrorListener; @@ -31,12 +37,6 @@ import org.springframework.core.NestedExceptionUtils; import org.springframework.util.Assert; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CopyOnWriteArrayList; - /** * Parse a textual, vector-store agnostic, filter expression language into * {@link Filter.Expression}. diff --git a/spring-ai-vector-store/src/test/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParserTests.java b/spring-ai-vector-store/src/test/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParserTests.java index 9ab18ef8c29..9222335ded9 100644 --- a/spring-ai-vector-store/src/test/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParserTests.java +++ b/spring-ai-vector-store/src/test/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParserTests.java @@ -16,18 +16,19 @@ package org.springframework.ai.vectorstore.filter; +import java.util.List; +import java.util.stream.Stream; + import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; + import org.springframework.ai.vectorstore.filter.Filter.Expression; import org.springframework.ai.vectorstore.filter.Filter.Group; import org.springframework.ai.vectorstore.filter.Filter.Key; import org.springframework.ai.vectorstore.filter.Filter.Value; -import java.util.List; -import java.util.stream.Stream; - import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.ai.vectorstore.filter.Filter.ExpressionType.AND; import static org.springframework.ai.vectorstore.filter.Filter.ExpressionType.EQ; @@ -85,7 +86,7 @@ void testNe() { new Expression(AND, new Expression(EQ, new Key("country"), new Value("BG")), new Expression(NE, new Key("city"), new Value("Sofia"))))); - assertThat(parser.getCache()) + assertThat(this.parser.getCache()) .containsEntry("WHERE " + "year >= 2020 OR country == \"BG\" AND city != \"Sofia\"", exp); } @@ -99,7 +100,7 @@ void testGroup() { new Expression(EQ, new Key("country"), new Value("BG")))), new Expression(NIN, new Key("city"), new Value(List.of("Sofia", "Plovdiv"))))); - assertThat(parser.getCache()) + assertThat(this.parser.getCache()) .containsEntry("WHERE " + "(year >= 2020 OR country == \"BG\") AND city NIN [\"Sofia\", \"Plovdiv\"]", exp); } @@ -113,7 +114,7 @@ void tesBoolean() { new Expression(GTE, new Key("year"), new Value(2020))), new Expression(IN, new Key("country"), new Value(List.of("BG", "NL", "US"))))); - assertThat(parser.getCache()) + assertThat(this.parser.getCache()) .containsEntry("WHERE " + "isOpen == true AND year >= 2020 AND country IN [\"BG\", \"NL\", \"US\"]", exp); } @@ -130,7 +131,7 @@ void tesNot() { new Expression(IN, new Key("country"), new Value(List.of("BG", "NL", "US"))))), null)); - assertThat(parser.getCache()).containsEntry( + assertThat(this.parser.getCache()).containsEntry( "WHERE " + "not(isOpen == true AND year >= 2020 AND country IN [\"BG\", \"NL\", \"US\"])", exp); } @@ -167,7 +168,7 @@ void tesNestedNot() { null))), null)); - assertThat(parser.getCache()).containsEntry( + assertThat(this.parser.getCache()).containsEntry( "WHERE " + "not(isOpen == true AND year >= 2020 AND NOT(country IN [\"BG\", \"NL\", \"US\"]))", exp); } @@ -180,7 +181,7 @@ void testDecimal() { assertThat(exp).isEqualTo(new Expression(AND, new Expression(GTE, new Key("temperature"), new Value(-15.6)), new Expression(LTE, new Key("temperature"), new Value(20.13)))); - assertThat(parser.getCache()).containsEntry("WHERE " + expText, exp); + assertThat(this.parser.getCache()).containsEntry("WHERE " + expText, exp); } @Test @@ -213,7 +214,7 @@ void testUnescapedIdentifierWithUnderscores() { @MethodSource("constantConstantProvider") @ParameterizedTest(name = "{index} => [{0}, expected={1}]") void testConstants(String expr, Object expectedValue) { - Expression result = parser.parse(expr); + Expression result = this.parser.parse(expr); assertThat(result).isEqualTo(new Expression(EQ, new Key("id"), new Value(expectedValue))); } From 4d655d8fa70860821701a02e5e76017b9ff79fe3 Mon Sep 17 00:00:00 2001 From: lance Date: Thu, 23 Oct 2025 22:36:50 +0800 Subject: [PATCH 3/3] FilterExpressionTextParser treats all number literals as Integer, failing on Long values Signed-off-by: lance --- .../ai/vectorstore/filter/FilterExpressionTextParser.java | 1 + 1 file changed, 1 insertion(+) diff --git a/spring-ai-vector-store/src/main/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParser.java b/spring-ai-vector-store/src/main/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParser.java index af98f03a774..955aecd46f4 100644 --- a/spring-ai-vector-store/src/main/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParser.java +++ b/spring-ai-vector-store/src/main/java/org/springframework/ai/vectorstore/filter/FilterExpressionTextParser.java @@ -30,6 +30,7 @@ import org.antlr.v4.runtime.RecognitionException; import org.antlr.v4.runtime.Recognizer; import org.antlr.v4.runtime.misc.ParseCancellationException; + import org.springframework.ai.vectorstore.filter.antlr4.FiltersBaseVisitor; import org.springframework.ai.vectorstore.filter.antlr4.FiltersLexer; import org.springframework.ai.vectorstore.filter.antlr4.FiltersParser;