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

Refactor db tests #15076

Merged
merged 10 commits into from
Jun 12, 2020
Merged

Refactor db tests #15076

merged 10 commits into from
Jun 12, 2020

Conversation

ruudboon
Copy link
Member

@ruudboon ruudboon commented Jun 7, 2020

Hello!

In raising this pull request, I confirm the following:

  • I have read and understood the Contributing Guidelines
  • I have checked that another pull request for this purpose does not exist
  • I wrote some tests for this PR
  • I have updated the relevant CHANGELOG
  • I have created a PR for the documentation about this change

Small description of change:

Refactored UniquenessCest

@ruudboon
Copy link
Member Author

ruudboon commented Jun 7, 2020

@niden If you have time can you ping me to check how we can solve the sql db-lock? https://github.com/phalcon/cphalcon/runs/747581338?check_suite_focus=true#step:17:180

@ruudboon ruudboon requested a review from niden June 7, 2020 20:07
@niden
Copy link
Member

niden commented Jun 8, 2020

Sqlite does not like group statements, where etc. Anything "fast" i.e. tests running seems to lock the sqlite database.

@ruudboon
Copy link
Member Author

ruudboon commented Jun 8, 2020

Ok, let's leave it open then. I will try to fix it or remove sqlite from these tests.

@ruudboon
Copy link
Member Author

ruudboon commented Jun 8, 2020

Looks like connection to db is still open (maybe caused by a transaction?)
I'm wondering if PRAGMA journal_mode = wal; could solve our sqlite issues.

@ruudboon
Copy link
Member Author

ruudboon commented Jun 9, 2020

@jenovateurs
Copy link
Contributor

Hi @ruudboon, I think it's a type trouble. PostgreSQL is very strict about that.
In your case, obj_name it's a varchar and obj_type is a smallint, so two differents types but in the SQL request there is no differences.
So I think you discovered a new Phalcon bug. I don't understand why Phalcon check obj_name and obj_type with the same parameters, it makes no sense to me.
I think it's work with MySQL because there is no strong check about the type and it doesn't affect the result with the keyword OR.
The problem might be here :

} elseif count(field) > 1 {

These days, I have too much work to do. I haven't the time to debug line by line this code.

@ruudboon
Copy link
Member Author

ruudboon commented Jun 10, 2020

@jenovateurs Thnx for your feedback. I expected some schema issues. But this is great feeback I will look into this. Thnx.

@ruudboon ruudboon requested a review from niden June 12, 2020 11:51
@ruudboon ruudboon merged commit a7bc295 into phalcon:4.1.x Jun 12, 2020
@ruudboon ruudboon deleted the refactor-db-tests branch June 12, 2020 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants