Skip to content

fix(csharp): throw ArgumentException for GetPrimaryKeys with null table name (PECO-2956)#342

Open
msrathore-db wants to merge 1 commit intomainfrom
fix/peco-2956-getprimarykeys-null-table-validation
Open

fix(csharp): throw ArgumentException for GetPrimaryKeys with null table name (PECO-2956)#342
msrathore-db wants to merge 1 commit intomainfrom
fix/peco-2956-getprimarykeys-null-table-validation

Conversation

@msrathore-db
Copy link
Collaborator

Summary

  • GetPrimaryKeys in SEA mode silently returned an empty result when table name was null/empty, while Thrift mode threw via the server — inconsistent behavior
  • Added client-side ArgumentException validation for null/empty table name in both SEA (StatementExecutionStatement.cs) and Thrift (DatabricksStatement.cs) paths
  • Both protocols now consistently throw ArgumentException("Table name is required for GetPrimaryKeys") before any server call
  • JDBC SEA also returns empty for null table (not throws), so this fix aligns C# with Thrift behavior rather than JDBC

Test plan

  • Verify GetPrimaryKeys with null table name throws ArgumentException in SEA mode
  • Verify GetPrimaryKeys with null table name throws ArgumentException in Thrift mode
  • Verify GetPrimaryKeys with valid table name still works in both modes
  • Verify GetColumnsExtended (which calls GetPrimaryKeys internally) still works with valid params

Closes PECO-2956

🤖 Generated with Claude Code

…le name (PECO-2956)

GetPrimaryKeys in SEA mode silently returned an empty result when table
name was null, while Thrift mode threw via the server. Add client-side
validation in both protocols so they consistently throw ArgumentException.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant