-
Notifications
You must be signed in to change notification settings - Fork 614
[jdbc-v2] Fix statement impl #2471
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
Merged
Merged
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
f6d9cfa
Implemented closing resultsets
chernser 33a5678
implemented largeMaxRows()
chernser 0e09ff7
drafted implementation of the SQLUtils and quoting literals and ident…
chernser d4d8613
reworking code of the statement impl
chernser a9515ae
Merge branch 'main' into fix_statement_impl
chernser 8333046
85% Line coverage 83% Branch coverage
chernser 1451bc7
fixed wrong test
chernser f8494c1
Merge branch 'main' into fix_statement_impl
chernser 8ad1aab
fixed datatype test. fixed getUpdateCount()
chernser d7341a4
fixed multiresult logic
chernser 8c3c2d7
fixed PreparedStatement tests
chernser c8a61a1
Merge branch 'main' into fix_statement_impl
chernser 7775fee
fixed unit & integration coverage report
chernser 517a0cc
fixed typos and tests
chernser File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
143 changes: 143 additions & 0 deletions
143
clickhouse-jdbc/src/test/java/com/clickhouse/client/api/sql/SQLUtilsTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| package com.clickhouse.client.api.sql; | ||
|
|
||
| import com.clickhouse.jdbc.internal.SqlParser; | ||
| import org.testng.annotations.DataProvider; | ||
| import org.testng.annotations.Test; | ||
|
|
||
| import static org.testng.Assert.assertEquals; | ||
|
|
||
| @Test(groups = {"unit"}) | ||
| public class SQLUtilsTest { | ||
| // Test data for enquoteLiteral | ||
| @DataProvider(name = "enquoteLiteralTestData") | ||
| public Object[][] enquoteLiteralTestData() { | ||
| return new Object[][] { | ||
| // input, expected output | ||
| {"test 123", "'test 123'"}, | ||
| {"こんにちは世界", "'こんにちは世界'"}, | ||
| {"O'Reilly", "'O''Reilly'"}, | ||
| {"😊👍", "'😊👍'"}, | ||
| {"", "''"}, | ||
| {"single'quote'double''quote\"", "'single''quote''double''''quote\"'"} | ||
| }; | ||
| } | ||
|
|
||
| // Test data for enquoteIdentifier | ||
| @DataProvider(name = "enquoteIdentifierTestData") | ||
| public Object[][] enquoteIdentifierTestData() { | ||
| return new Object[][] { | ||
| // input, expected output | ||
| {"column1", "\"column1\""}, | ||
| {"table.name", "\"table.name\""}, | ||
| {"column with spaces", "\"column with spaces\""}, | ||
| {"column\"with\"quotes", "\"column\"\"with\"\"quotes\""}, | ||
| {"UPPERCASE", "\"UPPERCASE\""}, | ||
| {"1column", "\"1column\""}, | ||
| {"column-with-hyphen", "\"column-with-hyphen\""}, | ||
| {"😊👍", "\"😊👍\""}, | ||
| {"", "\"\""} | ||
| }; | ||
| } | ||
|
|
||
| @Test(dataProvider = "enquoteLiteralTestData") | ||
| public void testEnquoteLiteral(String input, String expected) { | ||
| assertEquals(SQLUtils.enquoteLiteral(input), expected); | ||
| } | ||
|
|
||
| @Test(expectedExceptions = IllegalArgumentException.class) | ||
| public void testEnquoteLiteral_NullInput() { | ||
| SQLUtils.enquoteLiteral(null); | ||
| } | ||
|
|
||
| @Test(dataProvider = "enquoteIdentifierTestData") | ||
| public void testEnquoteIdentifier(String input, String expected) { | ||
| // Test with quotesRequired = true (always quote) | ||
| assertEquals(SQLUtils.enquoteIdentifier(input), expected); | ||
| assertEquals(SQLUtils.enquoteIdentifier(input, true), expected); | ||
|
|
||
| // Test with quotesRequired = false (quote only if needed) | ||
| boolean needsQuoting = !input.matches("[a-zA-Z_][a-zA-Z0-9_]*"); | ||
| String expectedUnquoted = needsQuoting ? expected : input; | ||
| assertEquals(SQLUtils.enquoteIdentifier(input, false), expectedUnquoted); | ||
| } | ||
|
|
||
| @Test(expectedExceptions = IllegalArgumentException.class) | ||
| public void testEnquoteIdentifier_NullInput() { | ||
| SQLUtils.enquoteIdentifier(null); | ||
| } | ||
|
|
||
| @Test(expectedExceptions = IllegalArgumentException.class) | ||
| public void testEnquoteIdentifier_NullInput_WithQuotesRequired() { | ||
| SQLUtils.enquoteIdentifier(null, true); | ||
| } | ||
|
|
||
| @Test | ||
| public void testEnquoteIdentifier_NoQuotesWhenNotNeeded() { | ||
| // These identifiers don't need quoting | ||
| String[] simpleIdentifiers = { | ||
| "column1", "table_name", "_id", "a1b2c3", "ColumnName" | ||
| }; | ||
|
|
||
| for (String id : simpleIdentifiers) { | ||
| // With quotesRequired=false, should return as-is | ||
| assertEquals(SQLUtils.enquoteIdentifier(id, false), id); | ||
| // With quotesRequired=true, should be quoted | ||
| assertEquals(SQLUtils.enquoteIdentifier(id, true), "\"" + id + "\""); | ||
| } | ||
| } | ||
|
|
||
| @DataProvider(name = "simpleIdentifierTestData") | ||
| public Object[][] simpleIdentifierTestData() { | ||
| return new Object[][] { | ||
| // identifier, expected result | ||
| {"Hello", true}, | ||
| {"hello_world", true}, | ||
| {"Hello123", true}, | ||
| {"H", true}, // minimum length | ||
| {"a".repeat(128), true}, // maximum length | ||
|
|
||
| // Test cases from requirements | ||
| {"G'Day", false}, | ||
| {"\"\"Bruce Wayne\"\"", false}, | ||
| {"GoodDay$", false}, | ||
| {"Hello\"\"World", false}, | ||
| {"\"\"Hello\"\"World\"\"", false}, | ||
|
|
||
| // Additional test cases | ||
| {"", false}, // empty string | ||
| {"123test", false}, // starts with number | ||
| {"_test", false}, // starts with underscore | ||
| {"test-name", false}, // contains hyphen | ||
| {"test name", false}, // contains space | ||
| {"test\"name", false}, // contains quote | ||
| {"test.name", false}, // contains dot | ||
| {"a".repeat(129), false}, // exceeds max length | ||
| {"testName", true}, | ||
| {"TEST_NAME", true}, | ||
| {"test123", true}, | ||
| {"t123", true}, | ||
| {"t", true} | ||
| }; | ||
| } | ||
|
|
||
| @Test(dataProvider = "simpleIdentifierTestData") | ||
| public void testIsSimpleIdentifier(String identifier, boolean expected) { | ||
| assertEquals(SQLUtils.isSimpleIdentifier(identifier), expected, | ||
| String.format("Failed for identifier: %s", identifier)); | ||
| } | ||
|
|
||
| @Test(expectedExceptions = IllegalArgumentException.class) | ||
| public void testIsSimpleIdentifier_NullInput() { | ||
| SQLUtils.isSimpleIdentifier(null); | ||
| } | ||
|
|
||
| @Test | ||
| public void testUnquoteIdentifier() { | ||
| String[] names = new String[]{"test", "`test name1`", "\"test name 2\""}; | ||
| String[] expected = new String[]{"test", "test name1", "test name 2"}; | ||
|
|
||
| for (int i = 0; i < names.length; i++) { | ||
| assertEquals(SQLUtils.unquoteIdentifier(names[i]), expected[i]); | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
131 changes: 131 additions & 0 deletions
131
client-v2/src/main/java/com/clickhouse/client/api/sql/SQLUtils.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| package com.clickhouse.client.api.sql; | ||
|
|
||
| import java.util.regex.Matcher; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| public class SQLUtils { | ||
| /** | ||
| * Escapes and quotes a string literal for use in SQL queries. | ||
| * | ||
| * @param str the string to be quoted, cannot be null | ||
| * @return the quoted and escaped string | ||
| * @throws IllegalArgumentException if the input string is null | ||
| */ | ||
| public static String enquoteLiteral(String str) { | ||
| if (str == null) { | ||
| throw new IllegalArgumentException("Input string cannot be null"); | ||
| } | ||
| return "'" + str.replace("'", "''") + "'"; | ||
| } | ||
|
|
||
| /** | ||
| * Escapes and quotes an SQL identifier (e.g., table or column name) by enclosing it in double quotes. | ||
| * Any existing double quotes in the identifier are escaped by doubling them. | ||
| * | ||
| * @param identifier the identifier to be quoted, cannot be null | ||
| * @param quotesRequired if false, the identifier will only be quoted if it contains special characters | ||
| * @return the quoted and escaped identifier, or the original identifier if quoting is not required | ||
| * @throws IllegalArgumentException if the input identifier is null | ||
| */ | ||
| public static String enquoteIdentifier(String identifier, boolean quotesRequired) { | ||
| if (identifier == null) { | ||
| throw new IllegalArgumentException("Identifier cannot be null"); | ||
| } | ||
|
|
||
| if (!quotesRequired && !needsQuoting(identifier)) { | ||
| return identifier; | ||
| } | ||
| return "\"" + identifier.replace("\"", "\"\"") + "\""; | ||
| } | ||
|
|
||
| /** | ||
| * Escapes and quotes an SQL identifier, always adding quotes. | ||
| * | ||
| * @param identifier the identifier to be quoted, cannot be null | ||
| * @return the quoted and escaped identifier | ||
| * @throws IllegalArgumentException if the input identifier is null | ||
| * @see #enquoteIdentifier(String, boolean) | ||
| */ | ||
| public static String enquoteIdentifier(String identifier) { | ||
| return enquoteIdentifier(identifier, true); | ||
| } | ||
|
|
||
| /** | ||
| * Checks if an identifier needs to be quoted. | ||
| * An identifier needs quoting if it: | ||
| * - Is empty | ||
| * - Contains any non-alphanumeric characters except underscore | ||
| * - Starts with a digit | ||
| * - Is a reserved keyword (not implemented in this basic version) | ||
| * | ||
| * @param identifier the identifier to check | ||
| * @return true if the identifier needs to be quoted, false otherwise | ||
| */ | ||
| private static boolean needsQuoting(String identifier) { | ||
| if (identifier.isEmpty()) { | ||
| return true; | ||
| } | ||
|
|
||
| // Check if first character is a digit | ||
| if (Character.isDigit(identifier.charAt(0))) { | ||
| return true; | ||
| } | ||
|
|
||
| // Check all characters are alphanumeric or underscore | ||
| for (int i = 0; i < identifier.length(); i++) { | ||
| char c = identifier.charAt(i); | ||
| if (!(Character.isLetterOrDigit(c) || c == '_')) { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the given string is a valid simple SQL identifier that doesn't require quoting. | ||
| * A simple identifier must: | ||
| * <ul> | ||
| * <li>Not be null or empty</li> | ||
| * <li>Be between 1 and 128 characters in length (inclusive)</li> | ||
| * <li>Start with an alphabetic character (a-z, A-Z)</li> | ||
| * <li>Contain only alphanumeric characters or underscores</li> | ||
| * <li>Not be enclosed in double quotes</li> | ||
| * </ul> | ||
| * | ||
| * @param identifier the identifier to check | ||
| * @return true if the identifier is a valid simple SQL identifier, false otherwise | ||
| * @throws IllegalArgumentException if the input identifier is null | ||
| */ | ||
| // Compiled pattern for simple SQL identifiers | ||
| private static final java.util.regex.Pattern SIMPLE_IDENTIFIER_PATTERN = | ||
| java.util.regex.Pattern.compile("^[a-zA-Z][a-zA-Z0-9_]{0,127}$"); | ||
|
|
||
| /** | ||
| * Checks if the given string is a valid simple SQL identifier using a compiled regex pattern. | ||
| * A simple identifier must match the pattern: ^[a-zA-Z][a-zA-Z0-9_]{0,127}$ | ||
| * | ||
| * @param identifier the identifier to check | ||
| * @return true if the identifier is a valid simple SQL identifier, false otherwise | ||
| * @throws IllegalArgumentException if the input identifier is null | ||
| */ | ||
| public static boolean isSimpleIdentifier(String identifier) { | ||
| if (identifier == null) { | ||
| throw new IllegalArgumentException("Identifier cannot be null"); | ||
| } | ||
| return SIMPLE_IDENTIFIER_PATTERN.matcher(identifier).matches(); | ||
| } | ||
|
|
||
| private final static Pattern UNQUOTE_INDENTIFIER = Pattern.compile( | ||
| "^[\\\"`]?(.+?)[\\\"`]?$" | ||
| ); | ||
|
|
||
| public static String unquoteIdentifier(String str) { | ||
| Matcher matcher = UNQUOTE_INDENTIFIER.matcher(str.trim()); | ||
| if (matcher.find()) { | ||
| return matcher.group(1); | ||
| } else { | ||
| return str; | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check here if the identifier is null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I've added IllegalArgumentException