diff --git a/csharp/src/StatementExecution/StatementExecutionConnection.cs b/csharp/src/StatementExecution/StatementExecutionConnection.cs index 2a112f41a..0117b3101 100644 --- a/csharp/src/StatementExecution/StatementExecutionConnection.cs +++ b/csharp/src/StatementExecution/StatementExecutionConnection.cs @@ -18,6 +18,8 @@ using System.Collections.Generic; using System.Linq; using System.Net.Http; +using System.Text; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using AdbcDrivers.Databricks.Http; @@ -571,7 +573,28 @@ public override Schema GetTableSchema(string? catalog, string? dbSchema, string async Task> IGetObjectsDataProvider.GetCatalogsAsync(string? catalogPattern, CancellationToken cancellationToken) { - string sql = new ShowCatalogsCommand(catalogPattern).Build(); + // Catalog-pattern handling (mirrors the Thrift path in HiveServer2Connection): + // + // SHOW CATALOGS LIKE '' is not suitable for ADBC patterns because: + // - "comparator\_tests" (ADBC escaped literal underscore) would emit + // SHOW CATALOGS LIKE 'comparator_tests' where SQL _ is a wildcard → too broad. + // - "%" would emit SHOW CATALOGS LIKE '*' which may not be valid SQL. + // + // Instead we always issue a bare SHOW CATALOGS (returns all catalogs) and then + // filter client-side using the standard ADBC/SQL pattern-to-regex conversion, + // exactly as the Thrift path does via PatternToRegEx(). + // + // Special cases: + // - null → no filter, return all catalogs (JDBC spec: null means "any catalog"). + // - "" → no catalog can have an empty name; return empty list immediately. + + if (catalogPattern != null && catalogPattern.Length == 0) + return System.Array.Empty(); + + // Build optional client-side filter. + Regex? catalogFilter = catalogPattern != null ? CatalogPatternToRegex(catalogPattern) : null; + + string sql = new ShowCatalogsCommand().Build(); // SHOW CATALOGS (no LIKE) var batches = await ExecuteMetadataSqlAsync(sql, cancellationToken).ConfigureAwait(false); var result = new List(); foreach (var batch in batches) @@ -580,8 +603,10 @@ async Task> IGetObjectsDataProvider.GetCatalogsAsync(strin if (catalogArray == null) continue; for (int i = 0; i < batch.Length; i++) { - if (!catalogArray.IsNull(i)) - result.Add(catalogArray.GetString(i)); + if (catalogArray.IsNull(i)) continue; + string catalog = catalogArray.GetString(i); + if (catalogFilter == null || catalogFilter.IsMatch(catalog)) + result.Add(catalog); } } return result; @@ -754,6 +779,70 @@ internal List ExecuteMetadataSql(string sql, CancellationToken canc return ExecuteMetadataSqlAsync(sql, cancellationToken).GetAwaiter().GetResult(); } + // ---------------------------------------------------------------- + // ADBC catalog-pattern helpers (used by GetCatalogsAsync) + // ---------------------------------------------------------------- + + /// + /// Returns if the ADBC/SQL pattern contains an unescaped + /// % or _ wildcard character. + /// A \ immediately before %, _, or \ is an escape sequence + /// and does not count as a wildcard. + /// + internal static bool ContainsUnescapedWildcard(string pattern) + { + for (int i = 0; i < pattern.Length; i++) + { + char c = pattern[i]; + if (c == '\\') + { + i++; // skip the escaped character + continue; + } + if (c == '%' || c == '_') + return true; + } + return false; + } + + /// + /// Converts an ADBC/SQL wildcard pattern to a case-insensitive + /// anchored to the full string. + /// + /// % → .* (any sequence of characters) + /// _ → . (any single character) + /// \_ → _ (literal underscore) + /// \% → % (literal percent) + /// \\ → \ (literal backslash) + /// + /// + internal static Regex CatalogPatternToRegex(string pattern) + { + var sb = new StringBuilder("^"); + for (int i = 0; i < pattern.Length; i++) + { + char c = pattern[i]; + if (c == '\\' && i + 1 < pattern.Length) + { + char next = pattern[i + 1]; + if (next == '_' || next == '%' || next == '\\') + { + sb.Append(Regex.Escape(next.ToString())); + i++; + continue; + } + } + if (c == '%') + sb.Append(".*"); + else if (c == '_') + sb.Append('.'); + else + sb.Append(Regex.Escape(c.ToString())); + } + sb.Append('$'); + return new Regex(sb.ToString(), RegexOptions.IgnoreCase); + } + /// /// Executes a SHOW COLUMNS command. When catalog is null, iterates over all catalogs /// since SHOW COLUMNS IN ALL CATALOGS is not yet supported by the backend. diff --git a/csharp/test/E2E/CloseOperationE2ETest.cs b/csharp/test/E2E/CloseOperationE2ETest.cs index 03d2cf91a..a4078f973 100644 --- a/csharp/test/E2E/CloseOperationE2ETest.cs +++ b/csharp/test/E2E/CloseOperationE2ETest.cs @@ -134,10 +134,11 @@ public async Task DisposeEmitsCloseOperationEvent(string description, string que var parameters = new Dictionary { - [DatabricksParameters.Protocol] = "thrift", [DatabricksParameters.UseCloudFetch] = useCloudFetch.ToString(), [DatabricksParameters.EnableDirectResults] = enableDirectResults.ToString(), }; + if (!string.IsNullOrEmpty(TestConfiguration.Protocol)) + parameters[DatabricksParameters.Protocol] = TestConfiguration.Protocol; // Keep connection alive without disposing — simulates a connection pool. // In a pool CloseSession is never sent, so CloseOperation is the only mechanism diff --git a/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs b/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs index 2f143b32d..d7eb26be8 100644 --- a/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs +++ b/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs @@ -47,6 +47,12 @@ private void SkipIfNotConfigured() Skip.IfNot(Utils.CanExecuteTestConfig(TestConfigVariable), "Test configuration not available"); } + private void SkipIfThriftNotSupported() + { + SkipIfNotConfigured(); + Skip.If(TestConfiguration.Protocol == "rest", "Thrift protocol not available on SEA-only endpoint"); + } + private AdbcConnection CreateThriftConnection() { var parameters = GetDriverParameters(TestConfiguration); @@ -126,7 +132,7 @@ private static string GetStringValue(IArrowArray array, int index) [SkippableFact] public async Task Thrift_GetCatalogs_ContainsMain() { - SkipIfNotConfigured(); + SkipIfThriftNotSupported(); using var conn = CreateThriftConnection(); var rows = await ReadMetadata(conn, "GetCatalogs"); Assert.True(rows.Count > 0, "GetCatalogs should return at least one catalog"); @@ -146,7 +152,7 @@ public async Task SEA_GetCatalogs_ContainsMain() [SkippableFact] public async Task GetCatalogs_ThriftAndSEA_SameRowCount() { - SkipIfNotConfigured(); + SkipIfThriftNotSupported(); using var thrift = CreateThriftConnection(); using var sea = CreateSeaConnection(); var thriftRows = await ReadMetadata(thrift, "GetCatalogs"); @@ -159,7 +165,7 @@ public async Task GetCatalogs_ThriftAndSEA_SameRowCount() [SkippableFact] public async Task Thrift_GetTables_ReturnsAllColumnTypes() { - SkipIfNotConfigured(); + SkipIfThriftNotSupported(); using var conn = CreateThriftConnection(); var rows = await ReadMetadata(conn, "GetTables", TestCatalog, TestSchema); Assert.Contains(rows, r => r["TABLE_NAME"] == TestTable); @@ -190,7 +196,7 @@ public async Task SEA_GetTables_ReturnsAllColumnTypes() [SkippableFact] public async Task GetTables_ThriftAndSEA_SameCount() { - SkipIfNotConfigured(); + SkipIfThriftNotSupported(); using var thrift = CreateThriftConnection(); using var sea = CreateSeaConnection(); var thriftRows = await ReadMetadata(thrift, "GetTables", TestCatalog, TestSchema); @@ -203,7 +209,7 @@ public async Task GetTables_ThriftAndSEA_SameCount() [SkippableFact] public async Task Thrift_GetColumnsExtended_Returns20Columns() { - SkipIfNotConfigured(); + SkipIfThriftNotSupported(); using var conn = CreateThriftConnection(); var rows = await ReadMetadata(conn, "GetColumnsExtended", TestCatalog, TestSchema, TestTable); Assert.Equal(20, rows.Count); @@ -221,7 +227,7 @@ public async Task SEA_GetColumnsExtended_Returns20Columns() [SkippableFact] public async Task GetColumnsExtended_ThriftAndSEA_SameColumnNames() { - SkipIfNotConfigured(); + SkipIfThriftNotSupported(); using var thrift = CreateThriftConnection(); using var sea = CreateSeaConnection(); var thriftRows = await ReadMetadata(thrift, "GetColumnsExtended", TestCatalog, TestSchema, TestTable); @@ -240,7 +246,7 @@ public async Task GetColumnsExtended_ThriftAndSEA_SameColumnNames() [SkippableFact] public async Task GetColumnsExtended_ThriftAndSEA_32ColumnSchema() { - SkipIfNotConfigured(); + SkipIfThriftNotSupported(); using var thrift = CreateThriftConnection(); using var sea = CreateSeaConnection(); var thriftRows = await ReadMetadata(thrift, "GetColumnsExtended", TestCatalog, TestSchema, TestTable); @@ -309,7 +315,7 @@ public async Task SEA_GetColumnsExtended_FallbackAndDescTable_SameResults() [SkippableFact] public async Task Thrift_GetPrimaryKeys_ReturnsPKColumns() { - SkipIfNotConfigured(); + SkipIfThriftNotSupported(); using var conn = CreateThriftConnection(); var rows = await ReadMetadata(conn, "GetPrimaryKeys", TestCatalog, TestSchema, TestTable); Assert.Equal(2, rows.Count); @@ -333,7 +339,7 @@ public async Task SEA_GetPrimaryKeys_ReturnsPKColumns() [SkippableFact] public void Thrift_GetTableSchema_Returns20Fields() { - SkipIfNotConfigured(); + SkipIfThriftNotSupported(); using var conn = CreateThriftConnection(); // Use cross_ref_customers to avoid Thrift NotImplementedException on complex types var schema = conn.GetTableSchema(TestCatalog, TestSchema, "cross_ref_customers"); @@ -354,7 +360,7 @@ public void SEA_GetTableSchema_ReturnsFields() [SkippableFact] public void GetTableSchema_ThriftAndSEA_SameFieldNames() { - SkipIfNotConfigured(); + SkipIfThriftNotSupported(); using var thrift = CreateThriftConnection(); using var sea = CreateSeaConnection(); var thriftSchema = thrift.GetTableSchema(TestCatalog, TestSchema, "cross_ref_customers"); diff --git a/csharp/test/Unit/StatementExecution/GetCatalogsAsyncTests.cs b/csharp/test/Unit/StatementExecution/GetCatalogsAsyncTests.cs new file mode 100644 index 000000000..75d056763 --- /dev/null +++ b/csharp/test/Unit/StatementExecution/GetCatalogsAsyncTests.cs @@ -0,0 +1,213 @@ +/* +* Copyright (c) 2025 ADBC Drivers Contributors +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +using System.Text.RegularExpressions; +using AdbcDrivers.Databricks.StatementExecution; +using Xunit; + +namespace AdbcDrivers.Databricks.Tests.Unit.StatementExecution +{ + /// + /// Unit tests for the helper methods introduced to fix PECO-3018 (D8): + /// and + /// . + /// + /// These helpers enable GetCatalogsAsync to correctly handle wildcard catalog + /// patterns (%, _), escaped literals (\_), and empty strings without sending + /// broken LIKE clauses to the server. + /// + /// Root cause: the previous implementation passed the ADBC catalogPattern directly + /// to ShowCatalogsCommand which converted it via ConvertPattern() and appended it as + /// a SQL LIKE clause. This was wrong because: + /// - "comparator\_tests" → SHOW CATALOGS LIKE 'comparator_tests' + /// (SQL _ is a single-char wildcard, so the literal underscore was not preserved) + /// - "%" → SHOW CATALOGS LIKE '*' + /// (the glob '*' may not be valid in all SQL LIKE contexts) + /// + /// Fix: always issue SHOW CATALOGS (no LIKE) and filter client-side using + /// CatalogPatternToRegex, mirroring the Thrift path's PatternToRegEx(). + /// + public class GetCatalogsAsyncTests + { + // ---------------------------------------------------------------- + // ContainsUnescapedWildcard + // ---------------------------------------------------------------- + + [Theory] + [InlineData("%", true)] // bare % is a wildcard + [InlineData("compar%", true)] // trailing % + [InlineData("_", true)] // bare _ is a wildcard + [InlineData("comparator_tests", true)] // unescaped _ is a wildcard + [InlineData(@"comparator\_tests", false)] // \_ is escaped → not a wildcard + [InlineData(@"\%", false)] // \% is escaped → not a wildcard + [InlineData("", false)] // empty — no wildcard + [InlineData("nonexistent", false)] // plain literal — no wildcard + [InlineData("main", false)] // plain literal — no wildcard + [InlineData(@"\\", false)] // escaped backslash — no wildcard + public void ContainsUnescapedWildcard_ReturnsExpected(string pattern, bool expected) + { + Assert.Equal(expected, StatementExecutionConnection.ContainsUnescapedWildcard(pattern)); + } + + // ---------------------------------------------------------------- + // CatalogPatternToRegex — wildcard expansion + // ---------------------------------------------------------------- + + [Fact] + public void CatalogPatternToRegex_PercentMatchesAllCatalogs() + { + Regex regex = StatementExecutionConnection.CatalogPatternToRegex("%"); + Assert.Matches(regex, "main"); + Assert.Matches(regex, "comparator_tests"); + Assert.Matches(regex, "hive_metastore"); + Assert.Matches(regex, ""); + } + + [Fact] + public void CatalogPatternToRegex_PercentPrefix_MatchesCatalogsStartingWithPrefix() + { + Regex regex = StatementExecutionConnection.CatalogPatternToRegex("compar%"); + Assert.Matches(regex, "comparator_tests"); + Assert.Matches(regex, "comparator-tests"); + Assert.Matches(regex, "compar"); + Assert.DoesNotMatch(regex, "main"); + Assert.DoesNotMatch(regex, "xcompar"); + } + + [Fact] + public void CatalogPatternToRegex_UnderscoreMatchesSingleChar() + { + // "comparator_tests" pattern: _ matches any single char + Regex regex = StatementExecutionConnection.CatalogPatternToRegex("comparator_tests"); + Assert.Matches(regex, "comparator_tests"); + Assert.Matches(regex, "comparator-tests"); + Assert.Matches(regex, "comparatorXtests"); + Assert.DoesNotMatch(regex, "comparatortests"); // _ requires exactly one char + Assert.DoesNotMatch(regex, "comparator__tests"); // one char too many + } + + [Fact] + public void CatalogPatternToRegex_EscapedUnderscore_MatchesLiteralUnderscore() + { + // D8 bug case: "comparator\_tests" — \_ is a literal underscore, not a wildcard. + Regex regex = StatementExecutionConnection.CatalogPatternToRegex(@"comparator\_tests"); + Assert.Matches(regex, "comparator_tests"); + Assert.DoesNotMatch(regex, "comparator-tests"); + Assert.DoesNotMatch(regex, "comparatorXtests"); + Assert.DoesNotMatch(regex, "comparator_Xtests"); + } + + [Fact] + public void CatalogPatternToRegex_EscapedPercent_MatchesLiteralPercent() + { + Regex regex = StatementExecutionConnection.CatalogPatternToRegex(@"cat\%log"); + Assert.Matches(regex, "cat%log"); + Assert.DoesNotMatch(regex, "catalog"); + Assert.DoesNotMatch(regex, "catXlog"); + } + + [Fact] + public void CatalogPatternToRegex_IsCaseInsensitive() + { + Regex regex = StatementExecutionConnection.CatalogPatternToRegex("MAIN"); + Assert.Matches(regex, "main"); + Assert.Matches(regex, "Main"); + Assert.Matches(regex, "MAIN"); + } + + [Fact] + public void CatalogPatternToRegex_RegexSpecialCharsInPattern_AreEscaped() + { + // Dots in a catalog name must be treated as literals, not regex wildcards. + Regex regex = StatementExecutionConnection.CatalogPatternToRegex("my.catalog"); + Assert.Matches(regex, "my.catalog"); + Assert.DoesNotMatch(regex, "myXcatalog"); // dot in pattern is literal + } + + [Fact] + public void CatalogPatternToRegex_EscapedBackslash() + { + // \\ in pattern → literal backslash in output + Regex regex = StatementExecutionConnection.CatalogPatternToRegex(@"back\\slash"); + Assert.Matches(regex, @"back\slash"); + Assert.DoesNotMatch(regex, "backslash"); + } + + // ---------------------------------------------------------------- + // D8 bug-report scenarios (from PECO-3018 bug report) + // ---------------------------------------------------------------- + + [Fact] + public void BugReport_D8_EscapedUnderscore_NotAWildcard() + { + // D8: "comparator\_tests" should match the literal catalog "comparator_tests" + // only, not "comparatorXtests" or other single-char variations. + Assert.False(StatementExecutionConnection.ContainsUnescapedWildcard(@"comparator\_tests")); + Regex regex = StatementExecutionConnection.CatalogPatternToRegex(@"comparator\_tests"); + Assert.Matches(regex, "comparator_tests"); + Assert.DoesNotMatch(regex, "comparator-tests"); + Assert.DoesNotMatch(regex, "comparatorXtests"); + } + + [Fact] + public void BugReport_UnescapedUnderscore_IsSingleCharWildcard() + { + // "comparator_tests" (no escape): _ is a wildcard matching any single char. + Assert.True(StatementExecutionConnection.ContainsUnescapedWildcard("comparator_tests")); + Regex regex = StatementExecutionConnection.CatalogPatternToRegex("comparator_tests"); + Assert.Matches(regex, "comparator_tests"); + Assert.Matches(regex, "comparator-tests"); + } + + [Fact] + public void BugReport_PercentPattern_DetectedAsWildcard() + { + // "%" should be a wildcard matching everything. + Assert.True(StatementExecutionConnection.ContainsUnescapedWildcard("%")); + Regex regex = StatementExecutionConnection.CatalogPatternToRegex("%"); + Assert.Matches(regex, "any_catalog"); + } + + [Fact] + public void BugReport_ComparPercentPrefix_DetectedAsWildcard() + { + // "compar%" should be a wildcard prefix match. + Assert.True(StatementExecutionConnection.ContainsUnescapedWildcard("compar%")); + Regex regex = StatementExecutionConnection.CatalogPatternToRegex("compar%"); + Assert.Matches(regex, "comparator_tests"); + Assert.DoesNotMatch(regex, "main"); + } + + [Fact] + public void BugReport_EmptyString_NotAWildcard() + { + // Empty string: GetCatalogsAsync returns empty list immediately (no server call). + Assert.False(StatementExecutionConnection.ContainsUnescapedWildcard("")); + } + + [Fact] + public void BugReport_NonexistentCatalog_NotAWildcard_NoMatch() + { + // "nonexistent": no wildcard. CatalogPatternToRegex produces a regex that + // matches only "nonexistent", so real catalogs won't appear in the result. + Assert.False(StatementExecutionConnection.ContainsUnescapedWildcard("nonexistent")); + Regex regex = StatementExecutionConnection.CatalogPatternToRegex("nonexistent"); + Assert.DoesNotMatch(regex, "main"); + Assert.DoesNotMatch(regex, "comparator_tests"); + Assert.Matches(regex, "nonexistent"); + } + } +}