This is an automated email from the ASF dual-hosted git repository. desruisseaux pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
The following commit(s) were added to refs/heads/geoapi-4.0 by this push: new 6d57ee4af0 Tune SQL LIKE search escape by being more conservative when no escape is known. Also escape the escape character itself. 6d57ee4af0 is described below commit 6d57ee4af0cd5204c029e24b947c53a4867c4691 Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Wed Mar 19 00:27:33 2025 +0100 Tune SQL LIKE search escape by being more conservative when no escape is known. Also escape the escape character itself. --- .../apache/sis/metadata/sql/privy/SQLBuilder.java | 12 ++++++------ .../apache/sis/metadata/sql/privy/SQLUtilities.java | 16 +++++++++------- .../org/apache/sis/metadata/sql/privy/Syntax.java | 21 +++++++++++++-------- .../sis/metadata/sql/privy/SQLUtilitiesTest.java | 11 +++++++++++ .../apache/sis/storage/sql/feature/Database.java | 2 +- .../sis/storage/sql/feature/TableAnalyzer.java | 2 +- 6 files changed, 41 insertions(+), 23 deletions(-) diff --git a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/metadata/sql/privy/SQLBuilder.java b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/metadata/sql/privy/SQLBuilder.java index 9c8c60629f..e914f49685 100644 --- a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/metadata/sql/privy/SQLBuilder.java +++ b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/metadata/sql/privy/SQLBuilder.java @@ -310,16 +310,16 @@ public class SQLBuilder extends Syntax { * @return this builder, for method call chaining. * * @see #escapeWildcards(String) + * @see #canEscapeWildcards() */ public final SQLBuilder appendWildcardEscaped(final String value) { - if (cannotEscapeWildcards()) { - buffer.append(value.replace("%", "_")); - } else { - final int start = buffer.length(); - buffer.append(value); + final int start = buffer.length(); + buffer.append(value); + if (canEscapeWildcards()) { + final char escapeChar = escape.charAt(0); for (int i = buffer.length(); --i >= start;) { final char c = buffer.charAt(i); - if (c == '_' || c == '%') { + if (c == '_' || c == '%' || (c == escapeChar && value.startsWith(escape, i))) { buffer.insert(i, escape); } } diff --git a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/metadata/sql/privy/SQLUtilities.java b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/metadata/sql/privy/SQLUtilities.java index 4e400eeaa9..814630e5e1 100644 --- a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/metadata/sql/privy/SQLUtilities.java +++ b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/metadata/sql/privy/SQLUtilities.java @@ -103,23 +103,25 @@ public final class SQLUtilities extends Static { * * <h4>Missing escape characters</h4> * Some databases do not provide an escape character. If the given {@code escape} is null or empty, - * then instead of escaping, this method will replace all occurrences of {@code '%'} by {@code '_'}. - * It will cause the database to return more metadata rows that desired. Callers should filter by + * then this method conservatively returns the pattern unchanged, with the wildcards still active. + * It will cause the database to return more metadata rows than desired. Callers should filter by * comparing the table and schema name specified in each row against the original {@code name}. * + * <p>Note: {@code '%'} could be replaced by {@code '_'} for reducing the number of false positives. + * However, if a database provides no escape character, maybe it does not support wildcards at all. + * Leaving the text unchanged and doing the filtering in the caller's code is more conservative.</p> + * * @param text the text to escape for use in a context equivalent to the {@code LIKE} statement. * @param escape value of {@link DatabaseMetaData#getSearchStringEscape()}. May be null or empty. * @return the given text with wildcard characters escaped. */ public static String escape(final String text, final String escape) { - if (text != null) { - if (escape == null || escape.isEmpty()) { - return text.replace("%", "_"); - } + if (text != null && escape != null && !escape.isEmpty()) { + final char escapeChar = escape.charAt(0); StringBuilder buffer = null; for (int i = text.length(); --i >= 0;) { final char c = text.charAt(i); - if (c == '_' || c == '%') { + if (c == '_' || c == '%' || (c == escapeChar && text.startsWith(escape, i))) { if (buffer == null) { buffer = new StringBuilder(text); } diff --git a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/metadata/sql/privy/Syntax.java b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/metadata/sql/privy/Syntax.java index 8021ffe8dd..3db59a8239 100644 --- a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/metadata/sql/privy/Syntax.java +++ b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/metadata/sql/privy/Syntax.java @@ -104,10 +104,14 @@ public class Syntax { * * <h4>Missing escape characters</h4> * Some databases do not provide an escape character. If the given {@code escape} is null or empty, - * then instead of escaping, this method will replace all occurrences of {@code '%'} by {@code '_'}. - * It will cause the database to return more metadata rows that desired. Callers should filter by + * then this method conservatively returns the pattern unchanged, with the wildcards still active. + * It will cause the database to return more metadata rows than desired. Callers should filter by * comparing the table and schema name specified in each row against the original {@code name}. * + * <p>Note: {@code '%'} could be replaced by {@code '_'} for reducing the number of false positives. + * However, if a database provides no escape character, maybe it does not support wildcards at all. + * Leaving the text unchanged and doing the filtering in the caller's code is more conservative.</p> + * * @param text the text to escape for use in a context equivalent to the {@code LIKE} statement. * @return the given text with wildcard characters escaped. */ @@ -116,15 +120,16 @@ public class Syntax { } /** - * Returns {@code true} if the database can <em>not</em> escape wildcard characters. + * Returns {@code false} if the database can <em>not</em> escape wildcard characters. * In such case, the string returned by {@link #escapeWildcards(String)} may produce - * false positive, and the callers need to apply additional filtering. + * false positives, and the caller needs to apply additional filtering. * - * <p>This is rarely true, but may happen with incomplete <abbr>JDBC</abbr> drivers.</p> + * <p>This method returns {@code true} for the vast majority of major databases, + * but it may return {@code false} with incomplete <abbr>JDBC</abbr> drivers.</p> * - * @return whether the database can <em>not</em> escape wildcard characters. + * @return whether the database can escape wildcard characters. */ - public final boolean cannotEscapeWildcards() { - return (escape == null) || escape.isEmpty(); + public final boolean canEscapeWildcards() { + return (escape != null) && !escape.isEmpty(); } } diff --git a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/metadata/sql/privy/SQLUtilitiesTest.java b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/metadata/sql/privy/SQLUtilitiesTest.java index 192e3e787f..538b5d2144 100644 --- a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/metadata/sql/privy/SQLUtilitiesTest.java +++ b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/metadata/sql/privy/SQLUtilitiesTest.java @@ -34,6 +34,17 @@ public final class SQLUtilitiesTest extends TestCase { public SQLUtilitiesTest() { } + /** + * Tests {@link SQLUtilities#escape(String, String)}. + */ + @Test + public void testEscape() { + assertEquals("foo", SQLUtilities.escape("foo", "\\")); + assertEquals("foo\\_biz\\%bar", SQLUtilities.escape("foo_biz%bar", "\\")); + assertEquals("foo\\\\bar", SQLUtilities.escape("foo\\bar", "\\")); + assertEquals("foo#!#!bar#not", SQLUtilities.escape("foo#!bar#not", "#!")); + } + /** * Tests {@link SQLUtilities#toLikePattern(String, int, int, boolean, boolean, StringBuilder)}. */ diff --git a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Database.java b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Database.java index 4ee4872dc0..b00d58183b 100644 --- a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Database.java +++ b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Database.java @@ -352,7 +352,7 @@ public class Database<G> extends Syntax { */ String crsTable = null; final var ignoredTables = new HashMap<String,Boolean>(8); - final boolean isSearchReliable = !cannotEscapeWildcards(); + final boolean isSearchReliable = canEscapeWildcards(); final SpatialSchema[] candidates = getPossibleSpatialSchemas(ignoredTables); for (int i=0; i<candidates.length; i++) { final SpatialSchema convention = candidates[i]; diff --git a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/TableAnalyzer.java b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/TableAnalyzer.java index 78d4d15b7f..728e8e1d9c 100644 --- a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/TableAnalyzer.java +++ b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/TableAnalyzer.java @@ -70,7 +70,7 @@ final class TableAnalyzer extends FeatureAnalyzer { this.dependencyOf = dependencyOf; this.tableEsc = analyzer.database.escapeWildcards(id.table); this.schemaEsc = analyzer.database.escapeWildcards(id.schema); - isSearchReliable = !analyzer.database.cannotEscapeWildcards() + isSearchReliable = analyzer.database.canEscapeWildcards() || (tableEsc == id.table && schemaEsc == id.schema); // Identity checks are okay. try (ResultSet reflect = analyzer.metadata.getPrimaryKeys(id.catalog, id.schema, id.table)) {