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

PDO Firebird $_transactionID property #935

Closed
dregad opened this issue Mar 17, 2023 · 3 comments
Closed

PDO Firebird $_transactionID property #935

dregad opened this issue Mar 17, 2023 · 3 comments
Assignees
Labels
bug firebird Firebird (Tier 2) pdo The PHP PDO Driver (Tier 2) PHP8.2 PHP8.2 Compatibility Issues
Milestone

Comments

@dregad
Copy link
Member

dregad commented Mar 17, 2023

While going through #926, I reviewed usage of the _transactionID property within PDO Firebird driver.

It is only accessed in _affectedrows() method when calling fbird_affected_rows(), but never actually set.

The native Firebird driver initializes it from fbird_trans()'s return value, when initializing the transaction in beginTrans().

AFAICT, it is not possible to retrieve the transaction ID using PDO functions - PDO::beginTransaction() just returns a boolean, so I believe the PDO driver should be modified like this.

	public function _affectedrows()
	{
-		return fbird_affected_rows($this->_transactionID ? $this->_transactionID : $this->_connectionID);
+		return fbird_affected_rows($this->_connectionID);
	}

@mnewnham thoughts ?

@mnewnham mnewnham added bug pdo The PHP PDO Driver (Tier 2) firebird Firebird (Tier 2) labels Mar 17, 2023
@mnewnham
Copy link
Contributor

Surely this entire function is a copy/paste mistake because it's calling native mode functionality, fbird_affected_rows() from within the pdo driver and that function wouldn't exist. The entire function should be removed. (it doesnt exist in any other driver apart from the base class).

@dregad
Copy link
Member Author

dregad commented Mar 17, 2023

OK thanks I'll do that

@dregad dregad added the PHP8.2 PHP8.2 Compatibility Issues label Mar 17, 2023
@dregad dregad assigned dregad and unassigned mnewnham Mar 17, 2023
@dregad dregad added this to the v5.22.5 milestone Mar 17, 2023
dregad added a commit to dregad/ADOdb that referenced this issue Mar 17, 2023
- _init() did not actually initialize anything useful, as the
  $pdoDriver property was not used anywhere.
- _affectedrows() was likely introduced by mistake (copy/paste error
  when the driver was refactored in 2019), as it references Firebird
  native (non-PDO) function fbird_affected_rows(), as discussed in
  ADOdb#935.

This fixes PHP 8.2 deprecation warnings due to undefined properties.
@dregad
Copy link
Member Author

dregad commented Mar 17, 2023

Fixed in 5766f6b

@dregad dregad closed this as completed Mar 17, 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) pdo The PHP PDO Driver (Tier 2) PHP8.2 PHP8.2 Compatibility Issues
Projects
None yet
Development

No branches or pull requests

2 participants