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

Delete with joins #2944

Closed
ipeshev opened this issue Mar 16, 2014 · 17 comments
Closed

Delete with joins #2944

ipeshev opened this issue Mar 16, 2014 · 17 comments

Comments

@ipeshev
Copy link

ipeshev commented Mar 16, 2014

It would be great, having possibility to build delete with joins query using active records class. If it's ok I can start implementing it right away.

@galindus
Copy link

Hi ipeshev I just forked to fix that too, my target db is postgresql but it has a different keyword I have had to use regexp. Just pushed the idea to my fork.

\BR

@ipeshev
Copy link
Author

ipeshev commented Mar 17, 2014

My problem emerged from MySQL, but I guess it should be implemented also for PostGre and MSSql atleast. I will have time to look at the code in the weekend sooner.

@narfbg
Copy link
Contributor

narfbg commented Mar 17, 2014

@ipeshev Your guess is something that most people are missing ... not only it should, but it must be implemented for all database platforms that support the feature. If it is specific to just i.e. MySQL, it won't be accepted.

@ipeshev
Copy link
Author

ipeshev commented Mar 17, 2014

"Guess" is for curtousy, mysql, mssql and postgre supports it as a feature so atleast for those it is possible to be implemented straight away.

@galindus
Copy link

Sure it needs to be fixed for all db drivers, I've been doing a bit of research about the current status for delete joins.

Postgresql:

DELETE FROM table1 using table2 WHERE table2.somecolumn = @somevalue

Limitations - does not support outer joins.

MySQL / MSSQL / PDO / SQLSRV?:

DELETE t1 FROM table1 t1 (TJOIN) JOIN table2 t2 ON t2.table1Id = t1.Id WHERE t2.somecolumn = @somevalue

Ibase / oci8 / odbc / SQLITE:
Not supported, use WHERE EXISTS(Subquery) or WHERE t1.id IN (Subquery)

DELETE FROM table1 t1
WHERE t1.id IN (
    SELECT t2.table1Id FROM table2 t2
    /** Possible to add other joins **/
    WHERE t2.somecolumn = @somevalue
)

My first idea was to add support for common AR join syntax.

$this->db->where("table2.somecolumn",$somevalue)
           ->join("table2", "table2.table1Id =  table1.id")
           ->delete("table1");

But maybe it is worth to add an AR command like delete_join.

What do you think?

\BR

@narfbg
Copy link
Contributor

narfbg commented Mar 17, 2014

Um, no ... I don't think that delete_join() would be appropriate. If it happens, it must be with regular Query Builder (that's how it's called now, no longer AR) syntax.

@ipeshev
Copy link
Author

ipeshev commented Mar 17, 2014

Basicly every delete and update is at first select, so supporting this kind of syntax would be the most intuitive.

@narfbg
Copy link
Contributor

narfbg commented Mar 17, 2014

FYI, I've looked into the same thing for UPDATEs and it turned out to be MySQL-specific (or with other databases having a syntax so different that it's not feasible to implement), so I wouldn't be surprised if we end up with the same decision here.

@galindus
Copy link

Hi! I've commited the idea to my fork in master branch since I started working on that release.

The idea is to use the join method in allowed engines: MySQL / MSSQL / PDO / SQLSRV

DELETE t1 FROM table1 t1 (TJOIN) JOIN table2 t2 ON t2.table1Id = t1.Id WHERE t2.somecolumn = @somevalue

And use subquery for other engines: postgresql / Ibase / oci8 / odbc / SQLITE:

DELETE FROM table1 t1
WHERE t1.id IN (
    SELECT t2.table1Id FROM table2 t2
    /** Possible to add other joins **/
    WHERE t2.somecolumn = @somevalue
)

I've run a first test with pgsql and seems to be working, I will run some more tests on mysql and mssql.

I've started looking into dev branch too but I'm still trying to see what have changed in query builder.

\BR

@narfbg
Copy link
Contributor

narfbg commented Mar 19, 2014

Sorry to disappoint you, but I'd suggest that you drop whatever you were doing in your 'master' branch and go straight to develop.
In this repository, $master === $release and that's currently 2.1.4 - we will not accept any new features into the 2.1.x version tree.

@narfbg
Copy link
Contributor

narfbg commented Mar 19, 2014

Oh, and PDO is not a database engine - it's a driver (or an interface) that allows connecting to many platforms, same goes for ODBC. I just noticed that you keep mentioning it as if it has it's own syntax, which is incorrect.

@galindus
Copy link

Hi! no disappoint at all, I'm using 2.1.4 for a project that's why I started working on master but of course it should be implemented in dev so we can move along when it's released.

You are right about PDO and ODBC then we should use IN syntax with that drivers too.

\BR

@galindus
Copy link

BTW can you point some documentation about how to run unit tests? I've been looking into them and from phpunit.xml seems like database tests are not run.

@narfbg
Copy link
Contributor

narfbg commented Mar 19, 2014

Database tests are run on Travis-CI, we otherwise can't assume what databases you have and don't have installed on your system and how are they set up. Some people have replicated the Travis environment, you should try that ... there's some help on the topic on their site.

@galindus
Copy link

Hi narfbg! Thanks I'm running tests now on Travis-CI.

@galindus
Copy link

awesome to see such improvement in tests coverage btw.

@narfbg
Copy link
Contributor

narfbg commented Feb 2, 2015

Well, this is obviously going nowhere ... closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants