search

semsol / arc2

ARC RDF Classes for PHP
Other
332 stars 89 forks source link

fetchRow() returns false, not null, resulting in false positive for hasHashColumn() #135

Closed craigdietrich closed 1 year ago

craigdietrich commented 4 years ago

(Submitting this sanity check before a pull request.)

We're working with some old ARC databases that we've upgraded the library for. It seems that 'val_hash' has been added as a column to the s2val table, but in our old databases this column doesn't get added. That said, there's a check for the existence of 'val_hash' in the core library:

https://github.com/semsol/arc2/blob/master/store/ARC2_Store.php#L189-L200

However, AFAICT, $this->db->fetchRow(...) returns false, not null, when it can't find the column. This results in a false positive because of this line which is looking for null:

$this->$var_name = null !== $row;

On our installs I've changed that line to this, and it seems to get things working as expected:

 $this->$var_name = null;
 if ($row) $this->$var_name = true;

Thanks for taking a look at this!

EDIT by @k00ni: Added code highlighting.

k00ni commented 4 years ago

Hi @craigdietrich, thank you for your feedback. My time is limited currently and i can only supervise this with limit resources. Maybe @semsol can help here. If you found problems, you are free to raise them and make pull requests. We would love to merge them, in case test suite runs through.

But currently, some tests fail since our last commits and i can't tell where the source of it is located. It seems to be in relation with mysqli. However, don't hesitate to add code / fix things. Please add tests to your pull request which show that your code is working and doesn't bring things.

Looking forward to hear from you.

k00ni commented 4 years ago

Hi @craigdietrich, i created #137 (see branch fix-tests) which aims to fix all tests and provide a stable basement for future improvements/pull requests. Please use it, if you want to make contributions. I will try to merge it to master asap.