Skip to content

Remove use of SDWORD and SWORD in ODBC extensions - #14396

Closed
NattyNarwhal wants to merge 2 commits into
php:masterfrom
NattyNarwhal:sqlinteger-is-mightier-than-the-sword
Closed

NattyNarwhal wants to merge 2 commits into
php:masterfrom
NattyNarwhal:sqlinteger-is-mightier-than-the-sword

Conversation

@NattyNarwhal

Copy link
Copy Markdown
Member

This changes instances of SDWORD to SQLINTEGER and instances of SWORD to SQLSMALLINT. There's no point in using these, when we should what the functions define the parameters to actually be. The former change fixes GH-14367.


There are still references to SDWORD and SWORD, but it seems isolated to the Solid driver manager. There is a curious comment in odbc_column_lengths, as well as some orphan definitions of them for Solid in headers. I have no removed these because I'm not sure if Solid still needs these.


I'm targeting master because of the SWORD refactor, but the SDWORD change should presumably be backported as a bug fix that fixes the build.

Some different driver managers disagree if this should be 4 or 8 bytes
in size. SQLGetDiagRec expects this to be an SQLINTEGER, so we should
just use that explicitly instead of hoping that it's the same size.

Fixes phpGH-14367
While this hasn't caused issues like the SQLINTEGER/SDWORD confusion
has, we should use what SQLDescrimeParam calls for, which is
SQLSMALLINT.
@SakiTakamachi

Copy link
Copy Markdown
Member

Thanks! Maybe targeting 8.2 would be a better choice?

@Girgias Girgias 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

@NattyNarwhal

Copy link
Copy Markdown
Member Author

Thanks! Maybe targeting 8.2 would be a better choice?

I agree after thinking about it, I'll try to retarget this PR for 8.2 - and hopefully GitHub doesn't confused like last time I tried changing the target branch.

@NattyNarwhal

Copy link
Copy Markdown
Member Author

Opened PR GH-14457 that targets 8.2 instead, I'll close this if we're going with it instead.

@NattyNarwhal

Copy link
Copy Markdown
Member Author

Merged the 8.2 targeting one instead; has been merged up into 8.3 and master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build failure (incompatible pointer types, gcc-14) with PDO/iODBC

3 participants