Skip to content

fix(sql): return error for unsupported dialect#1362

Merged
zeroshade merged 3 commits into
apache:mainfrom
fallintoplace:fix/sql-dialect-error-handling
Jul 4, 2026
Merged

fix(sql): return error for unsupported dialect#1362
zeroshade merged 3 commits into
apache:mainfrom
fallintoplace:fix/sql-dialect-error-handling

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

Summary

  • Changed SQL catalog dialect resolution to return typed errors instead of panicking for unsupported dialects.
  • Updated registry usage to rely on clean error flow from 'sql.NewCatalog'.
  • Added test for direct catalog creation via NewCatalog with unsupported dialect.

Testing

  • go test ./catalog/sql -run 'TestInvalidDialect|TestNewCatalogInvalidDialect' -count=1
  • go test ./catalog/sql -count=1

@fallintoplace fallintoplace requested a review from zeroshade as a code owner July 3, 2026 23:02

@zeroshade zeroshade left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Replacing the unsupported-dialect panic with a proper error return is the right call, the supported dialect set is preserved, and both the loader and direct-constructor paths are covered by tests.

@zeroshade zeroshade merged commit d416dfb into apache:main Jul 4, 2026
15 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