From 97a1c34afe3075bf4c50f048fbd1f59fbfbb8527 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" <3815206+peco-engineer-bot[bot]@users.noreply.github.com> Date: Wed, 8 Jul 2026 10:43:35 +0000 Subject: [PATCH 1/6] fix(csharp): address issue #524 --- .claude/knowledge/learning.md | 7 +- csharp/src/DatabricksStatement.cs | 65 +++++++ csharp/src/MetadataUtilities.cs | 21 +++ .../StatementExecution/SeaMetadataE2ETests.cs | 161 ++++++++++++++++++ 4 files changed, 253 insertions(+), 1 deletion(-) diff --git a/.claude/knowledge/learning.md b/.claude/knowledge/learning.md index 301f77d94..1eae488a2 100644 --- a/.claude/knowledge/learning.md +++ b/.claude/knowledge/learning.md @@ -38,4 +38,9 @@ reader can grep for it. ## Entries -(no entries yet) +### 2026-07-08: metadata (Thrift vs SEA) — empty-string identifier arg must map to empty result, not an exception + +**Issue:** #524 — passing an empty-string identifier (catalog="", schema="", table="", foreign_schema="", foreign_table="") to a metadata op threw `HiveServer2Exception` on the Thrift path but returned an empty `QueryResult` on SEA (1,763 outcome diffs across list_columns/list_tables/get_primary_keys/get_cross_reference). +**Root Cause:** `HiveServer2Statement.SetOption` stores the empty string verbatim into `CatalogName`/`SchemaName`/`TableName`/`Foreign*` and the Thrift metadata RPCs pass it straight through; the server rejects `""` (e.g. `ListTableSummaries` "name \"\" is not a valid name", GetPrimaryKeys "Can not create a Path from an empty string"). SEA already short-circuited via its `string.IsNullOrEmpty(...)` checks. The existing Thrift guards only covered `catalog != null` (multi-catalog) and PK/FK invalid-catalog — none caught a non-null empty string on schema/table/foreign args. `HandleSparkCatalog("")` returns `""` (only "SPARK"→null), so empty catalog also slipped through. +**Fix:** Added `MetadataUtilities.HasEmptyStringIdentifier(params string?[])` (true only for a non-null zero-length string; null = "no filter" untouched) and guarded the Databricks Thrift overrides in `DatabricksStatement.cs` (GetTables/GetColumns/GetPrimaryKeys/GetCrossReference/GetCrossReferenceAsForeignTable) to return the empty result *before* calling `base.*`, mirroring SEA. Note: on Thrift, GetColumns with catalog=""/schema="" already returned empty for some param combos (SHOW COLUMNS tolerates it) — only GetTables/PK/FK actually threw — but guarding all ops uniformly is the parity-correct behavior. +**Rule:** Normalize empty-string metadata identifier args to an empty result set up front in the editable Databricks overrides — never let `""` reach a base HiveServer2 RPC (it throws); null means "no filter" and must pass through. diff --git a/csharp/src/DatabricksStatement.cs b/csharp/src/DatabricksStatement.cs index bd7135fe3..35d822e02 100644 --- a/csharp/src/DatabricksStatement.cs +++ b/csharp/src/DatabricksStatement.cs @@ -806,6 +806,18 @@ protected override async Task GetTablesAsync(CancellationToken canc activity?.SetTag("statement.table_types", TableTypes ?? "(none)"); activity?.SetTag("statement.feature.enable_multiple_catalog_support", enableMultipleCatalogSupport); + // Issue #524: empty-string identifier -> empty result (parity with SEA); + // the Thrift RPC would otherwise raise HiveServer2Exception. + if (MetadataUtilities.HasEmptyStringIdentifier(CatalogName, SchemaName, TableName)) + { + activity?.AddEvent("statement.get_tables.returning_empty_result", [ + new("reason", "Empty-string identifier argument") + ]); + activity?.SetTag(SemanticConventions.Db.Response.ReturnedRows, 0); + activity?.AddEvent("statement.get_tables.complete"); + return MetadataSchemaFactory.CreateEmptyTablesResult(); + } + // Handle SPARK catalog case HandleSparkCatalog(); activity?.SetTag("statement.catalog_name_after_spark_handling", CatalogName ?? "(none)"); @@ -852,6 +864,19 @@ protected override async Task GetColumnsAsync(CancellationToken can activity?.SetTag("statement.column_name", ColumnName ?? "(none)"); activity?.SetTag("statement.feature.enable_multiple_catalog_support", enableMultipleCatalogSupport); + // Issue #524: empty-string identifier -> empty result (parity with SEA); + // the Thrift RPC would otherwise raise HiveServer2Exception. + if (MetadataUtilities.HasEmptyStringIdentifier(CatalogName, SchemaName, TableName, ColumnName)) + { + activity?.AddEvent("statement.get_columns.returning_empty_result", [ + new("reason", "Empty-string identifier argument") + ]); + activity?.SetTag(SemanticConventions.Db.Response.ReturnedRows, 0); + activity?.AddEvent("statement.get_columns.complete"); + return new QueryResult(0, new HiveInfoArrowStream( + MetadataSchemaFactory.CreateColumnMetadataSchema(), CreateColumnMetadataEmptyArray())); + } + // Handle SPARK catalog case HandleSparkCatalog(); activity?.SetTag("statement.catalog_name_after_spark_handling", CatalogName ?? "(none)"); @@ -916,6 +941,18 @@ protected override async Task GetPrimaryKeysAsync(CancellationToken activity?.SetTag("statement.table_name", TableName ?? "(none)"); activity?.SetTag("statement.feature.pk_fk_enabled", enablePKFK); + // Issue #524: empty-string identifier -> empty result (parity with SEA); + // the Thrift RPC would otherwise raise HiveServer2Exception. + if (MetadataUtilities.HasEmptyStringIdentifier(CatalogName, SchemaName, TableName)) + { + activity?.AddEvent("statement.get_primary_keys.returning_empty_result", [ + new("reason", "Empty-string identifier argument") + ]); + activity?.SetTag(SemanticConventions.Db.Response.ReturnedRows, 0); + activity?.AddEvent("statement.get_primary_keys.complete"); + return EmptyPrimaryKeysResult(); + } + if (ShouldReturnEmptyPkFkResult()) { activity?.AddEvent("statement.get_primary_keys.returning_empty_result", [ @@ -953,6 +990,20 @@ protected override async Task GetCrossReferenceAsync(CancellationTo activity?.SetTag("statement.foreign_table_name", ForeignTableName ?? "(none)"); activity?.SetTag("statement.feature.pk_fk_enabled", enablePKFK); + // Issue #524: empty-string identifier -> empty result (parity with SEA); + // the Thrift RPC would otherwise raise HiveServer2Exception. + if (MetadataUtilities.HasEmptyStringIdentifier( + CatalogName, SchemaName, TableName, + ForeignCatalogName, ForeignSchemaName, ForeignTableName)) + { + activity?.AddEvent("statement.get_cross_reference.returning_empty_result", [ + new("reason", "Empty-string identifier argument") + ]); + activity?.SetTag(SemanticConventions.Db.Response.ReturnedRows, 0); + activity?.AddEvent("statement.get_cross_reference.complete"); + return EmptyCrossReferenceResult(); + } + if (ShouldReturnEmptyPkFkResult()) { activity?.AddEvent("statement.get_cross_reference.returning_empty_result", [ @@ -980,6 +1031,20 @@ protected override async Task GetCrossReferenceAsForeignTableAsync( activity?.SetTag("statement.foreign_catalog_name", ForeignCatalogName ?? "(none)"); activity?.SetTag("statement.feature.pk_fk_enabled", enablePKFK); + // Issue #524: empty-string identifier -> empty result (parity with SEA); + // the Thrift RPC would otherwise raise HiveServer2Exception. + if (MetadataUtilities.HasEmptyStringIdentifier( + CatalogName, SchemaName, TableName, + ForeignCatalogName, ForeignSchemaName, ForeignTableName)) + { + activity?.AddEvent("statement.get_cross_reference_as_foreign_table.returning_empty_result", [ + new("reason", "Empty-string identifier argument") + ]); + activity?.SetTag(SemanticConventions.Db.Response.ReturnedRows, 0); + activity?.AddEvent("statement.get_cross_reference_as_foreign_table.complete"); + return EmptyCrossReferenceResult(); + } + if (ShouldReturnEmptyPkFkResult()) { activity?.AddEvent("statement.get_cross_reference_as_foreign_table.returning_empty_result", [ diff --git a/csharp/src/MetadataUtilities.cs b/csharp/src/MetadataUtilities.cs index 9391abf3d..703234253 100644 --- a/csharp/src/MetadataUtilities.cs +++ b/csharp/src/MetadataUtilities.cs @@ -31,6 +31,27 @@ internal static class MetadataUtilities return catalogName; } + /// + /// Issue #524: an empty-string identifier argument (catalog="", schema="", + /// table="", foreign_*="") is a degenerate filter that can never match a real + /// object. The Thrift metadata RPCs reject it with a HiveServer2Exception, + /// while the SEA path returns an empty result set. To keep the two protocols + /// consistent (empty-string → empty result set, never an exception), callers + /// short-circuit to an empty result when any relevant identifier is empty. + /// + /// Only a non-null, zero-length string counts; null means "no filter" + /// (e.g. "all catalogs") and is left untouched. + /// + internal static bool HasEmptyStringIdentifier(params string?[] identifiers) + { + foreach (string? identifier in identifiers) + { + if (identifier != null && identifier.Length == 0) + return true; + } + return false; + } + internal static bool IsInvalidPKFKCatalog(string? catalogName) { return string.IsNullOrEmpty(catalogName) || diff --git a/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs b/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs index 3173654e5..c766890c0 100644 --- a/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs +++ b/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs @@ -330,4 +330,165 @@ public void GetInfo_ReturnsDriverInfo() Assert.True(batch!.Length >= 5, "GetInfo should return at least 5 info codes"); } } + + /// + /// Issue #524: an empty-string identifier argument (catalog="", schema="", + /// table="", foreign_schema="", foreign_table="") passed to a metadata op must + /// NOT throw HiveServer2Exception on the Thrift path. It must return an empty + /// result set, matching the SEA/REST path (Thrift vs SEA outcome parity). + /// + /// These tests force the Thrift protocol (adbc.databricks.protocol=thrift) so the + /// regression reproduces regardless of the CI run's default protocol; the Thrift + /// path is where the divergence lives. + /// + public class EmptyStringMetadataArgE2ETest : TestBase + { + public EmptyStringMetadataArgE2ETest(ITestOutputHelper? outputHelper) + : base(outputHelper, new DatabricksTestEnvironment.Factory()) + { + } + + private void SkipIfNotConfigured() + { + Skip.IfNot(Utils.CanExecuteTestConfig(TestConfigVariable), "Test configuration not available"); + } + + // Connection pinned to the Thrift protocol regardless of the configured default. + private AdbcConnection CreateThriftConnection() + { + var parameters = GetDriverParameters(TestConfiguration); + parameters[DatabricksParameters.Protocol] = "thrift"; + var driver = new DatabricksDriver(); + var db = driver.Open(parameters); + return db.Connect(new Dictionary()); + } + + /// + /// Executes a metadata command and returns the total row count. Must not throw. + /// + private static async Task ExecuteMetadataRowCount(AdbcConnection connection, string command, + string? catalog = null, string? schema = null, string? table = null, + string? foreignCatalog = null, string? foreignSchema = null, string? foreignTable = null) + { + using var stmt = connection.CreateStatement(); + stmt.SetOption(ApacheParameters.IsMetadataCommand, "true"); + if (catalog != null) stmt.SetOption(ApacheParameters.CatalogName, catalog); + if (schema != null) stmt.SetOption(ApacheParameters.SchemaName, schema); + if (table != null) stmt.SetOption(ApacheParameters.TableName, table); + if (foreignCatalog != null) stmt.SetOption(ApacheParameters.ForeignCatalogName, foreignCatalog); + if (foreignSchema != null) stmt.SetOption(ApacheParameters.ForeignSchemaName, foreignSchema); + if (foreignTable != null) stmt.SetOption(ApacheParameters.ForeignTableName, foreignTable); + + stmt.SqlQuery = command; + QueryResult result = await stmt.ExecuteQueryAsync(); + + int rows = 0; + using var reader = result.Stream!; + while (true) + { + using var batch = await reader.ReadNextRecordBatchAsync(); + if (batch == null) break; + rows += batch.Length; + } + return rows; + } + + // GetColumns + + [SkippableFact] + public async Task GetColumns_EmptyCatalog_ReturnsEmptyWithoutThrowing() + { + SkipIfNotConfigured(); + using var conn = CreateThriftConnection(); + int rows = await ExecuteMetadataRowCount(conn, "GetColumns", + catalog: "", schema: TestConfiguration.Metadata.Schema, table: TestConfiguration.Metadata.Table); + Assert.Equal(0, rows); + } + + [SkippableFact] + public async Task GetColumns_EmptySchema_ReturnsEmptyWithoutThrowing() + { + SkipIfNotConfigured(); + using var conn = CreateThriftConnection(); + int rows = await ExecuteMetadataRowCount(conn, "GetColumns", + catalog: TestConfiguration.Metadata.Catalog, schema: "", table: TestConfiguration.Metadata.Table); + Assert.Equal(0, rows); + } + + // GetTables + + [SkippableFact] + public async Task GetTables_EmptyCatalog_ReturnsEmptyWithoutThrowing() + { + SkipIfNotConfigured(); + using var conn = CreateThriftConnection(); + int rows = await ExecuteMetadataRowCount(conn, "GetTables", + catalog: "", schema: TestConfiguration.Metadata.Schema); + Assert.Equal(0, rows); + } + + [SkippableFact] + public async Task GetTables_EmptySchema_ReturnsEmptyWithoutThrowing() + { + SkipIfNotConfigured(); + using var conn = CreateThriftConnection(); + int rows = await ExecuteMetadataRowCount(conn, "GetTables", + catalog: TestConfiguration.Metadata.Catalog, schema: ""); + Assert.Equal(0, rows); + } + + // GetPrimaryKeys + + [SkippableFact] + public async Task GetPrimaryKeys_EmptySchema_ReturnsEmptyWithoutThrowing() + { + SkipIfNotConfigured(); + using var conn = CreateThriftConnection(); + int rows = await ExecuteMetadataRowCount(conn, "GetPrimaryKeys", + catalog: TestConfiguration.Metadata.Catalog, schema: "", table: TestConfiguration.Metadata.Table); + Assert.Equal(0, rows); + } + + [SkippableFact] + public async Task GetPrimaryKeys_EmptyTable_ReturnsEmptyWithoutThrowing() + { + SkipIfNotConfigured(); + using var conn = CreateThriftConnection(); + int rows = await ExecuteMetadataRowCount(conn, "GetPrimaryKeys", + catalog: TestConfiguration.Metadata.Catalog, schema: TestConfiguration.Metadata.Schema, table: ""); + Assert.Equal(0, rows); + } + + // GetCrossReference + + [SkippableFact] + public async Task GetCrossReference_EmptyForeignSchema_ReturnsEmptyWithoutThrowing() + { + SkipIfNotConfigured(); + using var conn = CreateThriftConnection(); + int rows = await ExecuteMetadataRowCount(conn, "GetCrossReference", + catalog: TestConfiguration.Metadata.Catalog, + schema: TestConfiguration.Metadata.Schema, + table: TestConfiguration.Metadata.Table, + foreignCatalog: TestConfiguration.Metadata.Catalog, + foreignSchema: "", + foreignTable: TestConfiguration.Metadata.Table); + Assert.Equal(0, rows); + } + + [SkippableFact] + public async Task GetCrossReference_EmptyForeignTable_ReturnsEmptyWithoutThrowing() + { + SkipIfNotConfigured(); + using var conn = CreateThriftConnection(); + int rows = await ExecuteMetadataRowCount(conn, "GetCrossReference", + catalog: TestConfiguration.Metadata.Catalog, + schema: TestConfiguration.Metadata.Schema, + table: TestConfiguration.Metadata.Table, + foreignCatalog: TestConfiguration.Metadata.Catalog, + foreignSchema: TestConfiguration.Metadata.Schema, + foreignTable: ""); + Assert.Equal(0, rows); + } + } } From 302deca0f83a5f9398262c1db3586f743ad8d334 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Wed, 8 Jul 2026 11:11:01 +0000 Subject: [PATCH 2/6] fix(csharp): address issue #577 (1 review thread) Addresses: - #3543375469 at csharp/src/DatabricksStatement.cs:869 --- csharp/src/DatabricksStatement.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csharp/src/DatabricksStatement.cs b/csharp/src/DatabricksStatement.cs index 35d822e02..45e71bc18 100644 --- a/csharp/src/DatabricksStatement.cs +++ b/csharp/src/DatabricksStatement.cs @@ -866,7 +866,7 @@ protected override async Task GetColumnsAsync(CancellationToken can // Issue #524: empty-string identifier -> empty result (parity with SEA); // the Thrift RPC would otherwise raise HiveServer2Exception. - if (MetadataUtilities.HasEmptyStringIdentifier(CatalogName, SchemaName, TableName, ColumnName)) + if (MetadataUtilities.HasEmptyStringIdentifier(CatalogName, SchemaName, TableName)) { activity?.AddEvent("statement.get_columns.returning_empty_result", [ new("reason", "Empty-string identifier argument") From 082a7f48d14426390cfefd9c622bc5163d75b2c8 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Wed, 8 Jul 2026 11:15:19 +0000 Subject: [PATCH 3/6] fix(csharp): address issue #577 (1 review thread) Addresses: - #3543448305 at csharp/src/MetadataUtilities.cs:44 --- csharp/test/Unit/MetadataUtilitiesTests.cs | 44 ++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/csharp/test/Unit/MetadataUtilitiesTests.cs b/csharp/test/Unit/MetadataUtilitiesTests.cs index e8869b203..f762b2557 100644 --- a/csharp/test/Unit/MetadataUtilitiesTests.cs +++ b/csharp/test/Unit/MetadataUtilitiesTests.cs @@ -136,5 +136,49 @@ public void BuildQualifiedTableName_BackticksEscaped() { Assert.Equal("`main`.`my``schema`.`my``table`", MetadataUtilities.BuildQualifiedTableName("main", "my`schema", "my`table")); } + + // HasEmptyStringIdentifier + + [Fact] + public void HasEmptyStringIdentifier_EmptyString_ReturnsTrue() + { + Assert.True(MetadataUtilities.HasEmptyStringIdentifier("")); + } + + [Fact] + public void HasEmptyStringIdentifier_Null_ReturnsFalse() + { + Assert.False(MetadataUtilities.HasEmptyStringIdentifier((string?)null)); + } + + [Fact] + public void HasEmptyStringIdentifier_Whitespace_ReturnsFalse() + { + Assert.False(MetadataUtilities.HasEmptyStringIdentifier(" ")); + } + + [Fact] + public void HasEmptyStringIdentifier_NonEmpty_ReturnsFalse() + { + Assert.False(MetadataUtilities.HasEmptyStringIdentifier("main")); + } + + [Fact] + public void HasEmptyStringIdentifier_NoArgs_ReturnsFalse() + { + Assert.False(MetadataUtilities.HasEmptyStringIdentifier()); + } + + [Fact] + public void HasEmptyStringIdentifier_MixedWithEmpty_ReturnsTrue() + { + Assert.True(MetadataUtilities.HasEmptyStringIdentifier("main", null, "", "users")); + } + + [Fact] + public void HasEmptyStringIdentifier_MixedNoEmpty_ReturnsFalse() + { + Assert.False(MetadataUtilities.HasEmptyStringIdentifier("main", null, "default", "users")); + } } } From 5f26464a7d64c44f8a3c4d1d95e7d11b29abffa5 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Wed, 8 Jul 2026 11:41:52 +0000 Subject: [PATCH 4/6] fix(csharp): address issue #577 (1 review thread) Addresses: - #3543504258 at csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs:492 --- .../StatementExecution/SeaMetadataE2ETests.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs b/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs index c766890c0..2308ded4d 100644 --- a/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs +++ b/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs @@ -490,5 +490,28 @@ public async Task GetCrossReference_EmptyForeignTable_ReturnsEmptyWithoutThrowin foreignTable: ""); Assert.Equal(0, rows); } + + // GetColumnsExtended + // + // GetCrossReferenceAsForeignTableAsync has no metadata command name of its own; + // it is reached only through the base GetColumnsExtendedAsync, which the Databricks + // override delegates to on its fallback path (BuildTableName empty => empty TableName). + // On that path the base helper invokes GetCrossReferenceAsForeignTableAsync with the + // statement's own catalog/schema/table bound to the FOREIGN slot, so an empty-string + // identifier there is what the new guard (DatabricksStatement.cs ~L1031) must absorb. + // Without the guard the foreign-table cross-reference RPC would raise + // HiveServer2Exception. Empty table forces the fallback, exercising that guard. + + [SkippableFact] + public async Task GetColumnsExtended_EmptyTable_ReturnsEmptyWithoutThrowing() + { + SkipIfNotConfigured(); + using var conn = CreateThriftConnection(); + int rows = await ExecuteMetadataRowCount(conn, "GetColumnsExtended", + catalog: TestConfiguration.Metadata.Catalog, + schema: TestConfiguration.Metadata.Schema, + table: ""); + Assert.Equal(0, rows); + } } } From 71efaa293eae5df79ff0df7e22d86f7d24cb25a8 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Wed, 8 Jul 2026 11:54:12 +0000 Subject: [PATCH 5/6] fix(csharp): address issue #577 (1 review thread) Addresses: - #3543665730 at csharp/src/DatabricksStatement.cs:1047 --- csharp/src/DatabricksStatement.cs | 14 +++++++++ .../StatementExecution/SeaMetadataE2ETests.cs | 29 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/csharp/src/DatabricksStatement.cs b/csharp/src/DatabricksStatement.cs index 45e71bc18..f61be4a67 100644 --- a/csharp/src/DatabricksStatement.cs +++ b/csharp/src/DatabricksStatement.cs @@ -1083,6 +1083,20 @@ protected override async Task GetColumnsExtendedAsync(CancellationT activity?.SetTag("statement.desc_table_extended.full_table_name", fullTableName ?? "(none)"); activity?.SetTag("statement.desc_table_extended.can_use", canUseDescTableExtended); + // Issue #524: empty-string identifier -> empty result (parity with SEA and GetColumnsAsync). + // On the primary DESC TABLE EXTENDED path, BuildTableName drops an empty catalog/schema, so + // an empty catalog or schema would otherwise produce a valid identifier and return real rows, + // diverging from GetColumnsAsync. Guard here so both column APIs behave identically. + if (MetadataUtilities.HasEmptyStringIdentifier(CatalogName, SchemaName, TableName)) + { + activity?.AddEvent("statement.get_columns_extended.returning_empty_result", [ + new("reason", "Empty-string identifier argument") + ]); + activity?.SetTag(SemanticConventions.Db.Response.ReturnedRows, 0); + activity?.AddEvent("statement.get_columns_extended.complete"); + return CreateEmptyExtendedColumnsResult(MetadataSchemaFactory.CreateColumnMetadataSchema()); + } + if (!canUseDescTableExtended || string.IsNullOrEmpty(fullTableName)) { activity?.AddEvent("statement.get_columns_extended.fallback_to_base", [ diff --git a/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs b/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs index 2308ded4d..2c2bc4857 100644 --- a/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs +++ b/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs @@ -513,5 +513,34 @@ public async Task GetColumnsExtended_EmptyTable_ReturnsEmptyWithoutThrowing() table: ""); Assert.Equal(0, rows); } + + // On the primary DESC TABLE EXTENDED path, BuildTableName drops an empty catalog/schema, + // so without the guard these would produce a valid identifier and return real rows, + // diverging from GetColumnsAsync. These cover the empty-catalog / empty-schema cases with + // a non-empty table (which do NOT force the empty-table fallback). + + [SkippableFact] + public async Task GetColumnsExtended_EmptyCatalog_ReturnsEmptyWithoutThrowing() + { + SkipIfNotConfigured(); + using var conn = CreateThriftConnection(); + int rows = await ExecuteMetadataRowCount(conn, "GetColumnsExtended", + catalog: "", + schema: TestConfiguration.Metadata.Schema, + table: TestConfiguration.Metadata.Table); + Assert.Equal(0, rows); + } + + [SkippableFact] + public async Task GetColumnsExtended_EmptySchema_ReturnsEmptyWithoutThrowing() + { + SkipIfNotConfigured(); + using var conn = CreateThriftConnection(); + int rows = await ExecuteMetadataRowCount(conn, "GetColumnsExtended", + catalog: TestConfiguration.Metadata.Catalog, + schema: "", + table: TestConfiguration.Metadata.Table); + Assert.Equal(0, rows); + } } } From 9b7036488ccd7d21217c5ad83c07cba21a755937 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Wed, 8 Jul 2026 12:07:49 +0000 Subject: [PATCH 6/6] fix(csharp): address issue #577 (1 review thread) Addresses: - #3543721922 at csharp/src/DatabricksStatement.cs:1091 --- .../StatementExecutionStatement.cs | 12 ++++++ .../StatementExecution/SeaMetadataE2ETests.cs | 39 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/csharp/src/StatementExecution/StatementExecutionStatement.cs b/csharp/src/StatementExecution/StatementExecutionStatement.cs index 72e2ecbca..eaf6f9583 100644 --- a/csharp/src/StatementExecution/StatementExecutionStatement.cs +++ b/csharp/src/StatementExecution/StatementExecutionStatement.cs @@ -1374,6 +1374,18 @@ private async Task GetColumnsExtendedAsync(CancellationToken cancel activity?.SetTag("table", _metadataTableName ?? "(none)"); activity?.SetTag("use_desc_table_extended", _connection.UseDescTableExtended); + // Issue #524: empty-string identifier -> empty result (parity with the Thrift + // DatabricksStatement.GetColumnsExtendedAsync guard). Without this, an empty + // catalog/schema with a non-empty table would be dropped by BuildQualifiedTableName, + // producing a valid identifier and real rows on the DESC path -> Thrift<->SEA divergence. + if (MetadataUtilities.HasEmptyStringIdentifier(_metadataCatalogName, _metadataSchemaName, _metadataTableName)) + { + activity?.AddEvent("statement.get_columns_extended.returning_empty_result", [ + new("reason", "Empty-string identifier argument") + ]); + return CreateEmptyExtendedColumnsResult(MetadataSchemaFactory.CreateColumnMetadataSchema()); + } + if (string.IsNullOrEmpty(_metadataTableName)) throw new ArgumentException("Table name is required for GetColumnsExtended"); diff --git a/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs b/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs index 2c2bc4857..e1c571fbd 100644 --- a/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs +++ b/csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs @@ -363,6 +363,16 @@ private AdbcConnection CreateThriftConnection() return db.Connect(new Dictionary()); } + // Connection pinned to the SEA (REST) protocol regardless of the configured default. + private AdbcConnection CreateSeaConnection() + { + var parameters = GetDriverParameters(TestConfiguration); + parameters[DatabricksParameters.Protocol] = "rest"; + var driver = new DatabricksDriver(); + var db = driver.Open(parameters); + return db.Connect(new Dictionary()); + } + /// /// Executes a metadata command and returns the total row count. Must not throw. /// @@ -542,5 +552,34 @@ public async Task GetColumnsExtended_EmptySchema_ReturnsEmptyWithoutThrowing() table: TestConfiguration.Metadata.Table); Assert.Equal(0, rows); } + + // SEA-path parity: the empty-catalog / empty-schema short-circuit lives in + // StatementExecutionStatement.GetColumnsExtendedAsync as well. Without it, the DESC + // path's BuildQualifiedTableName drops the empty part and returns real rows, diverging + // from Thrift. These SEA-pinned variants verify both protocols agree on empty results. + + [SkippableFact] + public async Task GetColumnsExtended_EmptyCatalog_Sea_ReturnsEmptyWithoutThrowing() + { + SkipIfNotConfigured(); + using var conn = CreateSeaConnection(); + int rows = await ExecuteMetadataRowCount(conn, "GetColumnsExtended", + catalog: "", + schema: TestConfiguration.Metadata.Schema, + table: TestConfiguration.Metadata.Table); + Assert.Equal(0, rows); + } + + [SkippableFact] + public async Task GetColumnsExtended_EmptySchema_Sea_ReturnsEmptyWithoutThrowing() + { + SkipIfNotConfigured(); + using var conn = CreateSeaConnection(); + int rows = await ExecuteMetadataRowCount(conn, "GetColumnsExtended", + catalog: TestConfiguration.Metadata.Catalog, + schema: "", + table: TestConfiguration.Metadata.Table); + Assert.Equal(0, rows); + } } }