Skip to content

Conversation

@eric-wang-1990
Copy link
Contributor

Description:

Why

Power BI ODBC path do escape both _ and %:
image

Proposed changes

This PR improves the handling of SQL pattern wildcards in metadata operations for C# drivers:

  • Renamed EscapeUnderscore parameter to EscapePatternWildcards to better reflect its purpose
  • Enhanced wildcard escaping to handle both underscore (_) and percent (%) characters
  • Updated the parameter description to clarify its functionality
  • Fixed a bug in DatabricksConnection.cs where default namespace was not being set correctly

How this was tested

  • Updated existing test cases to use the new parameter name
  • Manually verified the wildcard escaping functionality works correctly with both underscore and percent sign characters

Additional context

SQL pattern matching uses underscores (_) and percent signs (%) as wildcards. When searching for tables or columns that contain these characters literally, they need to be escaped. Previously, we only handled escaping underscores, but this change extends the functionality to handle percent signs as well, providing more complete support for SQL pattern matching.

@github-actions github-actions bot added this to the ADBC Libraries 19 milestone Jun 11, 2025
@eric-wang-1990 eric-wang-1990 changed the title fix(csharp): Enhance pattern wildcard escaping in metadata operations fix(csharp/src/drivers): Enhance pattern wildcard escaping in metadata operations Jun 11, 2025
@eric-wang-1990 eric-wang-1990 changed the title fix(csharp/src/drivers): Enhance pattern wildcard escaping in metadata operations fix(csharp/src/Drivers): Enhance pattern wildcard escaping in metadata operations Jun 12, 2025
Copy link
Contributor

@CurtHagenlocher CurtHagenlocher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! One small comment/request for change.

EscapeUnderscoreInName(TableName),
(CatalogName),
(SchemaName),
(TableName),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this supposed to be changed to EscapePatternWildcardsInName or does this Thrift API not work with wild cards? If the latter, then please remove the parens from the arguments.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see in the other PR that this doesn't need wildcards and the parens are removed there. In any event, these would need to be removed to avoid a merge conflict.

Copy link
Contributor

@CurtHagenlocher CurtHagenlocher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@CurtHagenlocher CurtHagenlocher merged commit 8b1757a into apache:main Jun 20, 2025
6 checks passed
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.

2 participants