fix(pkg-r): Use database-agnostic SQL for SQL Server compatibility#197
Open
fix(pkg-r): Use database-agnostic SQL for SQL Server compatibility#197
Conversation
Replace LIMIT syntax with database-agnostic alternatives in DBISource: - Use `WHERE 1=0` instead of `LIMIT 0` to get column names - Use `dbSendQuery()` + `dbFetch(n=1)` instead of `LIMIT 1` for sampling Fixes #112 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes SQL Server compatibility issues in the R package's DBISource class by replacing non-standard LIMIT clauses with database-agnostic SQL patterns. The changes ensure the code works across all SQL database systems including SQL Server, which uses TOP N instead of LIMIT N.
Changes:
- Replace
LIMIT 0withWHERE 1=0for column name retrieval - Replace
LIMIT 1withdbSendQuery()+dbFetch(n=1)pattern for type detection sampling
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
LIMIT 0withWHERE 1=0to get column names (standard SQL)LIMIT 1withdbSendQuery()+dbFetch(n=1)for type detection samplingFixes #112
Problem
SQL Server uses
TOP Ninstead ofLIMIT N, causing errors like:The R package's
DBISourceclass was usingLIMITsyntax in two places:initialize():SELECT * FROM table LIMIT 0to get column namesget_schema_impl():SELECT * FROM table LIMIT 1to sample a row for type detectionSolution
Use database-agnostic approaches that work across all SQL databases:
Fix 1:
WHERE 1=0instead ofLIMIT 0WHERE 1=0is ANSI SQL that returns zero rows while preserving column metadata. This is a standard pattern supported by all SQL databases including SQL Server, PostgreSQL, MySQL, SQLite, and DuckDB.Fix 2:
dbFetch(n=1)instead ofLIMIT 1DBI's
dbSendQuery()+dbFetch(n=1)+dbClearResult()pattern delegates row limiting to the database driver, which handles dialect-specific syntax internally. This is the same approach already used byDBISource$test_query().Why Python doesn't need changes
The Python package handles SQL Server through different mechanisms:
DataFrameSource: Uses in-memory DuckDB which supportsLIMITSQLAlchemySource: Already has a fallback at lines 657-662:PolarsLazySource: Uses Polars SQLContext which supportsLIMITIbisSource: Usesresult.limit(1).execute()which is Ibis's API that handles SQL dialect translation automaticallyTest plan
🤖 Generated with Claude Code