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

ILIKE-based portion of the query for PostgreSQL #173

Closed
Portaflex opened this issue Jul 17, 2016 · 17 comments
Closed

ILIKE-based portion of the query for PostgreSQL #173

Portaflex opened this issue Jul 17, 2016 · 17 comments

Comments

@Portaflex
Copy link
Contributor

I would suggest to include some functions in Query Builder Class to use the PostgreSQL ILIKE extension. Despite not a SQL standard, it is important to generate case-insensitive LIKE expressions in Postgres.

We could add some abstract methods to BaseBuilder.php to be extendend by Postgre/Builder.php. Internal _like() method would need just a little change.

@mwhitneysdsu
Copy link
Contributor

Query Builder is supposed to be as database-independent as possible, so extending it with database-specific functionality probably shouldn't be part of the framework (though I am of the opinion that it should be possible for developers to do so in their application).

On the other hand, this does bring up the question of whether the Builder class for each database platform should be generating different SQL strings to get similar results across platforms in cases like this.

@jim-parry
Copy link
Contributor

BaseBuilder should only include cross-database methods, IMHO.
The Postgres drivers can extend those, but then a developer would have to know that they are using Postgre\Builder, and not just a BaseBuilder.

@Portaflex
Copy link
Contributor Author

As far as I can see (not that much), db-specific Builder is not called anywhere. So, given that we have the property $DBDriver and db-specific Builder extends BaseBuilder, it would make sense for me to change BaseConnection table() method to instantiate the specific Builder for each database. Then, we could add more functionality to Query Builder. It also would be pretty evident when reading table() method code.

@lonnieezell
Copy link
Member

The db-specific version is instantiated right in the table() command of the BaseConnection.

The connection class name will be a db-specific class when that method is called. So, for PostgreSQL it would be CodeIgniter\Database\Postgre\Connection. That line replaces Connection with Builder, leaving CodeIgniter\Database\Postgre\Builder.

I agree that we should provide things as cross-platform as possible, and not add in too many platform-specific things, though I know that the MySQL driver has always had a couple of features that seemed to never get implemented in most of the other drivers in the past. Hopefully we can fix that...

In this specific case, I'd be tempted to simply make the case-insensitive version the default. That matches with what you get from MySQL, and seems like it would be the most frequently used case to me, anyway. However, you said it's an extension? Is it not always available?

@Portaflex
Copy link
Contributor Author

Ja!, I new I should be wrong. I took $this as the mother, not the child class. Anyway, same family. ;-)

ILIKE is a native extensión, therefore it is always in Postgres instalation; looks like they know they are so "strict". In CI-3, I've being working fine just changing LIKE to ILIKE in Query Builder Class, but don't like it.

So, you all are the wizards, you decide. I'm happy to receive such interesting particular lessons.

@mwhitneysdsu
Copy link
Contributor

ILIKE is not available elsewhere, mostly because it would be useless on most other platforms most of the time, but it is always available in PostgreSQL, as Portaflex stated.

The main reasons I would be reluctant to make \Postgre\Builder's like() method use ILIKE instead of LIKE would be:

  • some LIKE expressions don't need to be case-insensitive
  • someone familiar with PostgreSQL's implementation of LIKE may write their query for LIKE and get unexpected results due to use of ILIKE

The obvious upside of using ILIKE for like() is that someone unfamiliar with PostgreSQL and/or porting queries from MySQL/MS SQL/Oracle/etc. doesn't have to know that PostgreSQL's LIKE implementation is different. If implemented with ILIKE, it would need to be noted somewhere in the docs for people moving from CI3 and/or those aware of the normal behavior of PostgreSQL's LIKE.

@lonnieezell
Copy link
Member

I wonder how many of the other platforms support both case-sensitive and case-insensitive version of LIKE and if it would be worth adding a case-sensitive flag to the like() method, along with noting in the docs which platforms support that. I'm not familiar enough with them, so will have to start reading more docs tonight, I suppose.

