clickhouse-java icon indicating copy to clipboard operation
clickhouse-java copied to clipboard

[jdbc-v2] Support multi-dot notation for database names

Open devdavidkarlsson opened this issue 2 months ago • 7 comments

Description

Fixes #2650

This PR adds support for multi-dot notation in database names without requiring quotes, enabling table references like a.b.c where a.b is the database name and c is the table name.

Changes

  • Grammar: Updated ANTLR databaseIdentifier rule to support multiple dot-separated identifiers: identifier (DOT identifier)*
  • Parser Logic: Added extractTableName() helper method in SqlParserFacade to properly unquote each identifier part separately and join them correctly
  • Tests: Added testMultiDotNotation() test method with 3 test cases:
    • Unquoted multi-dot: SELECT * FROM a.b.c WHERE id = ?
    • Quoted identifiers: SELECT * FROM `db.part1`.`table` WHERE id = ?
    • INSERT statement: INSERT INTO a.b.c (col1, col2) VALUES (?, ?)

Testing

  • ✅ All 367 existing tests continue to pass
  • ✅ New test validates parsing of multi-dot database names
  • ✅ Verified against ClickHouse server with databases containing dots in their names

Example Usage

-- Database with dot in name (must be quoted when creating)
CREATE DATABASE `test.db`;
CREATE TABLE `test.db`.users (id UInt32, name String) ENGINE = MergeTree ORDER BY id;

-- Can now be referenced in JDBC with proper parsing
SELECT * FROM `test.db`.users WHERE id = ?;
INSERT INTO `test.db`.users VALUES (?, ?);

The JDBC driver now correctly parses these statements and extracts the full table name test.db.users for metadata operations.

devdavidkarlsson avatar Nov 20 '25 14:11 devdavidkarlsson

/windsurf-review

devdavidkarlsson avatar Nov 20 '25 14:11 devdavidkarlsson

Good day, @devdavidkarlsson ! Thank you for the contribution!

I've left comments. Build fails with:

Error:  Tests run: 1182, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 3.501 s <<< FAILURE! -- in TestSuite
Error:  com.clickhouse.jdbc.internal.JavaCCParserTest.testMultiDotNotation -- Time elapsed: 0.010 s <<< FAILURE!
java.lang.AssertionError: expected [false] but found [true]
	at org.testng.Assert.fail(Assert.java:110)
	at org.testng.Assert.failNotEquals(Assert.java:1413)
	at org.testng.Assert.assertFalse(Assert.java:78)
	at org.testng.Assert.assertFalse(Assert.java:88)
	at com.clickhouse.jdbc.internal.BaseSqlParserFacadeTest.testMultiDotNotation(BaseSqlParserFacadeTest.java:169)
```	

chernser avatar Nov 21 '25 22:11 chernser

@chernser new attempt: I added the JavaCC (.jj) and got rid of extractTableName, tests should be green now. Thank you 🙏

devdavidkarlsson avatar Nov 24 '25 15:11 devdavidkarlsson

/windsurf-review

devdavidkarlsson avatar Nov 24 '25 15:11 devdavidkarlsson

/windsurf-review

devdavidkarlsson avatar Nov 25 '25 08:11 devdavidkarlsson

/windsurf-review

devdavidkarlsson avatar Nov 25 '25 09:11 devdavidkarlsson

Good day, @devdavidkarlsson ! Will review today.

chernser avatar Nov 25 '25 16:11 chernser

Good day, @devdavidkarlsson !

I appreciate your effort!

Code now looks much more simple - what is great! I've run tests and if they pass - will merge.

chernser avatar Dec 16 '25 18:12 chernser

@devdavidkarlsson I see there is a failure because we need to fix one test. I hopefully do it today and will merge the PR.

chernser avatar Dec 17 '25 20:12 chernser