Skip to content

[SEA] Fix GetCatalogsAsync to handle ADBC catalog patterns correctly (PECO-3018) #441

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.

Sign up for GitHub

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

Open
eric-wang-1990 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[SEA] Fix GetCatalogsAsync to handle ADBC catalog patterns correctly (PECO-3018) #441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 92 additions & 3 deletions csharp/src/StatementExecution/StatementExecutionConnection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -571,7 +573,28 @@ public override Schema GetTableSchema(string? catalog, string? dbSchema, string

async Task<IReadOnlyList<string>> IGetObjectsDataProvider.GetCatalogsAsync(string? catalogPattern, CancellationToken cancellationToken)
{
string sql = new ShowCatalogsCommand(catalogPattern).Build();
// Catalog-pattern handling (mirrors the Thrift path in HiveServer2Connection):
//
// SHOW CATALOGS LIKE '<pattern>' 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<string>();

// 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<string>();
foreach (var batch in batches)
Expand All @@ -580,8 +603,10 @@ async Task<IReadOnlyList<string>> 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;
Expand Down Expand Up @@ -754,6 +779,70 @@ internal List<RecordBatch> ExecuteMetadataSql(string sql, CancellationToken canc
return ExecuteMetadataSqlAsync(sql, cancellationToken).GetAwaiter().GetResult();
}

// ----------------------------------------------------------------
// ADBC catalog-pattern helpers (used by GetCatalogsAsync)
// ----------------------------------------------------------------

/// <summary>
/// Returns <see langword="true"/> if the ADBC/SQL pattern contains an unescaped
/// <c>%</c> or <c>_</c> wildcard character.
/// A <c>\</c> immediately before <c>%</c>, <c>_</c>, or <c>\</c> is an escape sequence
/// and does not count as a wildcard.
/// </summary>
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;
}

/// <summary>
/// Converts an ADBC/SQL wildcard pattern to a case-insensitive <see cref="Regex"/>
/// anchored to the full string.
/// <list type="bullet">
/// <item><c>%</c> → <c>.*</c> (any sequence of characters)</item>
/// <item><c>_</c> → <c>.</c> (any single character)</item>
/// <item><c>\_</c> → <c>_</c> (literal underscore)</item>
/// <item><c>\%</c> → <c>%</c> (literal percent)</item>
/// <item><c>\\</c> → <c>\</c> (literal backslash)</item>
/// </list>
/// </summary>
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);
}

/// <summary>
/// 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.
Expand Down
3 changes: 2 additions & 1 deletion csharp/test/E2E/CloseOperationE2ETest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,11 @@ public async Task DisposeEmitsCloseOperationEvent(string description, string que

var parameters = new Dictionary<string, string>
{
[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
Expand Down
26 changes: 16 additions & 10 deletions csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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");
Expand All @@ -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");
Expand Down
Loading
Loading