Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mysqli: metacolumns() returning different info. #980

Closed
1 task done
NathanGibbs3 opened this issue Jun 10, 2023 · 7 comments
Closed
1 task done

mysqli: metacolumns() returning different info. #980

NathanGibbs3 opened this issue Jun 10, 2023 · 7 comments
Assignees
Labels
bug firebird Firebird (Tier 2) ldap mysqli MySQL and compatible drivers (MariaDB, etc) (Tier 1) OCI8 (Oracle) Oracle DBMS (Tier 1) pdo The PHP PDO Driver (Tier 2) pgsql PostgreSQL (Tier 1) sqlite The SQLite Database (Tier 1) sybase Sybase (unsupported)

Comments

@NathanGibbs3
Copy link
Contributor

NathanGibbs3 commented Jun 10, 2023

Description

The metacolumns() function is returning different info in the master branch compared to the last release.

Environment

  • ADOdb version: d4f9f45
  • Driver or Module: mysqli
  • Database type and version: MySQL 10.3.23 - 14.14
  • PHP version: 7.0.25 - 8.1.12
  • Platform: Linux
  • I have tested that the problem is reproducible in the master branch

Steps to reproduce

Detailed, step-by-step instructions to reproduce the behavior, including:

  • Code to reproduce & compare with 5.22.5
$lib = 0; // 0 use 5.22.5 release 1 use snapshot of master branch.
$driver = 'mysqli';

$svr = 'db';
$usr = 'test';
$pwd = 'password';

$DB = 'test';
$tbl = 'test';
$fld = 'test';

if ( $lib == 0 ){
    include('../../ADOdb-5.22.5/adodb.inc.php');
}else{
    include('../../ADOdb/adodb.inc.php');
}

$db = newAdoConnection($driver);
$db->connect($svr, $usr, $pwd, $DB);
$mctmp = $db->metacolumns($tbl);

print $db->Version()."\n";
print $db->isConnected()."\n";
print $db->ErrorMsg()."\n";
var_dump($mctmp[strtoupper($fld)]->max_length);
  • SQL to create and populate database objects used by the code snippet
CREATE TABLE test ( test VARCHAR(255) );

Expected behavior

master branch code returns 255.

Additional context

Found this while working on #978
Reproduced locally and in CI.

@NathanGibbs3 NathanGibbs3 added the triage New issues not yet reviewed by ADOdb developers label Jun 10, 2023
@dregad dregad added bug mysqli MySQL and compatible drivers (MariaDB, etc) (Tier 1) labels Jun 10, 2023
@dregad dregad self-assigned this Jun 10, 2023
@dregad dregad added this to the v5.23.0 milestone Jun 10, 2023
@dregad
Copy link
Member

dregad commented Jun 10, 2023

Thanks for the bug report.

According to git bisect, the culprit is 5c9e24c. I'll look into it.

@dregad
Copy link
Member

dregad commented Jun 10, 2023

So the problem is caused by using $mode parameter instead of class property $adodbFetchMode (set by parent constructor). Problem is also present in other drivers, will add relevant tags.

@dregad dregad added pgsql PostgreSQL (Tier 1) OCI8 (Oracle) Oracle DBMS (Tier 1) pdo The PHP PDO Driver (Tier 2) sqlite The SQLite Database (Tier 1) firebird Firebird (Tier 2) ldap sybase Sybase (unsupported) and removed triage New issues not yet reviewed by ADOdb developers labels Jun 10, 2023
@dregad dregad removed this from the v5.23.0 milestone Jun 10, 2023
@dregad
Copy link
Member

dregad commented Jun 10, 2023

Removing the 5.23.0 milestone, since the commit that introduced the regression was never part of an official release.

@NathanGibbs3
Copy link
Contributor Author

Thanks for the bug report.

According to git bisect, the culprit is 5c9e24c. I'll look into it.

Oh, one of those. 😄
Fix an overarching issue in the app's architecture; then the wheels fall off everywhere else.
Usually because the app made assumptions based on the now corrected bug, or compensated for it in some way that is no longer applicable.
I've done it too, fix one bug, find 10 more. as a result. In the end the codebase gets better. 👍

@NathanGibbs3
Copy link
Contributor Author

It looks like the parent class $fetchMode variable is not propagating down to the drivers.
Probably this at the beginning of the drivers will set things right.

if ($mode === false) {
	$mode = $this->fetchMode;
}

Unless the $mode flag for the drivers doesn't need to be passed anymore in which case, simply

$mode = $this->fetchMode;

without the if block should do it.

@NathanGibbs3
Copy link
Contributor Author

Verified locally, both approaches to this issue correct it.
If you want, I'll go through and add the fix to the drivers changed in the git bisect above & put it together into a PR for you.
Would you want the single line or if block fix?

If you want the single line fix, is the $mode parameter for the drivers even necessary anymore?

@dregad
Copy link
Member

dregad commented Jun 10, 2023

Thanks for the offer, I have a patch ready that I'll push shortly. Got sidetracked as I started looking at your other issue 😉

@dregad dregad closed this as completed in 40e5339 Jun 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug firebird Firebird (Tier 2) ldap mysqli MySQL and compatible drivers (MariaDB, etc) (Tier 1) OCI8 (Oracle) Oracle DBMS (Tier 1) pdo The PHP PDO Driver (Tier 2) pgsql PostgreSQL (Tier 1) sqlite The SQLite Database (Tier 1) sybase Sybase (unsupported)
Projects
None yet
Development

No branches or pull requests

2 participants