fix: unbreak build (AE_PARSE_ERROR) + ODBC driver x86 (C4100/C2733)#119
Merged
Conversation
The ODBC driver failed to compile on MSVC x86 /WX (CI only builds x64, so these slipped past). Two x86-only issues: 1. C4100: SQLSetStmtAttr's SQLINTEGER param was named 'a' but unused → /WX error. Drop the name. 2. C2733: SQLColAttribute's final parameter is SQLLEN* on 64-bit but SQLPOINTER on 32-bit per the Windows SDK sqlext.h (guarded by 'defined(_WIN64) || defined(SQLCOLATTRIBUTE_SQLLEN)'). Our fixed SQLLEN* definition conflicted with the 32-bit declaration, which MSVC rejects as an extern "C" overload. Mirror the SDK guard and alias back to SQLLEN* internally. x64 is unchanged: the _WIN64 branch is byte-identical to the prior signature. Verified: - x86 /WX full build clean; x86 unit suite 873/873, 0 failed. - x64 openads_odbc.cpp compiles clean /W4 /WX. Addresses the x86 build gap reported in issue #3. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request resolves compilation issues on 32-bit Windows (x86) by conditionally defining the SQLColAttribute signature based on platform bitness, and removes an unused parameter name in SQLSetStmtAttr. The review feedback recommends using static_cast instead of reinterpret_cast when casting the SQLPOINTER (which is void*) to SQLLEN* to align with idiomatic C++ practices.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| SQLUSMALLINT field, SQLPOINTER charattr, | ||
| SQLSMALLINT bufmax, SQLSMALLINT* strlen, | ||
| SQLPOINTER numattr_) { | ||
| SQLLEN* numattr = reinterpret_cast<SQLLEN*>(numattr_); |
The INDEX ON SELECT-result guard (PR #118) referenced openads::AE_SYNTAX_ERROR, which is not a member of the openads error enum. On Windows the SAP ACE SDK headers define AE_SYNTAX_ERROR as a macro (see the #undef block in error.h), so a local MSVC build with that header present silently accepted it — but clang/gcc and a clean MSVC build reject it, breaking main on Linux/macOS/PHP and x64. Use AE_PARSE_ERROR (7200), the existing parse/syntax error code the SQL parser already emits, which exists in the enum on all toolchains. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 26, 2026
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
Two build fixes. The first unbreaks
main(currently red on clang/macOS/PHP and a clean MSVC); the second fixes the x86 (32-bit) ODBC driver build behind issue #3.1.
AE_PARSE_ERRORin the INDEX ON guard (unbreaks main)PR #118's INDEX-ON-SELECT guard referenced
openads::AE_SYNTAX_ERROR, which is not a member of theopenadserror enum. On Windows the SAP ACE SDK headers defineAE_SYNTAX_ERRORas a macro (see the#undefblock inerror.h), so a local MSVC build with that header silently accepted it — but clang/gcc and a clean MSVC reject it. UseAE_PARSE_ERROR(7200), the existing parse/syntax code the SQL parser already emits.2. ODBC driver x86 build — C4100 + C2733
The ODBC driver (
bindings/odbc/openads_odbc.cpp) does not compile on MSVC x86/WX(CI only builds x64, so it slipped past):SQLSetStmtAttr'sSQLINTEGERparam was namedabut unused. Drop the name.SQLColAttribute's final param isSQLLEN*on 64-bit butSQLPOINTERon 32-bit per the Windows SDKsqlext.h(defined(_WIN64) || defined(SQLCOLATTRIBUTE_SQLLEN)). OurSQLLEN*definition conflicted with the 32-bit declaration →extern "C"overload rejection. Mirror the SDK guard, alias back toSQLLEN*internally.x64 unchanged
On x64
_WIN64is defined, soSQLColAttributetakes the byte-identical original signature.SQLSetStmtAttrchange is cosmetic.Verification
/WXfull build clean; x86 unit suite 873/873, 0 failed (361,197 assertions).openads_odbc.cppcompiles clean/W4 /WX.Scope
bindings/odbc/+ one line in the existing INDEX ON guard. Does not touch the wire protocol.🤖 Generated with Claude Code