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

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

Closed
craigdietrich opened this issue Feb 19, 2020 · 2 comments · Fixed by #151
Closed
Assignees
Labels
question store Related to the RDF store.

Comments

@craigdietrich
Copy link
Contributor

craigdietrich commented Feb 19, 2020

(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
Copy link
Collaborator

k00ni commented Feb 19, 2020

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 k00ni added question store Related to the RDF store. labels Feb 19, 2020
@k00ni k00ni self-assigned this Feb 19, 2020
@k00ni
Copy link
Collaborator

k00ni commented Feb 20, 2020

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.

k00ni added a commit that referenced this issue Mar 2, 2020
Attempt to fix #133 + #135

Co-authored-by: Craig Dietrich <craigdietrich@gmail.com>
k00ni added a commit that referenced this issue Mar 2, 2020
…of empty values)

Attempt to fix #133 + #135

Co-authored-by: Craig Dietrich <craigdietrich@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question store Related to the RDF store.
Projects
None yet
2 participants