@mwhitneysdsu
Copy link
Contributor

A quick look shows that in MySQL, MS SQL Server, and Oracle it depends on configuration, encoding, and collation. In most cases (including PostgreSQL) to guarantee a case-insensitive search you would do something like this:

SELECT {columns} FROM {table} WHERE lower({column}) LIKE lower('AbCd%')

@Portaflex
Copy link
Contributor Author

upper and lower functions make a whole column scan, and indexes become useless. It can take much resources in big tables. In most DBs you can set collation different from binary to do case-insensitive comparison. But Postgres has ILIKE, which is unique and wise.

Perhaps we could add a variable $like = 'LIKE' at the end of _like() signature so a iLike() method() in Postgres\Builder could pass 'ILIKE' as last parameter to _like(). In that case, _ilike() would build an ILIKE quiery.

My point is that 99% of functions in Query Builder are MySQL ones. So Postgres deserves that little "ilike". Thanks.

@lonnieezell
Copy link
Member

I understand your point, but then we have another dozen engines that want just their little method. And it soon becomes a mess. We'll definitely look into this further and see if there's a way to do it that still makes it simple for those that do support things like this and those that don't.

To be fair - historically PHP and MySQL have been used together much more often than not, and any driver can always do raw where's, whole queries, etc for portions that are not covered by the builder. :)

@Portaflex
Copy link
Contributor Author

Ok. Let's move forward. Thanks.

@mwhitneysdsu
Copy link
Contributor

By default, ILIKE does not use an index, either, unless the search criteria begins with non-alphabetic characters. The small amount of information I found on the matter indicated that ILIKE may be slower than lower(...) LIKE lower(...) in most cases. You can install a module (pg_trgm) to add index types which will be used for LIKE and ILIKE queries with better performance than a standard index.

I'm not sure how much MySQL-specific functionality is actually in the Builder. There is a lot of MySQL-specific functionality in the configuration of the database library itself, but the purpose of the Query Builder is to construct SQL queries in a manner which is not platform-specific.

I do have to say that in researching this issue, I was quite surprised that LIKE queries on other platforms may not be case insensitive. I'm guessing that I get very frustrated with it when the issue arises, then forget about it when I don't have to deal with it again for months at a time.

@lonnieezell
Copy link
Member

Interesting tidbit I found tonight:

However, AFAICS there isn't any direct notion of "case insensitive LIKE"
in SQL92, so he's going to have to depend on something that's not in
the spec.

What the spec seems to envision is that you get this result by attaching
a case-insensitive collation spec to the column you're going to do the
LIKE on --- in other words, the meaning of "foo LIKE 'bar'" depends on
the charset and collation attributes of the foo column. If you want
something other than what the column was set up to provide, tough
cookies.

Not overly helpful, of course.

@lonnieezell
Copy link
Member

The more I'm reading about things tonight the more complex the issue is to handle correctly, since it all depends on collation of the database/table/column when it was created, and that's only based on the standard. It appears that different databases have different case-sensitive settings. We could hack stuff together to read the collation of the database and change it on the fly, but that gets messy.

My suggestion is we add a fourth parameter to like() that forces a case-insensitive search. Without passing true to it, the LIKE query would be whatever was set for that database/collation. By passing true this would force case-insensitive searches, simply doing the LOWER() conversions. We would need to note that in the docs, though, so that people would know to create their indexes appropriately. Then, in individual drivers if we find a simple solution to forcing case-sensitive, like PostgreSQL, we can implement it there by default.

Thoughts?

@Portaflex
Copy link
Contributor Author

I vote yes to Mr. Ezell proposal and will go to prepared queries, a very "hot spot" too. Thanks

@jim-parry
Copy link
Contributor

Sensible

@lonnieezell
Copy link
Member

Forgot to mark it in commit message, but fixed in cce0a43.

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

No branches or pull requests

4 participants